This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch 8.0.x-hibernate7 in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 1df637570e8cc9b5be2e9f88379ff2f1985eac61 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Tue Feb 24 14:29:41 2026 -0600 update tests --- grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md | 2 +- grails-data-hibernate7/STATUS.md | 97 +++++++ .../hibernate/query/GrailsHibernateQueryUtils.java | 10 +- .../orm/hibernate/query/HqlQueryContextSpec.groovy | 317 +++++++++++++++++++++ 4 files changed, 420 insertions(+), 6 deletions(-) diff --git a/grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md b/grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md index a0ec9672b1..d2a89e16af 100644 --- a/grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md +++ b/grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md @@ -1,6 +1,6 @@ # GORM Query Safety Audit — `grails-data-hibernate7` -> **Scope**: Section 2.6 of `HIBERNATE7-GRAILS8-UPGRADE.md` +> **Scope**: Section 2.6 of `UPGRADE.md` > **Priority**: P2 — SQL injection is OWASP Top-10 #3 > **Module**: `grails-data-hibernate7-core` > **Last updated**: 2026-02-23 diff --git a/grails-data-hibernate7/STATUS.md b/grails-data-hibernate7/STATUS.md new file mode 100644 index 0000000000..e9b43be0f5 --- /dev/null +++ b/grails-data-hibernate7/STATUS.md @@ -0,0 +1,97 @@ +<!-- +SPDX-License-Identifier: Apache-2.0 + +Licensed 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. +--> + +# `grails-data-hibernate7` — Goals Status + +> Goals defined in [`UPGRADE.md`](UPGRADE.md). Last updated: 2026-02-24. + +--- + +## Section 0.3 — Hibernate Dual-Version Support + +| Goal | Status | Notes | +|------|--------|-------| +| `grails-data-hibernate6` removed | ✅ Done | Only `hibernate5` and `hibernate7` in `settings.gradle` | +| Fully decomposed `domainbinding/binder/` hierarchy | ✅ Done | 20+ binders: `ClassBinder`, `ComponentBinder`, `ManyToOneBinder`, `CollectionBinder`, etc. | +| `HibernateCriteriaBuilder` with `CriteriaMethodInvoker`/`CriteriaMethods` | ✅ Done | Refactored; direct-call specs written for JaCoCo visibility | +| `HibernateHqlQuery`/`HqlQueryContext` split | ✅ Done | `HqlQueryContext` extracted into its own class | +| Each binder has its own Spock spec | ✅ Done | 240 test specs, 1609 tests, 0 failures | +| Hibernate 7 functional test examples | ✅ Done | 10 example projects (grails-hibernate, multi-datasource, multi-tenancy, data-service, etc.) | +| Apply binder refactoring to `grails-data-hibernate5` | ❌ Not done | `grails-data-hibernate5` still uses monolithic `GrailsDomainBinder` | + +--- + +## Section 1.4 — Static Compilation (Target: 90%+) + +| Metric | Status | Notes | +|--------|--------|-------| +| Core module Groovy files with `@CompileStatic` | ✅ 94% (62/66) | | +| Missing 4 files | ✅ Acceptable | All in `dbmigration/` submodule (scripts + plugin descriptor) | +| JaCoCo branch coverage | ⚠️ 66% branches | No enforced minimum; gaps are in `transaction` and `datasource` packages (0%) and dynamic dispatch paths in `grails.orm` | + +--- + +## Section 2.6 — GORM Query Safety Audit + +| Item | Status | Notes | +|------|--------|-------| +| `DefaultSchemaHandler` schema name DDL injection | ✅ Fixed | `quoteName()` with JDBC identifier quoting; 9 tests | +| `find/findAll/executeQuery/executeUpdate(CharSequence)` plain `String` guard | ✅ Fixed | `requireGString()` throws `UnsupportedOperationException`; 43 tests | +| `findWithSql`/`findAllWithSql` renamed to `findWithNativeSql`/`findAllWithNativeSql` | ✅ Fixed | Old names `@Deprecated` and delegating | +| `nextId()` dead code removed | ✅ Fixed | No callers; left over from Hibernate 5 | +| Safe query patterns documented | ✅ Done | `hql.adoc`, `nativeSql.adoc`, `upgradeNotes.adoc` | +| Compile-time GString warning (AST transform) | ✅ Superseded | Runtime exception at API boundary is stronger than a compile-time warning | + +Full detail: [`GORM-QUERY-SAFETY-AUDIT.md`](GORM-QUERY-SAFETY-AUDIT.md) + +--- + +## Section 4.1 — ExpandoMetaClass Elimination + +| Item | Status | Notes | +|------|--------|-------| +| `HibernateMappingBuilder.methodMissing` DSL replacement | ⚠️ Not started | Explicitly scoped to **8.1+** in `UPGRADE.md` — expected | +| `HibernateGormStaticApi.propertyMissing` | ⚠️ Present | Named criteria proxy dispatch; also 8.1+ scope | + +--- + +## Test Coverage + +| Package | Coverage | Status | +|---------|----------|--------| +| `domainbinding/binder` | ~94% instructions | ✅ | +| `domainbinding/util` | ~93% instructions | ✅ | +| `compiler` | ~93% instructions | ✅ | +| `query` | ~84% instructions | ✅ | +| `grails.orm` | ~79% instructions | ⚠️ | +| `org.grails.orm.hibernate` | ~74% instructions / ~54% branches | ⚠️ | +| `transaction` | 0% | ❌ No tests at all | +| `datasource` | 0% | ❌ No tests at all | +| **Overall** | **80% instructions / 66% branches** | ⚠️ | + +--- + +## Summary + +| Tier | Item | Status | +|------|------|--------| +| 0.3 | `hibernate6` removal | ✅ Done | +| 0.3 | Binder decomposition (`hibernate7`) | ✅ Done | +| 0.3 | Binder refactoring applied to `hibernate5` | ❌ Not done | +| 1.4 | Static compilation — core module | ✅ 94% | +| 2.6 | GORM query safety audit | ✅ Done | +| 4.1 | `methodMissing` elimination | ⚠️ 8.1+ scope | +| — | `transaction`/`datasource` test coverage | ❌ 0% | diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/GrailsHibernateQueryUtils.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/GrailsHibernateQueryUtils.java index d90eda0ddc..ed7d931c33 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/GrailsHibernateQueryUtils.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/GrailsHibernateQueryUtils.java @@ -183,7 +183,7 @@ public class GrailsHibernateQueryUtils { * Add order to criteria, creating necessary subCriteria if nested sort property (ie. * sort:'nested.property'). */ - private static void addOrderPossiblyNested( + protected static void addOrderPossiblyNested( CriteriaQuery query, From queryRoot, CriteriaBuilder criteriaBuilder, @@ -224,7 +224,7 @@ public class GrailsHibernateQueryUtils { } } - private static boolean isIgnoreCaseProperty( + protected static boolean isIgnoreCaseProperty( boolean ignoreCase, PersistentProperty<? extends Property> persistentProperty) { if (ignoreCase && persistentProperty != null && persistentProperty.getType() != String.class) { ignoreCase = false; @@ -233,7 +233,7 @@ public class GrailsHibernateQueryUtils { } /** Add order directly to criteria. */ - private static void addOrder( + protected static void addOrder( PersistentEntity entity, CriteriaQuery query, From queryRoot, @@ -322,14 +322,14 @@ public class GrailsHibernateQueryUtils { * @param targetClass The target class * @param criteria The criteria */ - private static void cacheCriteriaByMapping(Class<?> targetClass, Query criteria) { + protected static void cacheCriteriaByMapping(Class<?> targetClass, Query criteria) { Mapping m = MappingCacheHolder.getInstance().getMapping(targetClass); if (m != null && m.getCache() != null && m.getCache().getEnabled()) { criteria.setCacheable(true); } } - private static FlushMode convertFlushMode(Object object) { + protected static FlushMode convertFlushMode(Object object) { if (object == null) { return null; } diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/query/HqlQueryContextSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/query/HqlQueryContextSpec.groovy new file mode 100644 index 0000000000..b80fc6a085 --- /dev/null +++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/query/HqlQueryContextSpec.groovy @@ -0,0 +1,317 @@ +/* + * 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.orm.hibernate.query + +import org.grails.datastore.mapping.model.PersistentEntity +import spock.lang.Specification +import spock.lang.Unroll + +class HqlQueryContextSpec extends Specification { + + // ─── Record construction ────────────────────────────────────────────────── + + void "record accessors return constructor values"() { + given: + def params = [name: "Alice"] + def ctx = new HqlQueryContext("from Person", String, params, false, false) + + expect: + ctx.hql() == "from Person" + ctx.targetClass() == String + ctx.namedParams() == [name: "Alice"] + !ctx.isUpdate() + !ctx.isNative() + } + + void "record isUpdate and isNative flags are set correctly"() { + given: + def ctx = new HqlQueryContext("update Foo set x=1", Object, [:], true, true) + + expect: + ctx.isUpdate() + ctx.isNative() + } + + // ─── prepare ───────────────────────────────────────────────────────────── + + void "prepare with plain String produces correct hql and empty namedParams"() { + given: + def entity = Mock(PersistentEntity) { getJavaClass() >> String } + + when: + def ctx = HqlQueryContext.prepare("from Foo", false, false, null, entity) + + then: + ctx.hql() == "from Foo" + ctx.namedParams().isEmpty() + ctx.targetClass() == String + !ctx.isUpdate() + !ctx.isNative() + } + + void "prepare with GString expands interpolations into named parameters"() { + given: + def entity = Mock(PersistentEntity) { getJavaClass() >> String } + String val = "bar" + GString gq = "from Foo where name = ${val}" + + when: + def ctx = HqlQueryContext.prepare(gq, false, false, null, entity) + + then: + ctx.hql() == "from Foo where name = :p0" + ctx.namedParams() == [p0: "bar"] + } + + void "prepare merges caller-supplied namedParams with GString params"() { + given: + def entity = Mock(PersistentEntity) { getJavaClass() >> String } + String val = "bar" + GString gq = "from Foo where name = ${val} and status = :status" + + when: + def ctx = HqlQueryContext.prepare(gq, false, false, [status: "active"], entity) + + then: + ctx.namedParams().p0 == "bar" + ctx.namedParams().status == "active" + } + + void "prepare with isNative=true preserves hql without alias injection"() { + given: + def entity = Mock(PersistentEntity) { getJavaClass() >> String } + + when: + def ctx = HqlQueryContext.prepare("select name from foo", true, false, null, entity) + + then: + ctx.hql() == "select name from foo" // alias injection skipped for native SQL + ctx.isNative() + } + + void "prepare with isUpdate=true sets flag"() { + given: + def entity = Mock(PersistentEntity) { getJavaClass() >> String } + + when: + def ctx = HqlQueryContext.prepare("update Foo set x=1", false, true, null, entity) + + then: + ctx.isUpdate() + } + + void "prepare with null namedParams does not throw"() { + given: + def entity = Mock(PersistentEntity) { getJavaClass() >> String } + + when: + def ctx = HqlQueryContext.prepare("from Foo", false, false, null, entity) + + then: + noExceptionThrown() + ctx.namedParams() != null + } + + // ─── resolveHql ────────────────────────────────────────────────────────── + + void "resolveHql with null returns empty string"() { + expect: + HqlQueryContext.resolveHql(null, false, [:]) == "" + } + + void "resolveHql with plain String passes through normalisation"() { + expect: + HqlQueryContext.resolveHql("from Foo", false, [:]) == "from Foo" + } + + void "resolveHql with GString extracts parameters and returns parameterised HQL"() { + given: + String v = "hello" + GString gq = "from Foo where x = ${v}" + def params = [:] + + when: + String result = HqlQueryContext.resolveHql(gq, false, params) + + then: + result == "from Foo where x = :p0" + params.p0 == "hello" + } + + void "resolveHql with multi-value GString produces sequential parameter names"() { + given: + String a = "foo" + String b = "bar" + GString gq = "from Foo where x = ${a} and y = ${b}" + def params = [:] + + when: + String result = HqlQueryContext.resolveHql(gq, false, params) + + then: + result == "from Foo where x = :p0 and y = :p1" + params.p0 == "foo" + params.p1 == "bar" + } + + void "resolveHql collapses multiline query to single line"() { + expect: + HqlQueryContext.resolveHql("from Foo\nwhere x = 1", false, [:]) == "from Foo where x = 1" + } + + void "resolveHql with isNative=true skips alias normalisation"() { + // HQL path injects alias; native SQL path must leave the string unchanged + expect: + HqlQueryContext.resolveHql("select name from foo", true, [:]) == "select name from foo" + HqlQueryContext.resolveHql("select name from Foo", false, [:]) == "select e.name from Foo e" + } + + // ─── getTarget ──────────────────────────────────────────────────────────── + + void "getTarget with null hql returns entity class"() { + expect: + HqlQueryContext.getTarget(null, String) == String + } + + void "getTarget with no SELECT clause returns entity class"() { + expect: + HqlQueryContext.getTarget("from Person p", String) == String + } + + void "getTarget with single entity-alias projection returns entity class"() { + expect: + HqlQueryContext.getTarget("select p from Person p", String) == String + } + + void "getTarget with single qualified property projection returns Object"() { + expect: + HqlQueryContext.getTarget("select p.name from Person p", String) == Object + } + + void "getTarget with multiple projections returns Object array"() { + expect: + HqlQueryContext.getTarget("select p.name, p.age from Person p", String) == Object[].class + } + + // ─── countHqlProjections ───────────────────────────────────────────────── + + void "countHqlProjections returns 0 for null"() { + expect: HqlQueryContext.countHqlProjections(null) == 0 + } + + void "countHqlProjections returns 0 for empty string"() { + expect: HqlQueryContext.countHqlProjections("") == 0 + } + + void "countHqlProjections returns 0 when no SELECT clause"() { + expect: HqlQueryContext.countHqlProjections("from Person p") == 0 + } + + @Unroll + void "countHqlProjections returns 1 for single projection: #hql"() { + expect: HqlQueryContext.countHqlProjections(hql) == 1 + where: + hql << [ + "select p.name from Person p", + "select distinct p.name from Person p", + "select all p.name from Person p", + "select count(p) from Person p", + "select new map(p.name as n, p.age as a) from Person p", + ] + } + + void "countHqlProjections returns 2 for two top-level projections"() { + expect: HqlQueryContext.countHqlProjections("select p.name, p.age from Person p") == 2 + } + + void "countHqlProjections ignores commas inside single-level parentheses"() { + expect: HqlQueryContext.countHqlProjections("select coalesce(p.name, 'x') from Person p") == 1 + } + + void "countHqlProjections ignores commas inside nested parentheses"() { + expect: HqlQueryContext.countHqlProjections("select coalesce(trim(p.name), 'x') from Person p") == 1 + } + + void "countHqlProjections ignores commas inside single-quoted string literals"() { + expect: HqlQueryContext.countHqlProjections("select 'a,b' from Person p") == 1 + } + + void "countHqlProjections ignores commas inside double-quoted string literals"() { + expect: HqlQueryContext.countHqlProjections('select "a,b" from Person p') == 1 + } + + void "countHqlProjections handles escaped single-quote inside string literal"() { + expect: HqlQueryContext.countHqlProjections("select 'it''s,fine' from Person p") == 1 + } + + // ─── normalizeNonAliasedSelect ──────────────────────────────────────────── + + void "normalizeNonAliasedSelect returns null for null input"() { + expect: HqlQueryContext.normalizeNonAliasedSelect(null) == null + } + + void "normalizeNonAliasedSelect returns empty for empty input"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("") == "" + } + + void "normalizeNonAliasedSelect leaves query without SELECT unchanged"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("from Person") == "from Person" + } + + void "normalizeNonAliasedSelect leaves query that already has an alias unchanged"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select p.name from Person p") == "select p.name from Person p" + } + + void "normalizeNonAliasedSelect leaves query with AS alias unchanged"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select p.name from Person as p") == "select p.name from Person as p" + } + + void "normalizeNonAliasedSelect injects alias e for bare property projection"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select name from Person") == "select e.name from Person e" + } + + void "normalizeNonAliasedSelect replaces entity-name projection with alias"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select Person from Person") == "select e from Person e" + } + + void "normalizeNonAliasedSelect preserves DISTINCT prefix when injecting alias"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select distinct name from Person") == "select distinct e.name from Person e" + } + + void "normalizeNonAliasedSelect preserves ALL prefix when injecting alias"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select all name from Person") == "select all e.name from Person e" + } + + void "normalizeNonAliasedSelect does not qualify function expressions"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select count(id) from Person") == "select count(id) from Person e" + } + + void "normalizeNonAliasedSelect does not qualify constructor expressions"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select new map(name) from Person") == "select new map(name) from Person e" + } + + void "normalizeNonAliasedSelect injects alias before WHERE clause keyword"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select name from Person where age > 18") == + "select e.name from Person e where age > 18" + } + + void "normalizeNonAliasedSelect leaves malformed query without FROM unchanged"() { + expect: HqlQueryContext.normalizeNonAliasedSelect("select name") == "select name" + } +}
