From f5c61835d59cfd2546b008db15c35ed178ec9fa4 Mon Sep 17 00:00:00 2001 From: seonwoo_jung Date: Sun, 14 Jun 2026 06:13:31 +0900 Subject: [PATCH] Avoid reflective final field mutation in BasicLookupStrategy `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 gh-19127 --- .../acls/jdbc/BasicLookupStrategy.java | 52 ++++++------------- .../AbstractBasicLookupStrategyTests.java | 16 ++++++ 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java b/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java index b47ea993ab2..f205ff2e149 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java @@ -1,5 +1,5 @@ /* - * Copyright 2004, 2005, 2006, 2017 Acegi Technology Pty Limited + * Copyright 2004, 2005, 2006, 2017, 2026 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -127,8 +127,6 @@ public class BasicLookupStrategy implements LookupStrategy { private final Field fieldAces = FieldUtils.getField(AclImpl.class, "aces"); - private final Field fieldAcl = FieldUtils.getField(AccessControlEntryImpl.class, "acl"); - // SQL Customization fields private String selectClause = DEFAULT_SELECT_CLAUSE; @@ -171,7 +169,6 @@ public BasicLookupStrategy(DataSource dataSource, AclCache aclCache, this.objectIdentityGenerator = new ObjectIdentityRetrievalStrategyImpl(); this.aclClassIdUtils = new AclClassIdUtils(); this.fieldAces.setAccessible(true); - this.fieldAcl.setAccessible(true); } private String computeRepeatingSql(String repeatingSql, int requiredRepetitions) { @@ -201,22 +198,11 @@ private List readAces(AclImpl acl) { } } - private void setAclOnAce(AccessControlEntryImpl ace, AclImpl acl) { - try { - this.fieldAcl.set(ace, acl); - } - catch (IllegalAccessException ex) { - throw new IllegalStateException("Could not or set AclImpl on AccessControlEntryImpl fields", ex); - } - } - - private void setAces(AclImpl acl, List aces) { - try { - this.fieldAces.set(acl, aces); - } - catch (IllegalAccessException ex) { - throw new IllegalStateException("Could not set AclImpl entries", ex); - } + private void addAces(AclImpl acl, List aces) { + // Populate the existing aces list in place rather than replacing the + // (final) field reference via reflection, which Java 26+ blocks. The + // list itself is mutable; only the field that holds it is final. + readAces(acl).addAll(aces); } /** @@ -407,25 +393,19 @@ private AclImpl convert(Map inputMap, Long currentIdentity) { AclImpl result = new AclImpl(inputAcl.getObjectIdentity(), inputAcl.getId(), this.aclAuthorizationStrategy, this.grantingStrategy, parent, null, inputAcl.isEntriesInheriting(), owner); - // Copy the "aces" from the input to the destination - - // Obtain the "aces" from the input ACL + // Copy the "aces" from the input to the destination. + // + // Re-create each AccessControlEntryImpl rather than mutating its final + // "acl" back-reference via reflection, so that StubAclParent references + // are replaced with the new "result" AclImpl instance (as per SEC-951). + // Java 26+ blocks reflective mutation of final fields. List aces = readAces(inputAcl); - - // Create a list in which to store the "aces" for the "result" AclImpl instance - List acesNew = new ArrayList<>(); - - // Iterate over the "aces" input and replace each nested - // AccessControlEntryImpl.getAcl() with the new "result" AclImpl instance - // This ensures StubAclParent instances are removed, as per SEC-951 + List acesNew = new ArrayList<>(aces.size()); for (AccessControlEntryImpl ace : aces) { - setAclOnAce(ace, result); - acesNew.add(ace); + acesNew.add(new AccessControlEntryImpl(ace.getId(), result, ace.getSid(), ace.getPermission(), + ace.isGranting(), ace.isAuditSuccess(), ace.isAuditFailure())); } - - // Finally, now that the "aces" have been converted to have the "result" AclImpl - // instance, modify the "result" AclImpl instance - setAces(result, acesNew); + addAces(result, acesNew); return result; } diff --git a/acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java b/acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java index 0eda3d54635..9107cc97001 100644 --- a/acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java +++ b/acl/src/test/java/org/springframework/security/acls/jdbc/AbstractBasicLookupStrategyTests.java @@ -46,6 +46,7 @@ import org.springframework.security.acls.domain.ObjectIdentityImpl; import org.springframework.security.acls.domain.PrincipalSid; import org.springframework.security.acls.domain.SpringCacheBasedAclCache; +import org.springframework.security.acls.model.AccessControlEntry; import org.springframework.security.acls.model.Acl; import org.springframework.security.acls.model.AuditableAccessControlEntry; import org.springframework.security.acls.model.MutableAcl; @@ -320,6 +321,21 @@ public void testCreateGrantedAuthority() { assertThat(((GrantedAuthoritySid) result).getGrantedAuthority()).isEqualTo("sid"); } + // gh-19127 + @Test + public void readAclsByIdWhenChildLoadedThenAcesPointToConvertedAcl() { + ObjectIdentity topParentOid = new ObjectIdentityImpl(TARGET_CLASS, 100L); + ObjectIdentity middleParentOid = new ObjectIdentityImpl(TARGET_CLASS, 101L); + ObjectIdentity childOid = new ObjectIdentityImpl(TARGET_CLASS, 102L); + Map map = this.strategy + .readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null); + for (Acl acl : map.values()) { + for (AccessControlEntry ace : acl.getEntries()) { + assertThat(ace.getAcl()).isSameAs(acl); + } + } + } + @Test public void setObjectIdentityGeneratorWhenNullThenThrowsIllegalArgumentException() { // @formatter:off