Skip to content

Commit

Permalink
PRESIDECMS-2613 add complex feature enabled evaluation
Browse files Browse the repository at this point in the history
In many scenarios, we need to be able to check whether or not
multiple different features are enabled, or whether one feature is
not enabled and another is, etc. This change adds support for
using operators in our feature string to achieve this wherever
we have feature specifications. e.g. in a form, you could now do:

```xml
<field name="myfield" feature="!( featureX && featureY ) || featureZ" />
```
  • Loading branch information
DominicWatson committed Sep 27, 2023
1 parent 19d57db commit 048715b
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 1 deletion.
36 changes: 35 additions & 1 deletion system/services/features/FeatureService.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ component singleton=true autodoc=true displayName="Feature service" {
/**
* Returns whether or not the passed feature is currently enabled
*
* @feature.hint name of the feature to check
* @feature.hint name of the feature to check, or logical expression containing features to check, e.g. "(featurex || featurey) && featurez"
* @siteTemplate.hint current active site template - can be used to check features that can be site template specific
*/
public boolean function isFeatureEnabled( required string feature, string siteTemplate ) autodoc=true {
if ( _isComplexExpression( Trim( arguments.feature ) ) ) {
return _processComplexExpression( arguments.feature );
}
var features = _getConfiguredFeatures();
var isEnabled = IsBoolean( features[ arguments.feature ].enabled ?: "" ) && features[ arguments.feature ].enabled;

Expand Down Expand Up @@ -74,6 +77,37 @@ component singleton=true autodoc=true displayName="Feature service" {
return "";
}

// PRIVATE HELPERS
private boolean function _isComplexExpression( filter ) {
var evalChars = "|&\(\)!\s";
var featureNameChars = "a-zA-Z0-9_\-";

// must contain one ore more special evaluation chars
// and must ONLY contain feature name chars + evaluation chars
return ReFind( "[#evalChars#]+", arguments.filter ) && ReFind( "^[#evalChars##featureNameChars#]+$", arguments.filter );
}

private boolean function _processComplexExpression( filter, siteTemplate ) {
var compiled = arguments.filter;
var features = ReMatch( "\b[a-zA-Z0-9_\-]+\b", arguments.filter );

ArraySort( features, function( a, b ){
var lena = Len( arguments.a );
var lenb = Len( arguments.b );
return lena == lenb ? 0 : ( lena < lenb ? 1 : -1 );
} );

for( var feature in features ) {
compiled = ReplaceNoCase( compiled, feature, isFeatureEnabled( feature, arguments.siteTemplate ) ? "true" : "false", "all" );
}

try {
return Evaluate( compiled );
} catch( any e ) {
throw( type="preside.feature.bad.expression", message="The feature name expression, [#arguments.filter#], could not be evaluated. Compiler error: [#e.message#]." );
}
}

// GETTERS AND SETTERS
private struct function _getConfiguredFeatures() {
return _configuredFeatures;
Expand Down
21 changes: 21 additions & 0 deletions tests/integration/api/features/FeatureServiceTest.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,27 @@ component extends="testbox.system.BaseSpec"{
expect( svc.isFeatureEnabled( feature="sites", siteTemplate="" ) ) .toBeTrue();
} );

it( "should multi-evaluate feature expressions using parenthesis and && and || characters", function(){
var svc = _getService();

expect( svc.isFeatureEnabled( feature="sites || websiteUsers", siteTemplate="" ) ).toBeTrue();
expect( svc.isFeatureEnabled( feature="sites && assetManager", siteTemplate="" ) ).toBeFalse();
expect( svc.isFeatureEnabled( feature="sites && websiteUsers", siteTemplate="" ) ).toBeTrue();
expect( svc.isFeatureEnabled( feature="(sites || what) && websiteusers", siteTemplate="" ) ).toBeTrue();
expect( svc.isFeatureEnabled( feature="((sites && what) && websiteusers)", siteTemplate="" ) ).toBeFalse();
expect( svc.isFeatureEnabled( feature="((sites && !what) && websiteusers)", siteTemplate="" ) ).toBeTrue();
} );

it( "should not attempt dynamic lucee execution with malicious input", function(){
var svc = _getService();

expect( svc.isFeatureEnabled( feature="http url='https://www.google.com';", siteTemplate="" ) ).toBeFalse();
expect( svc.isFeatureEnabled( feature="int( 45.0 )", siteTemplate="" ) ).toBeFalse();
expect( function(){
svc.isFeatureEnabled( feature="floor( something )", siteTemplate="" )
} ).toThrow( "preside.feature.bad.expression" ) ;
} );

} );

describe( "isFeatureDefined()", function(){
Expand Down

0 comments on commit 048715b

Please sign in to comment.