This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch chore/cleanup-AbstractCriteriaBuilder in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit a8f61b81a297ca8ebfa3c0b1c55aac0e80948e31 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Wed Aug 12 18:50:33 2026 -0500 Fix infinite recursion in CriteriaBuilder.scroll() on malformed calls scroll(Closure c) unconditionally re-entered invokeMethod(SCROLL_CALL, new Object[]{c}) to reuse the shared criteria-construction logic. When scroll() is called with the wrong arg count/type (e.g. bare scroll(), or a call whose argument gets null-coerced by Groovy's meta-method matching), isCriteriaConstructionMethod() correctly rejects it, but invokeMethod then falls through to its getMetaClass() meta-method lookup, which is willing to match scroll(Closure) with a null argument and invoke it - re-entering scroll() with the same null argument forever, and StackOverflowing. Extracted the shared criteria-construction logic (evaluate the closure, run the query, reset state) into a new executeCriteriaConstruction() method, and changed scroll() to call it directly instead of bouncing back through the dynamic dispatch in invokeMethod. This closes the recursion path entirely: a malformed scroll() call now just runs the query with no criteria applied, matching how an empty/null closure already behaves everywhere else in this class, instead of crashing. Found and root-caused while extending the SCROLL_CALL branch's test coverage for PR #16140; CriteriaBuilder.java is already one of that PR's changed files even though this particular method wasn't touched by its diff. Co-Authored-By: Claude Sonnet 5 <[email protected]> --- .../main/groovy/grails/gorm/CriteriaBuilder.java | 2 +- .../query/criteria/AbstractCriteriaBuilder.java | 44 +++++++++++++++------- .../groovy/grails/gorm/CriteriaBuilderSpec.groovy | 25 +++++++++--- 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/grails-datamapping-core/src/main/groovy/grails/gorm/CriteriaBuilder.java b/grails-datamapping-core/src/main/groovy/grails/gorm/CriteriaBuilder.java index c342f51391..f12f80410d 100644 --- a/grails-datamapping-core/src/main/groovy/grails/gorm/CriteriaBuilder.java +++ b/grails-datamapping-core/src/main/groovy/grails/gorm/CriteriaBuilder.java @@ -167,6 +167,6 @@ public class CriteriaBuilder<T> extends AbstractCriteriaBuilder implements Build @Override public Object scroll(@DelegatesTo(Criteria.class) Closure c) { - return invokeMethod(SCROLL_CALL, new Object[]{c}); + return executeCriteriaConstruction(c); } } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/criteria/AbstractCriteriaBuilder.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/criteria/AbstractCriteriaBuilder.java index 69cbbc12ad..3d9c6fbaec 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/criteria/AbstractCriteriaBuilder.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/criteria/AbstractCriteriaBuilder.java @@ -279,20 +279,7 @@ public abstract class AbstractCriteriaBuilder extends GroovyObjectSupport implem ensureQueryIsInitialized(); if (isCriteriaConstructionMethod(name, args)) { - - uniqueResult = false; - - invokeClosureNode(args[0]); - - Object result; - if (!uniqueResult) { - result = invokeList(); - } - else { - result = query.singleResult(); - } - query = null; - return result; + return executeCriteriaConstruction(args[0]); } Object result = invokeMetaMethod(getMetaClass(), this, name, args); @@ -337,6 +324,35 @@ public abstract class AbstractCriteriaBuilder extends GroovyObjectSupport implem throw new MissingMethodException(name, getClass(), args); } + /** + * Evaluates a criteria-construction closure (the body of {@code call}/{@code doCall}/ + * {@code scroll}) and executes the resulting query. Callers that already know they are + * performing a criteria construction call - such as {@link #scroll(Closure)} - should call + * this directly rather than re-entering {@link #invokeMethod(String, Object)}: routing back + * through the dynamic dispatch there is not just redundant, it is unsafe, since a call with + * a mismatched/null argument can resolve back to the very method that made it, recursing + * indefinitely. + * + * @param callable The criteria-construction closure; ignored if not a {@link Closure} + * @return The query results, or a single result if {@link #uniqueResult} is set + */ + protected Object executeCriteriaConstruction(Object callable) { + ensureQueryIsInitialized(); + uniqueResult = false; + + invokeClosureNode(callable); + + Object result; + if (!uniqueResult) { + result = invokeList(); + } + else { + result = query.singleResult(); + } + query = null; + return result; + } + private Object invokeMetaMethod(MetaObjectProtocol metaClass, Object target, String name, Object[] args) { MetaMethod metaMethod = metaClass.getMetaMethod(name, args); return metaMethod != null ? metaMethod.invoke(target, args) : NOT_FOUND; diff --git a/grails-datamapping-core/src/test/groovy/grails/gorm/CriteriaBuilderSpec.groovy b/grails-datamapping-core/src/test/groovy/grails/gorm/CriteriaBuilderSpec.groovy index 228aa6cafe..019c72949b 100644 --- a/grails-datamapping-core/src/test/groovy/grails/gorm/CriteriaBuilderSpec.groovy +++ b/grails-datamapping-core/src/test/groovy/grails/gorm/CriteriaBuilderSpec.groovy @@ -1014,11 +1014,6 @@ class CriteriaBuilderSpec extends Specification { def criteria = newBuilder() when: - // Deliberately not scroll(): CriteriaBuilder.scroll(Closure) unconditionally re-enters - // invokeMethod with its (possibly null) argument, and Groovy's meta-method lookup keeps - // matching a single-null-arg call back to that same method - an infinite recursion this - // test must not trip. Two non-null args can never match that one-arg method, so this - // safely misses it and exercises the "wrong arg count" branch without the loop. criteria.scroll('not-a-closure', 'extra') then: @@ -1036,6 +1031,26 @@ class CriteriaBuilderSpec extends Specification { thrown(MissingMethodException) } + void "scroll with no arguments executes the query with no criteria applied, without recursing"() { + // Regression test: scroll(Closure) used to call invokeMethod(SCROLL_CALL, [c]) to run its + // shared construction logic. A zero-arg call resolves - via Groovy's meta-method lookup, + // which is willing to match a missing Closure argument as null - back to that very method + // with a null argument, which again called invokeMethod, which again resolved back to + // scroll(null): unbounded recursion and a StackOverflowError. scroll() now executes the + // shared logic directly instead of re-entering the dynamic dispatch, so a null/absent + // closure is just a no-op criteria and the call completes normally. + given: + def criteria = newBuilder() + query.list() >> ['a'] + + when: + def result = criteria.scroll() + + then: + result == ['a'] + 0 * query.add(_) + } + void "call with a non-closure argument executes the query without evaluating any closure"() { given: def criteria = newBuilder()
