Repository navigation
Merge 9.1.x (Spring Boot 4.2) into 9.0.x - #16937
codeconsole wants to merge 174 commits into
Conversation
Spring Framework 7.1 renamed org.springframework.aot.generate.ClassNameGenerator to NameGenerator (it now also generates resource paths). The constructors are unchanged, so the AOT specs only need the new type name.
…ionContext Spring Framework 7.1 adds it to BeanFactory as an abstract method. The mock resolves the reference to its raw class and applies the same name and type checks as getBean(String, Class).
graphql-java 26 (selected by Spring Boot 4.2) narrowed GraphQLSchema.Builder.additionalTypes to Set<? extends GraphQLNamedType>. GraphQLSchemaInterceptor.interceptSchema keeps handing interceptors a Set<GraphQLType>, so existing implementations still override it; Schema unwraps list and non-null wrappers before building, which adds the same named type graphql-java 25 reached by traversing the wrapper.
graphql-java-extended-scalars has no release newer than 24.0; the grails-data-graphql core and plugin test suites pass with 24.0 against graphql-java 26.1.
Spring Boot 4.2 manages Jedis 8, which depends on org.json:json:20260719 instead of 20251224. The SBOM license override is keyed by version, so grails-redis failed cyclonedxDirectBom until the entry moved with it.
Spring Boot 4.2.0-M2 manages the versions these pins overrode Spring Boot 4.1 with - Jackson 3.1.6, Logback 1.6.3 and Commons Codec 1.22.1 - so grails-bom no longer needs to redeclare them. The Spring Dependency Management end-to-end fixture only mirrors the logback and jackson3 overrides when they are present.
Jackson 2.21.6, which Spring Boot 4.2 manages, already carries the CVE-2026-68497 and CVE-2026-19032 fixes, so the 2.22.2 pin is only needed for Groovy YAML. MongoDB 5.12.0 now overrides 5.11.1, which already has the CVE-2026-18710 fix; it stays so the upgrade does not downgrade the driver.
Both coordinates are version-pinned. Spring Boot 4.2.0-M2 manages H2 2.5.250 (Sonatype still flags every H2 version for CVE-2018-14335) and Spring Security 7.2.0-M2 (outside the range Spring's CVE-2026-47838 advisory lists).
Names the Spring Security, Spring Data and Micrometer lines Spring Boot 4.2 manages and links its release notes and Spring Framework 7.1's. The structured logging note no longer ties the format IDs to Spring Boot 4.1.
Resolve the dependency and agent-guide conflicts while preserving Spring Boot 4.2-M2, the upstream BOM property names, and the relocated Grails developer skill.
Preserve the Spring Boot 4.2-M2 upgrade in the relocated vulnerability-scan plugin and integrate the latest target-branch updates. Validation: 9 SbomPluginSpec tests, vulnerability-scan compilation, and grails-base-bom/grails-bom property validation passed.
Spring Framework 7.1 renamed ClassNameGenerator to NameGenerator. The grails-core AOT specs already use the new type; grails-openapi still imported the old one, so every workflow that compiled its tests failed. Assisted-by: opencode:grok-4.7
The 9.0.x merge brought back this PR's older "Current Grails is built on" wording, which 9.x had already shortened.
The rule holds for whichever spring-boot-dependencies version the build imports, so it no longer needs editing on every Spring Boot bump.
Spring Boot 4.2 moves grails-data-graphql from graphql-java 25 to 26.1, and 26.0 has breaking changes. A new "Upgrading from Grails 9.0 to Grails 9.1" section lists the ones a Grails application can meet: query complexity limits enforced by default (100 levels, 100,000 fields) and how to change them with QueryComplexityLimits.setDefaultLimits, the deprecated non-null argument and input field validation that code-built schemas now get, and the named-type rule for additional schema types. What's New links to it. QueryComplexityLimitsSpec runs those limits against the schema the plugin generates: a 101-level query fails by default, raised and disabled limits let deeper queries through, and parse errors count as failures, since graphql-java's parser stops past about 165 levels on its own.
graphql-java only rejects deprecating a non-null argument or input field that has no default value (DeprecatedInputObjectAndArgumentsAreValid checks hasSetDefaultValue); a non-null one with a default can still be deprecated.
groovy-yaml 6.0.0 declares Jackson 2.22.2 as an ordinary dependency, which wins over the 2.21.6 Spring Boot 4.2 manages; nothing marks 2.22 as a minimum. The pin exists because the BOM must manage the winning version.
The PR now targets 9.0.x. Conflicts resolved as follows: - dependencies.gradle: 9.0.x pins Jackson 3 at 3.2.3, Jackson 2 at 2.22.3 and Logback at 1.6.5 for CVEs that the 3.1.6, 2.21.6 and 1.6.3 Spring Boot 4.2.0-M2 manages do not carry, so those pins stay, described against Spring Boot 4.2's versions. The jackson-core, jackson-databind, logback-classic and logback-core entries this branch had dropped come back with them. Guava 33.7.2 is taken from 9.0.x. - VulnerabilityScanPlugin.groovy: taken from 9.0.x, whose scan keys its exclusions by CVE and coordinate without a version. - AGENTS.md and the grails-developer skill: Groovy 6.0.x and Spock 2.5-groovy-6.0 from 9.0.x alongside Spring Boot 4.2.x and Spring Framework 7.1.x from this branch.
…sions 9.0.x pins Tomcat 11.0.26 as a security override of the 11.0.24 Spring Boot 4.1 manages, and SLF4J 2.0.19 above 4.1's 2.0.18 for Logback 1.6.5. Spring Boot 4.2.0-M2 manages exactly those versions, so validateBomProperties fails on the repeated pins and they and their entries go; the Tomcat CVE fixes and the SLF4J version Logback needs come from Spring Boot itself.
The reason still named 7.1.1 and Spring Boot 4.1.1 and asked to be reassessed on a Spring Security upgrade. Spring's advisory lists only 7.1.0 on the 7.1 line as affected; 7.2.0-M2, which Spring Boot 4.2.0-M2 manages, is not listed and contains the AesCbcBytesEncryptor and AesGcmBytesEncryptor replacements the 7.1.1 fix introduced, so the exclusion still holds.
With the PR on 9.0.x (9.0.0-SNAPSHOT), applications get Spring Boot 4.2 and graphql-java 26 when they upgrade from Grails 8, not from 9.0 to 9.1. The section moves to upgrading90x, "Upgrading from Grails 8 to Grails 9", above the Grails 7 to Grails 8 entry, and opens by describing the move from Grails 8 on Spring Boot 4.1. What's New points its introduction and its Spring Boot 4.2 section at it.
Both getBean(String, ParameterizedTypeReference) and getBeanProvider spelled the type out in full, while ResolvableType beside it is imported.
Renaming ClassNameGenerator in place left NameGenerator ahead of DefaultGenerationContext; the grails-core specs already list it in order.
Spring Framework 7.1 deprecates ForwardedHeaderUtils.adaptFromForwardedHeaders for removal in favor of parseStandardHeader and parseXForwardedHeaders, so callers choose a header format. GrailsSecurityHeadersFilter, which only needs the scheme for its HSTS decision, keeps the precedence the old method applied: the RFC 7239 Forwarded header when the request has one, otherwise X-Forwarded-Proto and X-Forwarded-Ssl. Two edge cases change, both only for HSTS, which user agents ignore on a plain connection: X-Forwarded-Ssl is no longer consulted next to a Forwarded header without proto, and a malformed for or by address in the Forwarded header now invalidates it like any other malformed forwarded value. The spec covers both, and that RFC 7239 obfuscated and quoted IPv6 for values still parse.
The upgrade guide now lists the Spring Boot 4.2 and Spring Framework 7.1 changes a Grails application is most likely to notice: - server.forward-headers-strategy=framework reads only X-Forwarded-* by default, and the filter now comes from WebMvcAutoConfiguration, so an application declaring @EnableWebMvc no longer gets it - embedded Tomcat keeps relative redirect locations relative, which changes the Location header of chain() but not of redirect() - TestRestTemplate is deprecated for removal in favor of RestTestClient - Spring MVC no longer auto-detects the JAXB converter (Grails render and respond are unaffected) - tests load static nested @configuration classes from superclasses and enclosing classes The deployment guide's reverse-proxy section names the header-format property for proxies that send the RFC 7239 Forwarded header.
|
@matrei thanks, all addressed at 1d04adf:
Also, 9.0.x moved after your review: #16938 made the same |
The approval no longer holds: a filled email or URL can be longer than the field allows, so the trace records a form that failed validation as a success.
jamesfredley
left a comment
jamesfredley
left a comment
There was a problem hiding this comment.
Not approving. A filled email or URL can be longer than the field allows, so the trace records a form that failed validation as a success.
| return within(String.format(Locale.ROOT, '%d-W%02d', | ||
| now.get(IsoFields.WEEK_BASED_YEAR), now.get(IsoFields.WEEK_OF_WEEK_BASED_YEAR)), tag) | ||
| case 'email': | ||
| return traced(maxLength == null ? null : Math.max(maxLength - EMAIL_DOMAIN.length(), 1)) + EMAIL_DOMAIN |
There was a problem hiding this comment.
An empty email field with maxlength 10 is filled with at least 13 characters, because the domain is always appended. A URL cannot be shorter than the base that is always prepended. The value fails the field's length constraint, the form is shown again with a success status, and the save is never traced. Fit the value to the declared length.
There was a problem hiding this comment.
Fixed in 76dc361. The email and URL values now fit the declared maxlength:
- Email:
traced-…@example.comwhen the length leaves room for a name. Otherwise the shorter@x.iodomain, which still passes theemailconstraint, somaxlength="10"gets12345@x.io. - URL:
https://example.com/<digits>when there is room for a path. Otherwise the longest ofhttps://example.com/,https://example.comandhttp://x.iothat fits. - Below 6 characters for an email, or 11 for a URL, no valid value fits, so the value is cut to the length.
The typed-form feature in TraceNativeMetadataTaskSpec now covers fields at maxlength 10 and 4 for email, and 24, 19, 12 and 5 for URL.
This code is also on 9.1.x. The merge back of 9.0.x into 9.1.x after this lands takes the fix there.
An empty email field was filled with at least the example domain plus one character, and a URL with at least its base, so a field whose maxlength was shorter failed its length constraint. The form was shown again with a 200, and the trace read it as a save that was never recorded. An email now falls back to a shorter domain, and a URL to the site without a path or a shorter site, whichever is the longest that fits. Where no valid value fits, the value is cut to the length.
jamesfredley
left a comment
jamesfredley
left a comment
There was a problem hiding this comment.
Not approving. An email now fits the field. A URL fitted to a short maxlength can be a link the URL check rejects, so the save is still not traced.
| if (maxLength == null || maxLength > URL_BASE.length()) { | ||
| return URL_BASE + traced(maxLength == null ? null : maxLength - URL_BASE.length()) | ||
| } | ||
| SHORT_URLS.find { String url -> url.length() <= maxLength } ?: cut(SHORT_URLS.last(), maxLength) |
There was a problem hiding this comment.
An empty URL field with maxlength 10 is filled with http://x.i. That is ten characters, and the URL check rejects it. ftp://x.io is also ten characters and passes. The form is shown again with a success status, and the save is never traced. Use a link that both fits and passes the URL check.
There was a problem hiding this comment.
Fixed in a122fe1, though not with ftp://x.io. As @matrei pointed out, x.io is a registered domain with live mail and web servers, so any short value on it could send a traced save's mail or link fetch to a third party. Every value now stays on example.com. The fallbacks are https://example.com, http://example.com and ftp://example.com, so 17 is the shortest URL field that gets a link. Below that the field gets the traced name, which fails the url constraint, so that save isn't traced. I don't cut values anymore, because a cut value can name another host (ftp://example.co).
The spec now runs every filled value through the validator the url constraint builds and through the default UrlValidator. It also checks that the fields too short for a link fail.
sbglasius
left a comment
sbglasius
left a comment
There was a problem hiding this comment.
This an AI assisted review
Reviewed at 76dc3617b0, base 9.0.x. The earlier reviews covered the merge itself in depth, so this one checks what changed after them.
What I checked:
- The tree:
9.1.xis an ancestor of the head. The head differs from9.1.xin 22 files: the four version files, the 19@since 9.0tags (55cfcda58f), the guide section number (1d04adf7c5), theGPathResultCollectionDataBindingSourcechange taken from #16938, and theTraceNativeMetadataTaskfix (76dc3617b0).grails-databinding-coreis identical to9.0.x. - Base:
9.0.xhas moved by one commit since the head (a74c662ef0, #16941, which changes only.github/workflows/gradle.yml). It still merges with no conflicts. - Merge-back recipe: I ran the recipe from the description in a scratch worktree on the head:
git show a1a2265c9f | git apply -R --index. It applies cleanly and touches onlygradle.properties,GrailsUtilTests,THREAT_MODEL.mdandthreat-model.yaml. The 9.1.x tree that results keeps the@since 9.0tags, the guide numbering, the #16938 change and the traceNativeMetadata fix, as intended. - CI: 90 checks pass. The only failures are the three Groovy Snapshot Canary shards, which also fail on the
9.0.xtip.
1. A URL field with maxlength 10 still gets a URL that fails validation
This is the issue @jamesfredley raised at TraceNativeMetadataTask.groovy:682, and it is still there at 76dc3617b0. When no entry in SHORT_URLS fits, link() cuts the last entry, so a field with maxlength 10 gets http://x.i. A URL validator rejects that because i is not a TLD. The form is shown again with a 200, and the trace reads it as a save that was never recorded. That is the failure the commit set out to fix.
ftp://x.io is 10 characters and passes the default UrlValidator schemes (http, https, ftp). Adding it as the last entry of SHORT_URLS would cover maxlength 10. Below 10, no valid URL fits, so cutting the value is the right fallback there.
The typed-form spec tests URL maxlength 24, 19, 12 and 5, which leaves this case out. A maxlength="10" URL field in TracedRunFixture, checked against a valid URL, would cover it.
2. The description no longer lists everything the head adds to 9.1.x (minor)
The description says that after this PR "the 9.0.x tree is the 9.1.x tree except for the four version files and those three commits". 76dc3617b0 now also changes TraceNativeMetadataTask and its spec and fixture. Please add it to the list, so whoever does the merge back knows the full set of changes 9.1.x receives.
Once 1 is fixed, this looks good to me. Please merge with a merge commit, as the description asks, so that 9.1.x stays an ancestor of 9.0.x.
matrei
left a comment
matrei
left a comment
There was a problem hiding this comment.
Second review of #16937 at 76dc3617b0 (base 9.0.x). Since my first review there are four new commits: the merge of 9.0.x for #16938 (91e777a66f), 55cfcda58f, 1d04adf7c5 and 76dc3617b0. It merges cleanly with the current tip, a74c662ef0.
@codeconsole, thanks. Everything from my first review is fixed:
- All 19 tags are
@since 9.0, in their own commit. No later commit touches the four version files, so reverse-applyinga1a2265c9fstill restores only the versions. - The sections of the Grails 9 upgrade guide now run from 1 to 23.
- The description has the merge-back recipe, and you've agreed not to cut a 9.0.0 milestone or RC before the Boot 4.2.0 bump.
What I checked and ran:
- The merge of
9.0.x:git show --remerge-diff 91e777a66fshows a single conflict, inGPathResultCollectionDataBindingSource, resolved with the9.0.xside.grails-databinding-coreis identical to9.0.x. - The tree: the head differs from
9.1.xin 22 files: the four version files, the 13 files with the@sincetags, the guide, the #16938 fix and its spec, andTraceNativeMetadataTaskwith its spec and fixture. - The values
76dc3617b0fills in: I checked them against commons-validator 1.10.1, the version behind Grails'emailandurlconstraints. I used both theUrlValidatorthat theurlconstraint builds (ALLOW_ALL_SCHEMES) and the default one. I also looked up the DNS records of the domains involved. - CI on this head: every build and test job passes, including "Build Gradle Plugins" on Java 21 and 25. The three Groovy Snapshot Canary shards fail, as they do on
9.0.x.
1. The short email and URL values point at a real domain
x.io is a registered domain that receives mail: it has an A record and Google Workspace MX records. example.com is reserved for examples (RFC 2606) and publishes a null MX (RFC 7505), so mail to it is never delivered. The trace submits forms to the running application. If an application sends mail on a save, as a sign-up or contact form does, an email field with maxlength 6 to 12 now sends it to an address on x.io, such as 12345@x.io at 10, which reaches a real mail server. A URL field with maxlength 11 to 18 now gets http://x.io, a live host, if the application fetches the link. ftp://x.io, suggested for maxlength 10, would add another link to that host.
None of the reserved names fits in less space. With commons-validator 1.10.1, 1@x.test, 1@x.example, 1@x.invalid and 1@localhost are all rejected, and so are http://x.test, http://x.invalid and http://localhost. The shortest values that stay on example.com and pass are 1@example.com (13 characters) and ftp://example.com (17).
I'd stay on example.com:
private static final List<String> SHORT_URLS = ['https://example.com/', 'https://example.com', 'http://example.com', 'ftp://example.com']Below those lengths, I'd send the traced name alone, traced(maxLength), rather than cutting a value. It has no @ and no scheme, so neither validator accepts it, and it names no host. Cutting is not safe: 1@example.com cut to 12 is 1@example.co, and ftp://example.com cut to 16 is ftp://example.co. Both pass validation, and example.co is a registered domain with MX records.
A save behind an email field shorter than 13 characters, or a URL field shorter than 17, then isn't traced. That seems a better trade than sending mail to a third party.
2. http://x.i fails the url constraint
I can confirm what @jamesfredley and @sbglasius found:
| Value | Length | url constraint |
Default UrlValidator |
|---|---|---|---|
http://x.i (maxlength 10 today) |
10 | rejected | rejected |
ftp://x.io |
10 | accepted | accepted |
http://x.io |
11 | accepted | accepted |
ab://x.io |
9 | accepted | rejected |
a://x.io |
8 | accepted | rejected |
Because the constraint accepts any scheme, "below 10 no valid URL fits" holds only for the default validator. With 1, this case goes away: maxlength 10 is below 17, so the field gets the traced name and fails on purpose.
3. Smaller points
- Tests: the typed-form feature checks the shape and the length of each value, not whether a validator accepts it. That is how
http://x.igot through. Passing the email and URL values throughEmailValidator.getInstance()and theUrlValidatortheurlconstraint builds would catch this kind of mistake. That needs commons-validator as a test dependency of the plugins project. - Guide:
deploymentAot.adoc:150says an email field gets "an address", and only a text field is said to stay withinmaxlength. URL fields aren't mentioned. Now that both kinds of field honormaxlength, something like "an address or a link onexample.comfor an email or URL field, ... each no longer than the field'smaxlength" would describe it. Naming the domain also tells readers where a traced save's mail would go. - Description: +1 to @sbglasius: "except for the four version files and those three commits" should also mention
76dc3617b0, so that whoever merges back into9.1.xknows what it receives.
Verified as correct
- Email values on
example.com: from maxlength 13, the address is a tail of the traced name on@example.com, for example1@example.comat 13. Every such value passesEmailValidator. - URL values on
example.com: from 20 characters up the link ishttps://example.com/…, and at 19 it ishttps://example.com. All of these pass both validators. - Merge-back recipe:
a1a2265c9fis unchanged, and none of55cfcda58f,1d04adf7c5or76dc3617b0touches its four files. The recipe in the description still restores only the 9.1.x version values.
The short fallbacks, @x.io and http://x.io, are a registered domain with live mail servers, so a traced save that sends mail could deliver it to a third party, and http://x.i, cut to fit a maxlength of 10, fails the url constraint. An email is now sent on example.com from maxlength 13, and a URL is one of https://example.com/..., https://example.com, http://example.com or ftp://example.com, the longest that fits. Below those lengths the traced name is sent alone, which no validator accepts and which names no host, since a cut value can name another domain (1@example.co). The spec checks each value with the validators behind the email and url constraints, from commons-validator as a test dependency, and the guide says which domain a traced save uses and which short fields it does not trace.
|
@matrei @sbglasius thanks, everything from both reviews is fixed in a122fe1. The domains (matrei 1, 2; sbglasius 1). I went with matrei's approach. Every email and URL value now stays on
No value is cut anymore. This differs from sbglasius's Tests (matrei 3). commons-validator is now a test dependency of the plugins project. Its version is read from the root The typed-form feature now checks:
The fixture has fields at 13 and 12 for email, and at 24, 19, 18, 17 and 16 for URL. Guide (matrei 3). Description (matrei 3; sbglasius 2). It now lists I ran the full |
sbglasius
left a comment
sbglasius
left a comment
There was a problem hiding this comment.
This an AI assisted review
Approving at a122fe1f4f. Both points from my first review are fixed, and keeping every value on example.com is the right call. One non-blocking nit: grails-gradle/plugins/build.gradle could use $commonsValidatorVersion directly, since org.apache.grails.buildsrc.properties already loads the root gradle.properties.
matrei
left a comment
matrei
left a comment
There was a problem hiding this comment.
Third review of #16937 at a122fe1f4f (base 9.0.x). There is one new commit since my second review. It still merges cleanly with the 9.0.x tip, a74c662ef0.
@codeconsole, thanks. Everything from my second review is fixed:
- Every email and URL value stays on
example.com: an address from maxlength 13, and a link from 17. Below that the field gets the traced name, and no value is cut anymore. - The typed-form feature runs every address and link through
EmailValidator, through theUrlValidatortheurlconstraint builds, and through the defaultUrlValidator. It checks that each is onexample.comand that the two fields too short for one fail. The fixture has fields on both sides of each boundary, at 13 and 12 for email and at 17 and 16 for URL. deploymentAot.adocnamesexample.comand says which fields aren't traced.- The description lists
76dc3617b0anda122fe1f4fwith the commits that9.1.xreceives.
What I checked and ran:
- The values: I ported
traced,addressandlinkto Java and ran them with no maxlength and with every maxlength from 0 to 30, against commons-validator 1.10.1. Every value that passes is onexample.com, every value that fails has no@and no scheme, and none is longer than its field. - The spec:
TraceNativeMetadataTaskSpecpasses locally on Java 21, all 30 tests. - The test dependency: commons-validator resolves at 1.10.1, the version
grails-datamapping-validationuses. I changedcommonsValidatorVersionin the rootgradle.propertiesas a test. Thegrails-gradlebuild then discarded its configuration cache entry and resolved the new version, so a later bump won't be missed. - The tree: the head differs from
9.1.xin 24 files: the 22 from my second review, plusdeploymentAot.adocandgrails-gradle/plugins/build.gradle. - Merge-back recipe:
a122fe1f4fdoesn't touch the four version files, andgit show a1a2265c9f7 | git apply -R --checkpasses on the head. - CI on this head: "Build Gradle Plugins" passes on Java 21 and 25. The three Groovy Snapshot Canary shards fail with the same compile errors as in my first review. The rest was still running when I posted this.
Optional: say when a field is too short to trace
The guide says a form that answers with itself "looks to the trace like a save that succeeded". So when an email field is shorter than 13 characters, or a URL field shorter than 17, the trace finishes as usual, but that save is not traced. Nothing in the output says so. A logger.warn in submit would tell the developer to give the field a value in nativeMetadata.forms. It would name the form and the field, as the warning for a value naming a missing field does, and it would skip a field that already has a value from the query string. This isn't a blocker, and it can be a follow-up.
From my side this is ready once CI passes. In the weekly meeting we also decided to get input from @jdaugherty before 9.1.x is merged into 9.0.x, so please hold the merge until we have it.
An email field shorter than 13 characters, or a URL field shorter than 17, is sent text it rejects, so the page answers with the form again and the trace finishes as if the save had been recorded. The trace now warns, naming the form and the field and how to give it a value in nativeMetadata.forms. A field given a value there is not warned about. The commons-validator test dependency takes its version from the shared root property, which the build's properties plugin already loads.
|
@matrei @sbglasius thanks for the approvals. I've done both of your optional points in e2664a5:
The full |
jamesfredley
left a comment
jamesfredley
left a comment
There was a problem hiding this comment.
Not approving. A field too short for an address is still sent text it rejects, and that answer is recorded as a traced save. Digest reauthentication also clears the password on the cached user.
| } | ||
| // A field too short for any address or link on example.com is sent text it rejects, so the page answers | ||
| // with the form again, which reads as a save that succeeded. Said here, as nothing after this can tell. | ||
| (tooShortFields(markup) - given.keySet()).each { String name -> |
There was a problem hiding this comment.
An empty URL field with maxlength 10 is filled with traced- and three digits. That fits, and the URL check rejects it. The warning says the save is not traced, then the submit still runs. The form comes back with status 200, and that status is recorded as a successful trace. Do not submit a field that cannot hold a valid address, and do not record that response as a trace.
There was a problem hiding this comment.
Fixed in e164d65. A form with an email field shorter than 13 or a URL field shorter than 17 is no longer submitted. Its answer is recorded as not traced, so the trace fails with FORM /typed -> not submitted: tinyHomepage too short for an address or link on example.com. Give each a value in nativeMetadata.forms, as in /typed?tinyHomepage=.... A field given a value in forms doesn't count. The spec checks that the trace fails, that the message names only the field without a value, and that nothing was posted. The guide now says the trace fails, not that it warns.
| if (authentication.authenticated && (!alwaysReauthenticate || authentication.credentials == null)) { | ||
| return authentication | ||
| } | ||
| Authentication authenticated = authenticationManager.authenticate(authentication) |
There was a problem hiding this comment.
Digest authentication is on, and users are cached. This authenticates the cached principal. The authentication manager erases its credentials, and the cache holds that same instance, so the password is gone. The next digest request misses the cache and loads the user again. Authenticate a copy, and leave the cached user with its password.
There was a problem hiding this comment.
Confirmed, and fixed in 1c493e2, at the cache rather than by copying in the interceptor. A copy there wouldn't help. With cacheUsers, the digest filter and daoAuthenticationProvider share userCache, and that cache stored users by reference. The provider looks the user up by name and returns the cached instance as the principal, so the authentication manager erased the cached user whatever token it was given.
The cache now stores users by value, which is the JCache default. Each lookup gets a serialized copy (UserDetails is Serializable), and the manager erases that copy as usual. The cached user keeps its stored hash, and the user in the security context and the session holds no password. This also fixes form sign-in with cacheUsers, which erased the cached user in the same way. A user that can't be serialized is left out of the cache with a warning, so its sign-in still works.
An intermediate fix, 22fc54c, skipped erasing a principal the cache holds. I replaced it because that left the hash on the session's user. DigestDaoAuthenticationProvider is back to how it was before.
Tests:
CachedDigestAuthenticationSpec(cacheUsers on, real filter chain): the cached user keeps its hash after a Digest sign-in; a second sign-in is answered from the cache, with one load for both; the signed-in user has no password while the cached one keeps it. The first two failed before the fix.SpringUserCacheFactoryBeanSpec, against Ehcache: erasing a looked-up user leaves the cached one intact; aGrailsUsercopy keeps its class,idand authorities; a user that can't be serialized isn't cached and doesn't throw.- All
:grails-spring-securitytests pass.
The Grails 9 upgrade guide has a new section 24 on the change, including how to go back to storing by reference. The upgrade skill and the plugin's user cache docs are updated too.
This code came in from 9.1.x (#16871), so the merge back fixes it there too.
With cacheUsers, DigestAuthenticationFilter and the daoAuthenticationProvider share the user cache, which holds users by reference. The security interceptor authenticates what the filter leaves on every request, the provider returns the cached user as the principal, and the authentication manager erases the credentials of what it returns, so the cached user lost its stored password. The next request then missed the cache and loaded the user again. DigestDaoAuthenticationProvider now returns a token whose erased credentials leave a principal the user cache holds alone. Its own credentials are still erased, and a principal nothing else holds is erased as before. Copying the token the interceptor authenticates would not help, as the provider looks the user up in the cache by name.
A form with an email field shorter than 13 characters, or a URL field shorter than 17, was submitted with text the field rejects, after a warning. The page answered with the form again and a 200, which was recorded as a traced save. Such a form is no longer submitted. Its answer is recorded as not traced, so the trace fails, naming each field and how to give it a value in nativeMetadata.forms. A field given a value there does not count.
…ed-in user 22fc54c kept the cached user's password by not erasing a principal the user cache holds, which left the stored password, or Digest's hash, on the user in the security context and the session. The user cache stored users by reference, so the cached user, the principal the provider returned and the one in the session were the same instance, and the password was either erased from the cache or kept in the session. The cache now stores users by value, as JCache does by default: each lookup gets a serialized copy, the authentication manager erases that copy as usual, and the cached user keeps what it is checked against. This also stops a form sign-in with cacheUsers from erasing the cached user. A user that cannot be serialized is left out of the cache with a warning rather than failing the sign-in. DigestDaoAuthenticationProvider no longer needs its own token, and is back to what it was before 22fc54c. The upgrade guide, the Grails 9 upgrade skill and the plugin's user cache docs describe the change.
Storing cached users by value (1c493e2) makes Ehcache create its key copier, Eh107IdentityCopier, reflectively when the user cache is created, and copy each user by Java serialization. No library ships metadata for either, so a native image with cacheUsers failed at startup, and with the copier registered every sign-in failed: a class without serialization metadata raises MissingReflectionRegistrationError, a LinkageError the cache's catch of RuntimeException let through. SpringSecurityRuntimeHints now registers the copier's constructor and, for serialization, the users the plugin creates: User and GrailsUser with a Long id, SimpleGrantedAuthority, and the sorted set User keeps them in. The user cache also catches a LinkageError, so a user of the application's own that an image cannot copy is left uncached with a warning, as on the JVM. A spec serializes a GrailsUser and checks every class written is registered, so a change in what Spring Security writes fails on the JVM. Verified in a native image whose trace had no sign-in, with the traced copier entry removed: it starts, five Digest sign-ins load the user once, and a user holding an unregistered type is warned about and still signs in.
The UI plugin's user cache page reads userCache.cache.nativeCache. SpringCacheBasedUserCache keeps its cache in a private field with no getter, which Groovy reached only on that class itself, so with the cache's subclass from 1c493e2 the page no longer found it, and DefaultSecurityInfoSpec and ExtendedSecurityInfoSpec failed. The subclass now exposes the cache as a property.
🚨 All tests passed but jobs failed 🚨Failed Jobs without Test Failures❌ Groovy Snapshot Canary Build / Build Grails (shard 0) 🏷️ Commit: e7e2c17 Rerun ControlsClick the checkbox to trigger a rerun:
Learn more about TestLens at testlens.app/docs. |
jamesfredley
left a comment
jamesfredley
left a comment
There was a problem hiding this comment.
A field too short for an address is no longer submitted or recorded as a trace. The cache stores a copy, so digest reauthentication does not clear the cached password.
Spring Boot 4.2.0 is scheduled to be released November 19, 2026
This PR was created PR the discussion in the weekly meeting 11/11/26
Brings the Spring Boot 4.2 upgrade from 9.1.x into 9.0.x.
Everything on 9.1.x that is not already on 9.0.x comes from these three pull requests:
The only other commits on 9.1.x are:
80a84cc659,23807a04d2and5d7b6adc8d: these switch AOT specs toNameGenerator, the Spring Framework 7.1 name forClassNameGenerator. Boot 4.2 brings in Framework 7.1, so 9.0.x needs them too.d6bfb8ea56(the data binding white list for a compiled parent): the same change is already on 9.0.x asce2db7a52d, and the files are identical on both branches.829de6092e, which sets the branch version to 9.1.0-SNAPSHOT. The second commit of this PR restores the 9.0.x values ingradle.properties,GrailsUtilTests,THREAT_MODEL.mdandthreat-model.yaml.Ten more commits come after it:
GPathResultCollectionDataBindingSourcechange as9bf4e00ba6on 9.1.x, so the conflict is resolved with the 9.0.x side.55cfcda58f, which tags the Boot 4.2 API@since 9.0instead of@since 9.1, because 9.0.0 is the first release it ships in.1d04adf7c5, which numbers the configuration key section of the Grails 9 upgrade guide 23. It had kept the number 21 from 9.0.x through an earlier merge-up.76dc3617b0,a122fe1f4f,e2664a51dcande164d65bc6, which keep whattraceNativeMetadatafills an email or URL field with within the field'smaxlengthand onexample.com. Where nothing onexample.comfits, the form is not submitted and the trace fails, naming the field. The bug is on 9.1.x too, so the merge back fixes it there.22fc54ca4f,1c493e2244andb6996ed8a0, which make the Spring Security user cache (cacheUsers) store users by value. It stored them by reference, so when the authentication manager erased the signed-in user's password, it erased the cached user too. The security interceptor from Spring Boot 4.2.0-RC2 #16871 authenticates a Digest user on every request, so every request then missed the cache. Each lookup now gets a copy, which the plugin's AOT hints let a native image make. Section 24 of the Grails 9 upgrade guide describes the change. The bug is on 9.1.x too.After this PR, the 9.0.x tree is the 9.1.x tree except for the four version files and those ten commits.
Merge-up note: once this lands, 9.1.x is an ancestor of 9.0.x, so a plain
git merge 9.0.xon 9.1.x fast-forwards and sets 9.1.x to 9.0.0-SNAPSHOT. Merge 9.0.x back into 9.1.x right after this lands, before anything else is pushed to 9.1.x, and reverse-apply the version commit inside that merge:git switch 9.1.x git merge --no-ff --no-commit origin/9.0.x git show a1a2265c9f7 | git apply -R --index git commitThat keeps
projectVersion=9.1.0-SNAPSHOT,githubBranch=9.1.x, theGrailsUtilTestsvalue and the 9.1.x threat-model binding, and 9.1.x still gets the@since 9.0tags and the guide numbering. From then on both branches share those lines, so later merge-ups keep the 9.1.x version.Please merge with a merge commit, not a squash, so that 9.1.x stays an ancestor of 9.0.x.