diff --git a/grails-controllers/src/main/groovy/grails/artefact/Controller.groovy b/grails-controllers/src/main/groovy/grails/artefact/Controller.groovy index 707ac284e93..7d6fbb27117 100644 --- a/grails-controllers/src/main/groovy/grails/artefact/Controller.groovy +++ b/grails-controllers/src/main/groovy/grails/artefact/Controller.groovy @@ -363,6 +363,11 @@ trait Controller implements ResponseRenderer, ResponseRedirector, RequestForward */ @Generated def initializeCommandObject(final Class type, final String commandObjectParameterName) throws Exception { + initializeCommandObject(type, commandObjectParameterName, null) + } + + @Generated + def initializeCommandObject(final Class type, final String commandObjectParameterName, final List bindAllowedProperties) throws Exception { final HttpServletRequest request = getRequest() def commandObjectInstance = null try { @@ -434,7 +439,8 @@ trait Controller implements ResponseRenderer, ResponseRedirector, RequestForward } if (shouldDoDataBinding) { - bindData(commandObjectInstance, commandObjectBindingSource, Collections.EMPTY_MAP, null) + final Map includeExclude = bindAllowedProperties == null ? Collections.EMPTY_MAP : [include: bindAllowedProperties] + bindData(commandObjectInstance, commandObjectBindingSource, includeExclude, null) } } } catch (Exception e) { diff --git a/grails-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java b/grails-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java index af4d0dbe958..ed43cec8526 100644 --- a/grails-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java +++ b/grails-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java @@ -42,6 +42,7 @@ import org.codehaus.groovy.ast.ModuleNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; +import org.codehaus.groovy.ast.Variable; import org.codehaus.groovy.ast.expr.ArgumentListExpression; import org.codehaus.groovy.ast.expr.BinaryExpression; import org.codehaus.groovy.ast.expr.BooleanExpression; @@ -93,6 +94,7 @@ import grails.web.Action; import grails.web.RequestParameter; import grails.web.controllers.ControllerMethod; +import grails.web.databinding.BindAllowed; import org.grails.compiler.injection.GrailsASTUtils; import org.grails.compiler.injection.TraitInjectionUtils; import org.grails.core.DefaultGrailsControllerClass; @@ -185,6 +187,7 @@ public class ControllerActionTransformer implements GrailsArtefactClassInjector, GrailsResourceUtils.GRAILS_APP_DIR + "/controllers/(.+)Controller\\.groovy"); private static final String ALLOWED_METHODS_HANDLED_ATTRIBUTE_NAME = "ALLOWED_METHODS_HANDLED"; private static final ClassNode OBJECT_CLASS = new ClassNode(Object.class); + private static final ClassNode BIND_ALLOWED_CLASS_NODE = new ClassNode(BindAllowed.class); public static final AnnotationNode ACTION_ANNOTATION_NODE = new AnnotationNode( new ClassNode(Action.class)); private static final String ACTION_MEMBER_TARGET = "commandObjects"; @@ -722,6 +725,7 @@ protected void initializeMethodParameter(final ClassNode classNode, final BlockS String requestParameterName = paramName; List requestParameters = param.getAnnotations( new ClassNode(RequestParameter.class)); + List bindAllowedAnnotations = param.getAnnotations(BIND_ALLOWED_CLASS_NODE); //Check to see if the method was inherited from a trait if (actionNode instanceof MethodNode && paramName.startsWith("arg")) { @@ -743,6 +747,7 @@ protected void initializeMethodParameter(final ClassNode classNode, final BlockS //Set the request parameter name based off of the parameter in the trait helper method requestParameterName = helperParam.getName(); requestParameters = helperParam.getAnnotations(new ClassNode(RequestParameter.class)); + bindAllowedAnnotations = helperParam.getAnnotations(BIND_ALLOWED_CLASS_NODE); } } } @@ -759,14 +764,15 @@ protected void initializeMethodParameter(final ClassNode classNode, final BlockS } else if (paramTypeClassNode.equals(new ClassNode(String.class))) { initializeStringParameter(classNode, wrapper, param, requestParameterName); } else if (!paramTypeClassNode.equals(OBJECT_CLASS)) { + final Expression bindAllowedExpression = getBindAllowedExpression(source, param, bindAllowedAnnotations); initializeAndValidateCommandObjectParameter(wrapper, classNode, paramTypeClassNode, - actionNode, actionName, paramName, source, context); + actionNode, actionName, paramName, bindAllowedExpression, source, context); } } protected void initializeAndValidateCommandObjectParameter(final BlockStatement wrapper, final ClassNode controllerNode, final ClassNode commandObjectNode, - final ASTNode actionNode, final String actionName, final String paramName, + final ASTNode actionNode, final String actionName, final String paramName, final Expression bindAllowedExpression, final SourceUnit source, final GeneratorContext context) { final DeclarationExpression declareCoExpression = declX(localVarX(paramName, commandObjectNode), new EmptyExpression()); wrapper.addStatement(stmt(declareCoExpression)); @@ -778,7 +784,7 @@ protected void initializeAndValidateCommandObjectParameter(final BlockStatement "]. Interface types and abstract class types are not supported as command objects. This parameter will be ignored."; GrailsASTUtils.warning(source, actionNode, warningMessage); } else { - initializeCommandObjectParameter(wrapper, commandObjectNode, paramName, source); + initializeCommandObjectParameter(wrapper, commandObjectNode, paramName, bindAllowedExpression, source); @SuppressWarnings("unchecked") boolean argumentIsValidateable = GrailsASTUtils.hasAnyAnnotations( @@ -854,14 +860,102 @@ protected void initializeAndValidateCommandObjectParameter(final BlockStatement } protected void initializeCommandObjectParameter(final BlockStatement wrapper, - final ClassNode commandObjectNode, final String paramName, SourceUnit source) { + final ClassNode commandObjectNode, final String paramName, final Expression bindAllowedExpression, SourceUnit source) { final ArgumentListExpression initializeCommandObjectArguments = args(classX(commandObjectNode), constX(paramName)); + if (bindAllowedExpression != null) { + initializeCommandObjectArguments.addExpression(bindAllowedExpression); + } final MethodCallExpression initializeCommandObjectMethodCall = callThisX("initializeCommandObject", initializeCommandObjectArguments); applyDefaultMethodTarget(initializeCommandObjectMethodCall, commandObjectNode); final Expression assignCommandObjectToParameter = assignX(varX(paramName), initializeCommandObjectMethodCall); wrapper.addStatement(stmt(assignCommandObjectToParameter)); } + private Expression getBindAllowedExpression(final SourceUnit source, final Parameter param, final List bindAllowedAnnotations) { + if (bindAllowedAnnotations == null || bindAllowedAnnotations.isEmpty()) { + return null; + } + final AnnotationNode bindAllowedAnnotation = bindAllowedAnnotations.get(0); + final Expression valueExpression = bindAllowedAnnotation.getMember("value"); + final ListExpression allowedProperties = resolveBindAllowedProperties(valueExpression); + if (allowedProperties == null) { + GrailsASTUtils.error(source, param, "@BindAllowed requires a literal list of property names or a static final constant list."); + return new ListExpression(); + } + return allowedProperties; + } + + private ListExpression resolveBindAllowedProperties(final Expression expression) { + if (expression instanceof ConstantExpression) { + final Object value = ((ConstantExpression) expression).getValue(); + if (value instanceof String) { + final ListExpression listExpression = new ListExpression(); + listExpression.addExpression(new ConstantExpression(value)); + return listExpression; + } + } + if (expression instanceof ListExpression) { + return copyStringLiteralList((ListExpression) expression); + } + final FieldNode constantField = resolveStaticFinalField(expression); + if (constantField != null) { + return resolveBindAllowedProperties(constantField.getInitialExpression()); + } + return null; + } + + private ListExpression copyStringLiteralList(final ListExpression sourceList) { + final ListExpression listExpression = new ListExpression(); + for (Expression element : sourceList.getExpressions()) { + final Object value = resolveBindAllowedStringValue(element); + if (!(value instanceof String)) { + return null; + } + listExpression.addExpression(new ConstantExpression(value)); + } + return listExpression; + } + + private Object resolveBindAllowedStringValue(final Expression expression) { + if (expression instanceof ConstantExpression) { + return ((ConstantExpression) expression).getValue(); + } + final FieldNode constantField = resolveStaticFinalField(expression); + if (constantField != null && constantField.getInitialExpression() instanceof ConstantExpression) { + return ((ConstantExpression) constantField.getInitialExpression()).getValue(); + } + return null; + } + + private FieldNode resolveStaticFinalField(final Expression expression) { + if (expression instanceof VariableExpression) { + final Variable accessedVariable = ((VariableExpression) expression).getAccessedVariable(); + if (accessedVariable instanceof FieldNode) { + return staticFinalFieldOrNull((FieldNode) accessedVariable); + } + } + if (expression instanceof PropertyExpression) { + final PropertyExpression propertyExpression = (PropertyExpression) expression; + final Expression objectExpression = propertyExpression.getObjectExpression(); + if (objectExpression instanceof ClassExpression) { + final FieldNode fieldNode = ((ClassExpression) objectExpression).getType().getField(propertyExpression.getPropertyAsString()); + return staticFinalFieldOrNull(fieldNode); + } + } + return null; + } + + private FieldNode staticFinalFieldOrNull(final FieldNode fieldNode) { + if (fieldNode == null) { + return null; + } + final int modifiers = fieldNode.getModifiers(); + if (Modifier.isStatic(modifiers) && Modifier.isFinal(modifiers)) { + return fieldNode; + } + return null; + } + /** * Checks to see if a Module is defined at a path which includes the specified substring * diff --git a/grails-doc/src/en/guide/theWebLayer/controllers/dataBinding.adoc b/grails-doc/src/en/guide/theWebLayer/controllers/dataBinding.adoc index 017661bfb4e..987620c15f7 100644 --- a/grails-doc/src/en/guide/theWebLayer/controllers/dataBinding.adoc +++ b/grails-doc/src/en/guide/theWebLayer/controllers/dataBinding.adoc @@ -1085,6 +1085,6 @@ def p = new Person() bindData(p, params, [include: ['firstName', 'lastName']]) ---- -NOTE: If an empty List is provided as a value for the `include` parameter then all fields will be subject to binding if they are not explicitly excluded. +NOTE: If an empty List is provided as a value for the `include` parameter then no fields will be bound. -The link:{constraintsRef}bindable.html[bindable] constraint can be used to globally prevent data binding for certain properties. +The link:{constraintsRef}bindable.html[bindable] constraint can be used to allow data binding for certain properties by declaring `bindable: true`. Properties are not included in default mass binding unless they are explicitly bindable. Controller action command object parameters may also be annotated with `grails.web.databinding.BindAllowed` to allow only the listed fields for that action. diff --git a/grails-doc/src/en/guide/upgrading.adoc b/grails-doc/src/en/guide/upgrading.adoc index 3d831dcf5bd..84b1047d4fd 100644 --- a/grails-doc/src/en/guide/upgrading.adoc +++ b/grails-doc/src/en/guide/upgrading.adoc @@ -17,3 +17,8 @@ specific language governing permissions and limitations under the License. //// +=== Data Binding Defaults + +Mass data binding now uses an explicit allowlist by default. Properties must declare `bindable: true`, be supplied through `bindData(..., [include: ...])`, or be listed on a controller action parameter with `@BindAllowed` to participate in automatic binding. Empty `include` lists bind no properties. + +To restore the previous behavior while migrating an application, set `grails.databinding.legacyBindableDefault = true`. Prefer replacing that setting with explicit allowlists before production deployment. diff --git a/grails-doc/src/en/ref/Constraints/bindable.adoc b/grails-doc/src/en/ref/Constraints/bindable.adoc index e19d4338385..7c35dc12b90 100644 --- a/grails-doc/src/en/ref/Constraints/bindable.adoc +++ b/grails-doc/src/en/ref/Constraints/bindable.adoc @@ -79,11 +79,12 @@ assert 'Percussion' == employee.department assert 42.0 == employee.salary ---- -Statically typed instance properties are bindable by default. Properties which are not bindable by default are those related to transient fields, dynamically typed properties and static properties. +Properties are not bindable by default. Add `bindable: true` to the relevant constraint to opt a property into mass data binding. Properties with `bindable: false`, transient fields, dynamically typed properties and static properties are not bindable. + +Applications that need the previous default during migration may set `grails.databinding.legacyBindableDefault = true` to allow statically typed instance properties to be bound when no explicit include list is supplied. New applications should prefer explicit `bindable: true` constraints or `bindData(..., [include: ...])` allowlists. See the link:{guidePath}theWebLayer.html#dataBinding[data binding] section for more details on data binding. NOTE: The bindable constraint must be assigned a literal boolean value. Dynamic expressions are not valid values for the bindable constraint. The value must be the literal `true` or `false`. The bindable constraint must be applied in the constraints closure which is defined in the relevant class. This means that bindable may not be used as a shared constraint. - diff --git a/grails-doc/src/en/ref/Controllers/bindData.adoc b/grails-doc/src/en/ref/Controllers/bindData.adoc index a809e63528d..48a698a802c 100644 --- a/grails-doc/src/en/ref/Controllers/bindData.adoc +++ b/grails-doc/src/en/ref/Controllers/bindData.adoc @@ -60,7 +60,29 @@ Arguments: * `includesExcludes` - (Optional) A map with 'include' and/or 'exclude' lists containing the names of properties to either include or exclude. * `prefix` - (Optional) A string representing a prefix to use to filter parameters. The method will automatically append a '.' when matching the prefix to parameters, so you can use 'author' to filter for parameters such as 'author.name'. -NOTE: Note that if an empty List or no List is provided as a value for the `include` parameter then all statically typed instance properties will be subject to binding if they are not explicitly excluded. See the link:{constraintsRefFromRef}bindable.html[bindable] constraint documentation for more information on how to control what is bindable and what is not. +If no `include` list is supplied, `bindData` uses the target class default binding allowlist. Only properties declared with `bindable: true` are included by default. An empty `include` list binds no properties. + +Use `include` to allow only the properties needed for a request: + +[source,groovy] +---- +bindData(target, params, [include: ['firstName', 'lastName']]) +---- + +For controller action command object parameters, use `grails.web.databinding.BindAllowed` to allow request binding for only the listed properties: + +[source,groovy] +---- +import grails.web.databinding.BindAllowed + +class PersonController { + def save(@BindAllowed(['firstName', 'lastName']) PersonCommand command) { + // command.email, command.admin, etc. are not bound by this action + } +} +---- + +See the link:{constraintsRefFromRef}bindable.html[bindable] constraint documentation for more information on controlling default bindability. Applications that need the previous default during migration may set `grails.databinding.legacyBindableDefault = true`. The underlying implementation uses Spring's Data Binding framework. If the target is a domain class, type conversion errors are stored in the `errors` property of the domain class. diff --git a/grails-test-suite-web/src/test/groovy/org/grails/web/binding/DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec.groovy b/grails-test-suite-web/src/test/groovy/org/grails/web/binding/DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec.groovy index c533f468bcd..101049ba502 100644 --- a/grails-test-suite-web/src/test/groovy/org/grails/web/binding/DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec.groovy +++ b/grails-test-suite-web/src/test/groovy/org/grails/web/binding/DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec.groovy @@ -20,8 +20,11 @@ package org.grails.web.binding import grails.gorm.dirty.checking.DirtyCheck import grails.persistence.Entity +import grails.util.Holders import groovy.transform.CompileStatic +import org.grails.config.PropertySourcesConfig import org.grails.validation.ConstraintEvalUtils +import org.grails.web.databinding.DefaultASTDatabindingHelper import spock.lang.Issue import spock.lang.Specification @@ -63,7 +66,7 @@ class DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec extends when: 'a domain class that extends a @DirtyCheck abstract base is bound with id and version' def obj = new MyDirtyCheckedDomain(id: 99L, version: 7L, name: 'Grace') - then: 'the regular property is bound but id and version are not' + then: 'the explicitly bindable regular property is bound but id and version are not' obj.name == 'Grace' obj.id == null obj.version == null @@ -75,7 +78,7 @@ class DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec extends def now = new Date() def obj = new DomainWithInheritedTimestamps(id: 1L, version: 2L, name: 'Alan', title: 'Mathematician', dateCreated: now, lastUpdated: now) - then: 'regular inherited and declared properties are bound' + then: 'explicitly bindable inherited and declared properties are bound' obj.name == 'Alan' obj.title == 'Mathematician' @@ -87,11 +90,11 @@ class DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec extends } @Issue('https://github.com/apache/grails-core/issues/15681') - void 'Test a regular property declared on a @DirtyCheck abstract base is still bound in the domain subclass'() { + void 'Test an explicitly bindable property declared on a @DirtyCheck abstract base is bound in the domain subclass'() { when: 'only regular properties are bound' def obj = new DomainWithInheritedTimestamps(name: 'Grace', title: 'Admiral') - then: 'all regular properties are bound and the special properties remain null' + then: 'explicitly bindable properties are bound and the special properties remain null' obj.name == 'Grace' obj.title == 'Admiral' obj.id == null @@ -117,6 +120,30 @@ class DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec extends obj.version == null } + void 'Test a regular property without bindable true is not bound by default'() { + when: + def obj = new DomainWithDefaultDeniedProperty(name: 'Grace', title: 'Admiral') + + then: + obj.name == null + obj.title == 'Admiral' + } + + void 'Test legacy bindable default binds ordinary properties but preserves bindable false'() { + given: + Holders.setConfig(new PropertySourcesConfig([(DefaultASTDatabindingHelper.LEGACY_BINDABLE_DEFAULT): true])) + + when: + def obj = new DomainWithLegacyBindableFalse(name: 'Grace', title: 'Admiral') + + then: + obj.name == 'Grace' + obj.title == null + + cleanup: + Holders.setConfig(null) + } + @Issue('https://github.com/apache/grails-core/issues/15795') void 'Test explicit bindable:true on a special property declared in a @DirtyCheck abstract base is inherited by the domain subclass'() { when: 'a domain subclass inherits a bindable:true id constraint from its abstract base and declares no constraint of its own' @@ -206,7 +233,7 @@ abstract class AbstractDirtyCheckedBase { @Entity class MyDirtyCheckedDomain extends AbstractDirtyCheckedBase { static constraints = { - name nullable: false + name nullable: false, bindable: true } } @@ -221,6 +248,11 @@ abstract class AbstractDirtyCheckedBaseWithTimestamps { @Entity class DomainWithInheritedTimestamps extends AbstractDirtyCheckedBaseWithTimestamps { String title + + static constraints = { + name bindable: true + title bindable: true + } } @Entity @@ -228,6 +260,8 @@ class DomainWithInheritedBindableTimestamps extends AbstractDirtyCheckedBaseWith String title static constraints = { + name bindable: true + title bindable: true dateCreated bindable: true lastUpdated bindable: true } @@ -238,6 +272,7 @@ abstract class AbstractDirtyCheckedBaseWithBindableId { String name static constraints = { + name bindable: true id bindable: true } } @@ -245,6 +280,10 @@ abstract class AbstractDirtyCheckedBaseWithBindableId { @Entity class DomainInheritingBindableId extends AbstractDirtyCheckedBaseWithBindableId { String title + + static constraints = { + title bindable: true + } } @Entity @@ -253,6 +292,7 @@ class DomainOverridingBindableIdToFalse extends AbstractDirtyCheckedBaseWithBind static constraints = { id bindable: false + title bindable: true } } @@ -261,6 +301,7 @@ abstract class AbstractDirtyCheckedGrandparentWithBindableId { String grandparentName static constraints = { + grandparentName bindable: true id bindable: true } } @@ -273,6 +314,12 @@ abstract class AbstractPlainParentOfBindableId extends AbstractDirtyCheckedGrand @Entity class DomainInheritingBindableIdThroughIntermediate extends AbstractPlainParentOfBindableId { String childName + + static constraints = { + grandparentName bindable: true + parentName bindable: true + childName bindable: true + } } @DirtyCheck @@ -289,4 +336,34 @@ abstract class AbstractPlainParent extends AbstractDirtyCheckedGrandparent { @Entity class DomainExtendingMultiLevelHierarchy extends AbstractPlainParent { String childName + + static constraints = { + grandparentName bindable: true + parentName bindable: true + childName bindable: true + } +} + +@DirtyCheck +abstract class AbstractDefaultDeniedBase { + String name +} + +@Entity +class DomainWithDefaultDeniedProperty extends AbstractDefaultDeniedBase { + String title + + static constraints = { + title bindable: true + } +} + +@Entity +class DomainWithLegacyBindableFalse { + String name + String title + + static constraints = { + title bindable: false + } } diff --git a/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectsSpec.groovy b/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectsSpec.groovy index 7194410726e..5ae89cacf9b 100644 --- a/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectsSpec.groovy +++ b/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/CommandObjectsSpec.groovy @@ -23,6 +23,7 @@ import grails.artefact.Artefact import grails.testing.gorm.DataTest import grails.testing.web.controllers.ControllerUnitTest import grails.validation.Validateable +import grails.web.databinding.BindAllowed import org.grails.validation.ConstraintEvalUtils import spock.lang.Issue import spock.lang.Specification @@ -382,10 +383,28 @@ class CommandObjectsSpec extends Specification implements ControllerUnitTest implements Validateable { String firstName G lastName + + static constraints = { + firstName bindable: true + lastName bindable: true + } } class ConcreteGenericBased extends WithGeneric { } + +class UserCommand implements Validateable { + String displayName + boolean admin + String role +} diff --git a/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/NonValidateableCommand.groovy b/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/NonValidateableCommand.groovy index 5d3c53e1226..11c73e42a65 100644 --- a/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/NonValidateableCommand.groovy +++ b/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/NonValidateableCommand.groovy @@ -20,4 +20,8 @@ package org.grails.web.commandobjects class NonValidateableCommand { String name + + static constraints = { + name bindable: true + } } diff --git a/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/SomeValidateableClass.groovy b/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/SomeValidateableClass.groovy index 4ca9947f2fb..01ab63a5f9a 100644 --- a/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/SomeValidateableClass.groovy +++ b/grails-test-suite-web/src/test/groovy/org/grails/web/commandobjects/SomeValidateableClass.groovy @@ -24,6 +24,6 @@ class SomeValidateableClass implements Validateable { String name static constraints = { - name matches: /[A-Z]*/ + name matches: /[A-Z]*/, bindable: true } } diff --git a/grails-test-suite-web/src/test/groovy/org/grails/web/servlet/BindDataMethodTests.groovy b/grails-test-suite-web/src/test/groovy/org/grails/web/servlet/BindDataMethodTests.groovy index d1b9fbfee64..85a56b6da39 100644 --- a/grails-test-suite-web/src/test/groovy/org/grails/web/servlet/BindDataMethodTests.groovy +++ b/grails-test-suite-web/src/test/groovy/org/grails/web/servlet/BindDataMethodTests.groovy @@ -20,6 +20,7 @@ package org.grails.web.servlet import grails.artefact.Artefact import grails.testing.web.controllers.ControllerUnitTest +import grails.web.databinding.DataBindingUtils import spock.lang.Specification /** @@ -57,14 +58,14 @@ class BindDataMethodTests extends Specification implements ControllerUnitTest CLASS_TO_BINDING_INCLUDE_LIST = new ConcurrentHashMap<>(); + private static final Map CLASS_TO_LEGACY_BINDING_INCLUDE_LIST = new ConcurrentHashMap<>(); /** * Associations both sides of any bidirectional relationships found in the object and source map to bind @@ -119,13 +120,19 @@ public static BindingResult bindObjectToInstance(Object object, Object source) { } protected static List getBindingIncludeList(final Object object) { - List includeList = Collections.emptyList(); + final boolean legacyBindableDefaultEnabled = isLegacyBindableDefaultEnabled(); + final String whiteListFieldName = legacyBindableDefaultEnabled ? + DefaultASTDatabindingHelper.LEGACY_DATABINDING_WHITELIST : + DefaultASTDatabindingHelper.DEFAULT_DATABINDING_WHITELIST; + final Map includeListCache = legacyBindableDefaultEnabled ? + CLASS_TO_LEGACY_BINDING_INCLUDE_LIST : CLASS_TO_BINDING_INCLUDE_LIST; + List includeList = legacyBindableDefaultEnabled ? Collections.emptyList() : Collections.singletonList(DefaultASTDatabindingHelper.NO_BINDABLE_PROPERTIES); try { final Class objectClass = object.getClass(); - if (CLASS_TO_BINDING_INCLUDE_LIST.containsKey(objectClass)) { - includeList = CLASS_TO_BINDING_INCLUDE_LIST.get(objectClass); + if (includeListCache.containsKey(objectClass)) { + includeList = includeListCache.get(objectClass); } else { - final Field whiteListField = objectClass.getDeclaredField(DefaultASTDatabindingHelper.DEFAULT_DATABINDING_WHITELIST); + final Field whiteListField = objectClass.getDeclaredField(whiteListFieldName); if (whiteListField != null) { if ((whiteListField.getModifiers() & Modifier.STATIC) != 0) { final Object whiteListValue = whiteListField.get(objectClass); @@ -135,7 +142,7 @@ protected static List getBindingIncludeList(final Object object) { } } if (!Environment.getCurrent().isReloadEnabled()) { - CLASS_TO_BINDING_INCLUDE_LIST.put(objectClass, includeList); + includeListCache.put(objectClass, includeList); } } } catch (Exception e) { @@ -143,6 +150,15 @@ protected static List getBindingIncludeList(final Object object) { return includeList; } + private static boolean isLegacyBindableDefaultEnabled() { + GrailsApplication application = Holders.findApplication(); + if (application != null) { + return application.getConfig().getProperty(DefaultASTDatabindingHelper.LEGACY_BINDABLE_DEFAULT, Boolean.class, false); + } + Object value = Holders.getFlatConfig().get(DefaultASTDatabindingHelper.LEGACY_BINDABLE_DEFAULT); + return Boolean.TRUE.equals(value) || "true".equalsIgnoreCase(String.valueOf(value)); + } + /** * Binds the given source object to the given target object performing type conversion if necessary * @@ -212,9 +228,12 @@ public static void bindToCollection(final Class targetType, final Collect * @return A BindingResult if there were errors or null if it was successful */ public static BindingResult bindObjectToInstance(Object object, Object source, List include, List exclude, String filter) { - if (include == null && exclude == null) { + if (include == null) { include = getBindingIncludeList(object); } + else if (include.isEmpty()) { + include = Collections.singletonList(DefaultASTDatabindingHelper.NO_BINDABLE_PROPERTIES); + } GrailsApplication application = Holders.findApplication(); PersistentEntity entity = null; if (application != null) { diff --git a/grails-web-databinding/src/main/groovy/org/grails/web/databinding/DefaultASTDatabindingHelper.java b/grails-web-databinding/src/main/groovy/org/grails/web/databinding/DefaultASTDatabindingHelper.java index 766f77d3c7b..6e5e2c725c9 100644 --- a/grails-web-databinding/src/main/groovy/org/grails/web/databinding/DefaultASTDatabindingHelper.java +++ b/grails-web-databinding/src/main/groovy/org/grails/web/databinding/DefaultASTDatabindingHelper.java @@ -50,10 +50,14 @@ public class DefaultASTDatabindingHelper implements ASTDatabindingHelper { public static final String BINDABLE_CONSTRAINT_NAME = "bindable"; public static final String DEFAULT_DATABINDING_WHITELIST = "$defaultDatabindingWhiteList"; + public static final String LEGACY_DATABINDING_WHITELIST = "$legacyDatabindingWhiteList"; public static final String NO_BINDABLE_PROPERTIES = "$_NO_BINDABLE_PROPERTIES_$"; + public static final String LEGACY_BINDABLE_DEFAULT = "grails.databinding.legacyBindableDefault"; private static Map> CLASS_NODE_TO_WHITE_LIST_PROPERTY_NAMES = new HashMap<>(); + private static Map> CLASS_NODE_TO_LEGACY_WHITE_LIST_PROPERTY_NAMES = new HashMap<>(); + private static Map> CLASS_NODE_TO_EXPLICITLY_BINDABLE_SPECIAL_PROPERTY_NAMES = new HashMap<>(); @SuppressWarnings("serial") @@ -90,12 +94,19 @@ public void injectDatabindingCode(final SourceUnit source, final GeneratorContex private void addDefaultDatabindingWhitelistField(final SourceUnit sourceUnit, final ClassNode classNode) { final FieldNode defaultWhitelistField = classNode.getDeclaredField(DEFAULT_DATABINDING_WHITELIST); - if (defaultWhitelistField != null) { - return; + if (defaultWhitelistField == null) { + final Set propertyNamesToIncludeInWhiteList = getPropertyNamesToIncludeInWhiteList(sourceUnit, classNode); + addWhitelistField(classNode, DEFAULT_DATABINDING_WHITELIST, propertyNamesToIncludeInWhiteList, true); } - final Set propertyNamesToIncludeInWhiteList = getPropertyNamesToIncludeInWhiteList(sourceUnit, classNode); + final FieldNode legacyWhitelistField = classNode.getDeclaredField(LEGACY_DATABINDING_WHITELIST); + if (legacyWhitelistField == null) { + final Set legacyPropertyNamesToIncludeInWhiteList = getLegacyPropertyNamesToIncludeInWhiteList(sourceUnit, classNode); + addWhitelistField(classNode, LEGACY_DATABINDING_WHITELIST, legacyPropertyNamesToIncludeInWhiteList, true); + } + } + private void addWhitelistField(final ClassNode classNode, final String fieldName, final Set propertyNamesToIncludeInWhiteList, final boolean denyWhenEmpty) { final ListExpression listExpression = new ListExpression(); if (propertyNamesToIncludeInWhiteList.size() > 0) { for (String propertyName : propertyNamesToIncludeInWhiteList) { @@ -114,11 +125,11 @@ private void addDefaultDatabindingWhitelistField(final SourceUnit sourceUnit, fi listExpression.addExpression(new ConstantExpression(propertyName + ".*")); } } - } else { + } else if (denyWhenEmpty) { listExpression.addExpression(new ConstantExpression(NO_BINDABLE_PROPERTIES)); } - classNode.addField(DEFAULT_DATABINDING_WHITELIST, + classNode.addField(fieldName, Modifier.STATIC | Modifier.PUBLIC | Modifier.FINAL, new ClassNode(List.class), listExpression); } @@ -222,6 +233,86 @@ private Set getPropertyNamesToIncludeInWhiteList(final SourceUnit source } } + propertyNamesToIncludeInWhiteList.addAll(bindablePropertyNames); + CLASS_NODE_TO_WHITE_LIST_PROPERTY_NAMES.put(classNode, propertyNamesToIncludeInWhiteList); + CLASS_NODE_TO_EXPLICITLY_BINDABLE_SPECIAL_PROPERTY_NAMES.put(classNode, explicitlyBindableSpecialPropertyNames); + return propertyNamesToIncludeInWhiteList; + } + + private Set getLegacyPropertyNamesToIncludeInWhiteListForParentClass(final SourceUnit sourceUnit, final ClassNode parentClassNode) { + final Set propertyNames; + if (CLASS_NODE_TO_LEGACY_WHITE_LIST_PROPERTY_NAMES.containsKey(parentClassNode)) { + propertyNames = CLASS_NODE_TO_LEGACY_WHITE_LIST_PROPERTY_NAMES.get(parentClassNode); + } else { + propertyNames = getLegacyPropertyNamesToIncludeInWhiteList(sourceUnit, parentClassNode); + } + return propertyNames; + } + + private Set getLegacyPropertyNamesToIncludeInWhiteList(final SourceUnit sourceUnit, final ClassNode classNode) { + final Set propertyNamesToIncludeInWhiteList = new HashSet<>(); + final Set unbindablePropertyNames = new HashSet<>(); + final Set bindablePropertyNames = new HashSet<>(); + + final Set explicitlyBindableSpecialPropertyNames = new HashSet<>(); + final boolean isDomainClass = GrailsASTUtils.isDomainClass(classNode, sourceUnit); + if (!classNode.getSuperClass().equals(new ClassNode(Object.class))) { + final Set parentClassPropertyNames = getLegacyPropertyNamesToIncludeInWhiteListForParentClass(sourceUnit, classNode.getSuperClass()); + final Set parentExplicitlyBindableSpecialPropertyNames = getExplicitlyBindableSpecialPropertyNamesForParentClass(sourceUnit, classNode.getSuperClass()); + explicitlyBindableSpecialPropertyNames.addAll(parentExplicitlyBindableSpecialPropertyNames); + for (final String parentPropertyName : parentClassPropertyNames) { + if (isDomainClass && DOMAIN_CLASS_PROPERTIES_TO_EXCLUDE_BY_DEFAULT.contains(parentPropertyName) && + !parentExplicitlyBindableSpecialPropertyNames.contains(parentPropertyName)) { + continue; + } + bindablePropertyNames.add(parentPropertyName); + } + } + + final FieldNode constraintsFieldNode = classNode.getDeclaredField(CONSTRAINTS_FIELD_NAME); + if (constraintsFieldNode != null && constraintsFieldNode.hasInitialExpression()) { + final Expression constraintsInitialExpression = constraintsFieldNode.getInitialExpression(); + if (constraintsInitialExpression instanceof ClosureExpression) { + + final Map> constraintsInfo = GrailsASTUtils.getConstraintMetadata((ClosureExpression) constraintsInitialExpression); + + for (Entry> constraintConfig : constraintsInfo.entrySet()) { + final String propertyName = constraintConfig.getKey(); + final Map mapEntryExpressions = constraintConfig.getValue(); + for (Entry entry : mapEntryExpressions.entrySet()) { + final String constraintName = entry.getKey(); + if (BINDABLE_CONSTRAINT_NAME.equals(constraintName)) { + final Expression valueExpression = entry.getValue(); + Boolean bindableValue = null; + if (valueExpression instanceof ConstantExpression) { + final Object constantValue = ((ConstantExpression) valueExpression).getValue(); + if (constantValue instanceof Boolean) { + bindableValue = (Boolean) constantValue; + } + } + if (bindableValue != null) { + if (Boolean.TRUE.equals(bindableValue)) { + unbindablePropertyNames.remove(propertyName); + bindablePropertyNames.add(propertyName); + if (DOMAIN_CLASS_PROPERTIES_TO_EXCLUDE_BY_DEFAULT.contains(propertyName)) { + explicitlyBindableSpecialPropertyNames.add(propertyName); + } + } else { + bindablePropertyNames.remove(propertyName); + unbindablePropertyNames.add(propertyName); + explicitlyBindableSpecialPropertyNames.remove(propertyName); + } + } else { + GrailsASTUtils.warning(sourceUnit, valueExpression, "The bindable constraint for property [" + + propertyName + "] in class [" + classNode.getName() + + "] has a value which is not a boolean literal and will be ignored."); + } + } + } + } + } + } + final Set fieldsInTransientsList = getPropertyNamesExpressedInTransientsList(classNode); propertyNamesToIncludeInWhiteList.addAll(bindablePropertyNames); @@ -248,7 +339,7 @@ private Set getPropertyNamesToIncludeInWhiteList(final SourceUnit source final String restOfMethodName = methodName.substring(3); final String propertyName = GrailsNameUtils.getPropertyName(restOfMethodName); if (!unbindablePropertyNames.contains(propertyName) && - (!isDomainClass || !DOMAIN_CLASS_PROPERTIES_TO_EXCLUDE_BY_DEFAULT.contains(propertyName))) { + (!isDomainClass || !DOMAIN_CLASS_PROPERTIES_TO_EXCLUDE_BY_DEFAULT.contains(propertyName))) { propertyNamesToIncludeInWhiteList.add(propertyName); } } @@ -256,8 +347,7 @@ private Set getPropertyNamesToIncludeInWhiteList(final SourceUnit source } } } - CLASS_NODE_TO_WHITE_LIST_PROPERTY_NAMES.put(classNode, propertyNamesToIncludeInWhiteList); - CLASS_NODE_TO_EXPLICITLY_BINDABLE_SPECIAL_PROPERTY_NAMES.put(classNode, explicitlyBindableSpecialPropertyNames); + CLASS_NODE_TO_LEGACY_WHITE_LIST_PROPERTY_NAMES.put(classNode, propertyNamesToIncludeInWhiteList); Map allAssociationMap = GrailsASTUtils.getAllAssociationMap(classNode); for (String associationName : allAssociationMap.keySet()) { if (!propertyNamesToIncludeInWhiteList.contains(associationName) && !unbindablePropertyNames.contains(associationName)) { @@ -305,4 +395,5 @@ private Set getPropertyNamesExpressedInTransientsList(final ClassNode cl } return transientFields; } + }