Skip to content

Avoid reflective final field mutation in BasicLookupStrategy#19331

Open
seonwooj0810 wants to merge 1 commit into
spring-projects:mainfrom
seonwooj0810:gh-19127-acl-final-field-mutation
Open

Avoid reflective final field mutation in BasicLookupStrategy#19331
seonwooj0810 wants to merge 1 commit into
spring-projects:mainfrom
seonwooj0810:gh-19127-acl-final-field-mutation

Conversation

@seonwooj0810

Copy link
Copy Markdown

Closes gh-19127

Background

On Java 26 the JVM warns whenever ACLs are loaded through BasicLookupStrategy:

WARNING: Final field aces in class org.springframework.security.acls.domain.AclImpl has been mutated reflectively
by class org.springframework.security.acls.jdbc.BasicLookupStrategy in unnamed module ...
WARNING: Use --enable-final-field-mutation=ALL-UNNAMED to avoid a warning
WARNING: Mutating final fields will be blocked in a future release unless final field mutation is enabled

BasicLookupStrategy.convert(...) is responsible for re-parenting a loaded AclImpl onto a freshly constructed one: stub parent references are resolved and the back-reference on every AccessControlEntryImpl is rewritten to point at the new ACL (per SEC-951). The previous implementation did this by reflectively rewriting two final fields:

  • AclImpl.aces was replaced through Field.set(...) (setAces).
  • AccessControlEntryImpl.acl was replaced through Field.set(...) (setAclOnAce).

Both writes trigger the Java 26 warning today and will be rejected outright on the JVMs that flip the default. Reading the aces field reflectively is not affected — only mutation is.

Change

Neither final field actually needs to be replaced:

  • The new AclImpl is created with an empty, mutable aces list (initialised inline as new ArrayList<>()). Adding to that existing list in place achieves the same end state without touching the field reference. addAces (replacing setAces) reuses the existing read helper and calls addAll on the list.
  • AccessControlEntryImpl is documented as immutable and exposes a public constructor that takes every field. Re-creating each ACE with the converted Acl lets us substitute the StubAclParent-bearing ACE for an equivalent one without mutating any final field. setAclOnAce and the cached Field for AccessControlEntryImpl.acl go away entirely.

The reflective read of AclImpl.aces is preserved — it is necessary to populate the result list and is unaffected by the upcoming JVM restriction. The internal ProcessResultSet.mapRow path is unchanged: it already uses the same read-then-add idiom to populate the input ACLs.

Tests

Added readAclsByIdWhenChildLoadedThenAcesPointToConvertedAcl to AbstractBasicLookupStrategyTests, which asserts the SEC-951 invariant directly: after readAclsById(...), every ACE returned by acl.getEntries() reports the converted Acl (not a StubAclParent) via getAcl(). Both BasicLookupStrategyTests and BasicLookupStrategyWithAclClassTypeTests extend the abstract class, so the new test runs against both class-id strategies.

./gradlew :spring-security-acl:check passes (test, checkFormat, checkstyle, license, jacoco).

Verification done

  1. No in-flight PR: gh pr list --repo spring-projects/spring-security --search "AclImpl OR \"final field\" OR \"Java 26\"" returns nothing related, and gh pr list --search "19127" is empty.
  2. No claim in the issue thread: Final field mutation reported on Java 26 #19127 has zero comments at the time of writing.
  3. Code-focused fix: touches acl/src/main/java/.../BasicLookupStrategy.java and the existing abstract test class.
  4. Pattern still present on main: confirmed fieldAces.set(...) and fieldAcl.set(...) still live in BasicLookupStrategy at HEAD (5594fc018f).
  5. Reproducer matches: the warning text in Final field mutation reported on Java 26 #19127 names BasicLookupStrategy mutating AclImpl.aces, which is the call this change removes.

`BasicLookupStrategy.convert(...)` previously used reflection to
re-write two final fields when re-parenting ACEs onto the freshly
created `AclImpl`:

- `AclImpl.aces` was overwritten via `Field.set(...)`
- `AccessControlEntryImpl.acl` was overwritten via `Field.set(...)`

On Java 26 the JVM emits a `WARNING: Final field ... has been mutated
reflectively` for each ACL load, and a future release intends to block
this entirely.

Mutating the field reference was never required:

- The new `AclImpl` is constructed with an empty, mutable `aces`
  list, so the ACEs can be added to that existing list in place.
  Only the field is `final`; the list it points to is not.
- `AccessControlEntryImpl` is documented as immutable and exposes a
  constructor that accepts every field. Re-creating each ACE with
  the converted `Acl` back-reference removes the StubAclParent
  reference exactly as before, without touching its final `acl`
  field.

`fieldAcl` and `setAclOnAce(...)` are removed, and `setAces(...)` is
replaced with `addAces(...)` that calls `addAll` on the result's
existing aces list. The reflective read of `AclImpl.aces` is kept,
since reads of final fields are unaffected.

Closes spring-projectsgh-19127
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting-for-triage An issue we've not yet triaged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Final field mutation reported on Java 26

2 participants