diff --git a/grails-test-examples/gorm/build.gradle b/grails-test-examples/gorm/build.gradle
index 5fc38241f3a..811c2ba9145 100644
--- a/grails-test-examples/gorm/build.gradle
+++ b/grails-test-examples/gorm/build.gradle
@@ -61,6 +61,7 @@ dependencies {
console 'org.apache.grails:grails-console'
integrationTestImplementation testFixtures('org.apache.grails:grails-geb')
+ integrationTestImplementation project(':grails-testing-support-http-client')
}
apply {
diff --git a/grails-test-examples/gorm/grails-app/controllers/gorm/DirtyCheckBindingController.groovy b/grails-test-examples/gorm/grails-app/controllers/gorm/DirtyCheckBindingController.groovy
new file mode 100644
index 00000000000..b833a67665c
--- /dev/null
+++ b/grails-test-examples/gorm/grails-app/controllers/gorm/DirtyCheckBindingController.groovy
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package gorm
+
+/**
+ * Controller used by the functional test for issue 15681. It binds the incoming request parameters to a new
+ * {@link DirtyCheckedRecord} and renders the resulting {@code id}, {@code version} and {@code description} so the
+ * test can assert over HTTP that {@code id}/{@code version} were not bound by default.
+ */
+class DirtyCheckBindingController {
+
+ def bind() {
+ def record = new DirtyCheckedRecord()
+ bindData(record, params)
+ render "id=${record.id}|version=${record.version}|description=${record.description}"
+ }
+}
diff --git a/grails-test-examples/gorm/grails-app/domain/gorm/DirtyCheckedRecord.groovy b/grails-test-examples/gorm/grails-app/domain/gorm/DirtyCheckedRecord.groovy
new file mode 100644
index 00000000000..510415902e2
--- /dev/null
+++ b/grails-test-examples/gorm/grails-app/domain/gorm/DirtyCheckedRecord.groovy
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package gorm
+
+/**
+ * Concrete GORM domain class (located in {@code grails-app/domain}) that extends an abstract
+ * {@code @DirtyCheck} base defined in {@code src/main/groovy}. Data binding must never bind
+ * {@code id} or {@code version} on this class by default. See issue 15681.
+ */
+class DirtyCheckedRecord extends AbstractDirtyCheckedRecord {
+
+ static constraints = {
+ description nullable: true
+ }
+}
diff --git a/grails-test-examples/gorm/src/integration-test/groovy/gorm/DirtyCheckBindingSpec.groovy b/grails-test-examples/gorm/src/integration-test/groovy/gorm/DirtyCheckBindingSpec.groovy
new file mode 100644
index 00000000000..df75978a9ee
--- /dev/null
+++ b/grails-test-examples/gorm/src/integration-test/groovy/gorm/DirtyCheckBindingSpec.groovy
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package gorm
+
+import grails.testing.mixin.integration.Integration
+import spock.lang.Issue
+import spock.lang.Specification
+import spock.lang.Tag
+
+import org.apache.grails.testing.http.client.HttpClientSupport
+
+/**
+ * Functional test reproducing issue 15681 end-to-end: a real Grails application binds request parameters
+ * (including {@code id} and {@code version}) to a domain class that extends an abstract {@code @DirtyCheck}
+ * base. The framework must not bind {@code id} or {@code version} by default.
+ */
+@Integration(applicationClass = Application)
+@Tag('http-client')
+class DirtyCheckBindingSpec extends Specification implements HttpClientSupport {
+
+ private static final String FORM = 'application/x-www-form-urlencoded'
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'bindData over HTTP does not bind id or version on a domain extending a @DirtyCheck base'() {
+ when: 'a form submission posts id, version and a regular property'
+ def response = httpPost('/dirtyCheckBinding/bind', 'id=99&version=5&description=Opening+balance', FORM)
+
+ then: 'the request succeeds'
+ response.assertStatus(200)
+
+ and: 'the regular property is bound'
+ response.assertContains('description=Opening balance')
+
+ and: 'id and version are not bound, matching the documented default behaviour'
+ response.assertContains('id=null')
+ response.assertContains('version=null')
+ }
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'binding only a regular property over HTTP leaves id and version null'() {
+ when:
+ def response = httpPost('/dirtyCheckBinding/bind', 'description=Closing+balance', FORM)
+
+ then:
+ response.assertStatus(200)
+ response.assertContains('description=Closing balance')
+ response.assertContains('id=null')
+ response.assertContains('version=null')
+ }
+}
diff --git a/grails-test-examples/gorm/src/main/groovy/gorm/AbstractDirtyCheckedRecord.groovy b/grails-test-examples/gorm/src/main/groovy/gorm/AbstractDirtyCheckedRecord.groovy
new file mode 100644
index 00000000000..7fd6b6388e8
--- /dev/null
+++ b/grails-test-examples/gorm/src/main/groovy/gorm/AbstractDirtyCheckedRecord.groovy
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package gorm
+
+import grails.gorm.dirty.checking.DirtyCheck
+import groovy.transform.CompileStatic
+
+/**
+ * Abstract base class living in {@code src/main/groovy} (i.e. not itself a domain class) annotated with
+ * {@link DirtyCheck}. GORM injects {@code id} and {@code version} onto the furthest unresolved parent in the
+ * hierarchy, which is this class. This reproduces the conditions of
+ * issue 15681 where {@code id} and
+ * {@code version} would incorrectly leak into the data binding whitelist of the concrete domain subclass.
+ */
+@DirtyCheck
+@CompileStatic
+abstract class AbstractDirtyCheckedRecord {
+ String description
+}
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 119fc9bcb0b..5d58896cf6d 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
@@ -18,7 +18,9 @@
*/
package org.grails.web.binding
+import grails.gorm.dirty.checking.DirtyCheck
import grails.persistence.Entity
+import groovy.transform.CompileStatic
import org.grails.validation.ConstraintEvalUtils
import spock.lang.Issue
import spock.lang.Specification
@@ -55,6 +57,80 @@ class DefaultASTDatabindingHelperDomainClassSpecialPropertiesSpec extends
obj.dateCreated == now
obj.lastUpdated == now
}
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'Test id and version are not bound when a domain class extends a @DirtyCheck abstract base class'() {
+ 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'
+ obj.name == 'Grace'
+ obj.id == null
+ obj.version == null
+ }
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'Test dateCreated and lastUpdated declared on a @DirtyCheck abstract base are not bound in the domain subclass'() {
+ when: 'a domain subclass inheriting timestamp properties from a @DirtyCheck base is bound'
+ 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'
+ obj.name == 'Alan'
+ obj.title == 'Mathematician'
+
+ and: 'the default-excluded special properties are not bound regardless of where they are declared'
+ obj.id == null
+ obj.version == null
+ obj.dateCreated == null
+ obj.lastUpdated == null
+ }
+
+ @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'() {
+ 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'
+ obj.name == 'Grace'
+ obj.title == 'Admiral'
+ obj.id == null
+ obj.version == null
+ }
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'Test explicit bindable:true on inherited special properties re-enables binding without affecting id and version'() {
+ when: 'a domain subclass declares an explicit bindable:true constraint for inherited timestamp properties'
+ def now = new Date()
+ def obj = new DomainWithInheritedBindableTimestamps(id: 5L, version: 3L, name: 'Edsger', title: 'Scientist', dateCreated: now, lastUpdated: now)
+
+ then: 'the explicitly bindable timestamps are bound'
+ obj.dateCreated == now
+ obj.lastUpdated == now
+
+ and: 'regular properties are bound'
+ obj.name == 'Edsger'
+ obj.title == 'Scientist'
+
+ and: 'id and version remain excluded as there is no explicit override for them'
+ obj.id == null
+ obj.version == null
+ }
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'Test id and version are not bound across multiple levels of abstract inheritance'() {
+ when: 'a domain class extends a plain abstract class which extends a @DirtyCheck abstract base'
+ def obj = new DomainExtendingMultiLevelHierarchy(id: 42L, version: 9L, grandparentName: 'g', parentName: 'p', childName: 'c')
+
+ then: 'all regular properties throughout the hierarchy are bound'
+ obj.grandparentName == 'g'
+ obj.parentName == 'p'
+ obj.childName == 'c'
+
+ and: 'id and version are not bound even though they are injected several levels up the hierarchy'
+ obj.id == null
+ obj.version == null
+ }
}
@Entity
@@ -73,3 +149,55 @@ class SomeDomainClassWithExplicitBindableRules {
lastUpdated bindable: true
}
}
+
+@DirtyCheck
+@CompileStatic
+abstract class AbstractDirtyCheckedBase {
+ String name
+}
+
+@Entity
+class MyDirtyCheckedDomain extends AbstractDirtyCheckedBase {
+ static constraints = {
+ name nullable: false
+ }
+}
+
+@DirtyCheck
+@CompileStatic
+abstract class AbstractDirtyCheckedBaseWithTimestamps {
+ String name
+ Date dateCreated
+ Date lastUpdated
+}
+
+@Entity
+class DomainWithInheritedTimestamps extends AbstractDirtyCheckedBaseWithTimestamps {
+ String title
+}
+
+@Entity
+class DomainWithInheritedBindableTimestamps extends AbstractDirtyCheckedBaseWithTimestamps {
+ String title
+
+ static constraints = {
+ dateCreated bindable: true
+ lastUpdated bindable: true
+ }
+}
+
+@DirtyCheck
+@CompileStatic
+abstract class AbstractDirtyCheckedGrandparent {
+ String grandparentName
+}
+
+@CompileStatic
+abstract class AbstractPlainParent extends AbstractDirtyCheckedGrandparent {
+ String parentName
+}
+
+@Entity
+class DomainExtendingMultiLevelHierarchy extends AbstractPlainParent {
+ String childName
+}
diff --git a/grails-test-suite-web/src/test/groovy/org/grails/web/binding/DirtyCheckBaseBindDataSpec.groovy b/grails-test-suite-web/src/test/groovy/org/grails/web/binding/DirtyCheckBaseBindDataSpec.groovy
new file mode 100644
index 00000000000..bedc5493ab5
--- /dev/null
+++ b/grails-test-suite-web/src/test/groovy/org/grails/web/binding/DirtyCheckBaseBindDataSpec.groovy
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.grails.web.binding
+
+import grails.artefact.Artefact
+import grails.gorm.dirty.checking.DirtyCheck
+import grails.persistence.Entity
+import grails.testing.web.controllers.ControllerUnitTest
+import groovy.transform.CompileStatic
+import spock.lang.Issue
+import spock.lang.Specification
+
+/**
+ * Reproduces the controller {@code bindData(domain, params)} scenario from the bug report: a domain class
+ * that extends an abstract {@code @DirtyCheck} base class must never bind {@code id} or {@code version} by default.
+ */
+class DirtyCheckBaseBindDataSpec extends Specification implements ControllerUnitTest {
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'Test bindData does not bind id or version when the domain extends a @DirtyCheck base'() {
+ given: 'request params that include id and version alongside a regular property'
+ params.id = '99'
+ params.version = '5'
+ params.description = 'Opening balance'
+
+ when: 'the controller binds the params to a new domain instance'
+ def model = controller.create()
+ def entry = model.entry
+
+ then: 'the regular property is bound'
+ entry.description == 'Opening balance'
+
+ and: 'id and version are not bound, matching the documented default behaviour'
+ entry.id == null
+ entry.version == null
+ }
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'Test the documented exclude workaround continues to prevent id binding'() {
+ given:
+ params.id = '99'
+ params.description = 'Closing balance'
+
+ when:
+ def model = controller.createWithExcludeWorkaround()
+ def entry = model.entry
+
+ then:
+ entry.description == 'Closing balance'
+ entry.id == null
+ }
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'Test an explicit include list binds only the listed property and never id'() {
+ given:
+ params.id = '99'
+ params.description = 'Adjustment'
+
+ when:
+ def model = controller.createWithIncludes()
+ def entry = model.entry
+
+ then: 'only the included property is bound'
+ entry.description == 'Adjustment'
+
+ and: 'id is still not bound'
+ entry.id == null
+ }
+
+ @Issue('https://github.com/apache/grails-core/issues/15681')
+ void 'Test binding a Map directly through bindData does not bind id'() {
+ when: 'a plain Map (rather than the request params) is bound'
+ def model = controller.createFromMap()
+ def entry = model.entry
+
+ then:
+ entry.description == 'From map'
+ entry.id == null
+ entry.version == null
+ }
+}
+
+@DirtyCheck
+@CompileStatic
+abstract class AbstractAuditableLedgerBase {
+ String description
+}
+
+@Entity
+class DirtyCheckLedgerEntry extends AbstractAuditableLedgerBase {
+ static constraints = {
+ description nullable: true
+ }
+}
+
+@Artefact('Controller')
+class DirtyCheckLedgerController {
+
+ def create() {
+ def entry = new DirtyCheckLedgerEntry()
+ bindData(entry, params)
+ [entry: entry]
+ }
+
+ def createWithExcludeWorkaround() {
+ def entry = new DirtyCheckLedgerEntry()
+ bindData(entry, params, [exclude: ['id']])
+ [entry: entry]
+ }
+
+ def createWithIncludes() {
+ def entry = new DirtyCheckLedgerEntry()
+ bindData(entry, params, [include: ['description']])
+ [entry: entry]
+ }
+
+ def createFromMap() {
+ def entry = new DirtyCheckLedgerEntry()
+ bindData(entry, [id: '99', version: '5', description: 'From map'])
+ [entry: entry]
+ }
+}
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 e9852d0d852..359fea50d22 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
@@ -145,9 +145,20 @@ private Set getPropertyNamesToIncludeInWhiteList(final SourceUnit source
final Set propertyNamesToIncludeInWhiteList = new HashSet<>();
final Set unbindablePropertyNames = new HashSet<>();
final Set bindablePropertyNames = new HashSet<>();
+ final boolean isDomainClass = GrailsASTUtils.isDomainClass(classNode, sourceUnit);
if (!classNode.getSuperClass().equals(new ClassNode(Object.class))) {
final Set parentClassPropertyNames = getPropertyNamesToIncludeInWhiteListForParentClass(sourceUnit, classNode.getSuperClass());
- bindablePropertyNames.addAll(parentClassPropertyNames);
+ for (final String parentPropertyName : parentClassPropertyNames) {
+ // The id, version, dateCreated and lastUpdated properties of a domain class are never bound by default.
+ // A parent class may legitimately include these in its own whitelist when it is not itself recognised as
+ // a domain class (for example an abstract @DirtyCheck base class in src/main/groovy onto which GORM injects
+ // id and version). Such properties must not be inherited into a domain class' whitelist, otherwise the
+ // exclusion performed by shouldFieldBeInWhiteList is bypassed via the inherited bindable property names.
+ if (isDomainClass && DOMAIN_CLASS_PROPERTIES_TO_EXCLUDE_BY_DEFAULT.contains(parentPropertyName)) {
+ continue;
+ }
+ bindablePropertyNames.add(parentPropertyName);
+ }
}
final FieldNode constraintsFieldNode = classNode.getDeclaredField(CONSTRAINTS_FIELD_NAME);
@@ -191,7 +202,6 @@ private Set getPropertyNamesToIncludeInWhiteList(final SourceUnit source
}
final Set fieldsInTransientsList = getPropertyNamesExpressedInTransientsList(classNode);
- final boolean isDomainClass = GrailsASTUtils.isDomainClass(classNode, sourceUnit);
propertyNamesToIncludeInWhiteList.addAll(bindablePropertyNames);
final List fields = classNode.getFields();