You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2026/08/14 13:51:49 UTC
(struts) branch WW-5675-share-parsed-ognl-security-config created (now 571014c98)
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a change to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
at 571014c98 WW-5675 fix(ognl): move the allowlist union onto the config bean
This branch includes the following new commits:
new b10154e17 WW-5675 docs(ognl): add design for sharing parsed OGNL security config
new 6216411b4 WW-5675 docs(ognl): record that bootstrapFactories is on the production path
new e91a14870 WW-5675 docs(ognl): settle the dev-mode setter removal as decided
new d8a4e3003 WW-5675 docs(ognl): add implementation plan for sharing parsed security config
new 66113f81d WW-5675 docs(ognl): fix Task 5 to handle the tests the signature change breaks
new 75b162f96 WW-5675 perf(config): hoist the whitespace pattern in validatePackageNames
new 32ceb1924 WW-5675 feat(ognl): add a container-singleton OGNL security config bean
new cac69c15c WW-5675 perf(ognl): share parsed config across SecurityMemberAccess instances
new 02fa6aa9e WW-5675 test(ognl): strengthen SecurityMemberAccessConfigSharingTest assertions
new cb3dff2ff WW-5675 refactor(ognl): drop the lazy dev-mode flip from the access path
new cb162c631 WW-5675 fix(ognl): register SecurityMemberAccessConfig for the Dispatcher container
new 9b1eb89be WW-5675 docs(ognl): correct the wiring claim the full-suite run disproved
new ae41c5a8d WW-5675 test(ognl): cover the production registration of the config bean
new 6028e9dc8 WW-5675 perf(ognl): precompute the allowlist package union
new 571014c98 WW-5675 fix(ognl): move the allowlist union onto the config bean
The 15 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(struts) 03/15: WW-5675 docs(ognl): settle the dev-mode setter removal as decided
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit e91a14870290f870ab503e763b0cd3fcbcb66763
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 13:51:27 2026 +0200
WW-5675 docs(ognl): settle the dev-mode setter removal as decided
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...security-member-access-config-sharing-design.md | 33 ++++++++++++----------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
index 3523beb01..50ecb2962 100644
--- a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
+++ b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
@@ -4,8 +4,6 @@
**Target:** 7.4.0
**Date:** 2026-08-14
**Status:** Design approved, pending implementation plan
-**Open decision:** whether the five dev-mode setters are deleted or deprecated — see "`SecurityMemberAccess`
-changes"
## Problem
@@ -44,7 +42,8 @@ default configuration. Hoist it to a static constant.
- Parse the OGNL security configuration once per container instead of once per `SecurityMemberAccess`.
- Preserve OGNL allow/deny semantics exactly. No configuration may become more permissive.
- Keep source compatibility for 7.4.0: existing subclasses and direct setter callers must continue to compile and
- behave identically.
+ behave identically. The five dev-mode setters are the one signed-off exception — see "`SecurityMemberAccess`
+ changes".
- Collapse the two-set allowlist walk introduced by WW-5674 into a single precomputed set.
## Non-goals
@@ -157,20 +156,21 @@ which seeds its fields by copying immutable set references — no parsing, no `H
**Field added:** `allowlistPackageNamesUnion`.
-The five dev-mode setters are proposed for **deletion outright rather than deprecation**. This is a deliberate
-deviation from the "additive and deprecate, no breakage in a minor" policy chosen for the rest of this change, and
-needs explicit sign-off.
+The five dev-mode setters are **deleted outright rather than deprecated** — decided 2026-08-14. This is a
+deliberate, signed-off deviation from the "additive and deprecate, no breakage in a minor" policy that governs the
+rest of this change.
-The case for deleting them: they are `public`, but only ever container-injected, with no direct caller anywhere in
-core, plugins, or tests. Preserving them faithfully would mean keeping `isDevMode` plus the four dev-mode set
-fields on the instance and reinstating some form of the lazy flip — that is, keeping precisely the code this
-change exists to delete, to serve a caller that does not demonstrably exist.
+They are `public`, but only ever container-injected, with no direct caller anywhere in core, plugins, or tests.
+Preserving them faithfully would mean keeping `isDevMode` plus the four dev-mode set fields on the instance and
+reinstating some form of the lazy flip — that is, keeping precisely the code this change exists to delete, to
+serve a caller that does not demonstrably exist. Retention in simplified form was rejected because today's
+semantics are subtle enough that any simplification would silently change them: a manual
+`useDevModeExcludedClasses` call accumulates into the dev-mode set, which then *replaces* — rather than unions
+with — `excludedClasses` on first access.
-The case against: they are public methods on a user-overridable bean, so a deployment could in principle call
-them, and removal in a minor release would break it at compile time. Deprecating them while preserving exact
-semantics is not cheap, because today's semantics are subtle — a manual `useDevModeExcludedClasses` call
-accumulates into the dev-mode set, which then *replaces* (not unions with) `excludedClasses` on first access. Any
-simplified retention would silently change that.
+The accepted risk is that a deployment calling these methods directly breaks at compile time on upgrade to 7.4.0.
+This is a loud, immediate failure with an obvious fix, not a silent behavioural change, which is what makes it
+acceptable where the constructor break discussed below was not.
The remaining eleven configuration setters stay as `@Deprecated` methods with their `@Inject` annotations removed.
They keep mutating that instance exactly as they do now. Deprecation is by annotation only — no runtime warnings,
@@ -298,3 +298,6 @@ OGNL allow/deny semantics are unchanged.
free here by the union collapse. The remaining visibility narrowing stays with that ticket.
- **WW-5667** — the parent should be updated to note that this ticket, not WW-5674, is the one expected to move
the reported 9%.
+- **Migration guide entry for 7.4.0** — the removal of the five dev-mode setters is a source-breaking change in a
+ minor release and must be called out in the Version Notes and Migration Guide, however narrow the affected
+ audience.
(struts) 12/15: WW-5675 docs(ognl): correct the wiring claim the full-suite run disproved
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 9b1eb89be1bfa018e48cb95ec4f5be30ff97f1e1
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:45:23 2026 +0200
WW-5675 docs(ognl): correct the wiring claim the full-suite run disproved
Dispatcher installs its own provider list and never adds
StrutsDefaultConfigurationProvider, so bootstrapFactories is not on the
production path. The bean needs registering in struts-beans.xml too,
matching ProviderAllowlist and ThreadAllowlist.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...security-member-access-config-sharing-design.md | 37 ++++++++++++++++------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
index 50ecb2962..319ce54ec 100644
--- a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
+++ b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
@@ -86,16 +86,33 @@ Concrete class, no interface, **not** aliased in `StrutsBeanSelectionProvider`.
the shape of `ProviderAllowlist` and `ThreadAllowlist` (`DefaultConfiguration.java:418-419`), not a user extension
point.
-`bootstrapFactories` is on the production path, not test-only: `ConfigurationManager.addDefaultContainerProviders`
-(`ConfigurationManager.java:94`) registers `StrutsDefaultConfigurationProvider`, which calls it at
-`StrutsDefaultConfigurationProvider.java:116`, and `Dispatcher` drives `ConfigurationManager`. It reads as
-test-oriented in a grep only because a dozen tests name the provider explicitly and `XWorkTestCaseHelper` — test
-scaffolding that lives in `core/src/main` — registers it too.
-
-The method serves both the bootstrap container (`DefaultConfiguration.java:360`) and the main container, so each
-gets its own configuration singleton. The bootstrap container carries only `BOOTSTRAP_CONSTANTS`, so most security
-constants are absent there, the `required = false` setters do not fire, and the bean falls back to defaults —
-exactly as a `SecurityMemberAccess` constructed in that container behaves today.
+**The bean must be registered in two places.** An earlier draft of this design claimed `bootstrapFactories` was on
+the production path because `ConfigurationManager.addDefaultContainerProviders` (`ConfigurationManager.java:94`)
+registers `StrutsDefaultConfigurationProvider`, which calls it at
+`StrutsDefaultConfigurationProvider.java:116`. **That claim is wrong**, and it was only caught when the full core
+suite failed with 1579 errors during implementation.
+
+`ConfigurationManager.addDefaultContainerProviders()` fires only when `containerProviders.isEmpty()`
+(`ConfigurationManager.java:78-80`). `Dispatcher.init()` (`Dispatcher.java:711-719`) installs its own provider
+list — including `StrutsBeanSelectionProvider` via `init_AliasStandardObjects` — so the list is never empty and
+`StrutsDefaultConfigurationProvider` is never added. The production container is built from
+`StrutsBeanSelectionProvider` plus `struts-beans.xml`, and `bootstrapFactories` is not on that path at all.
+
+The registration therefore goes in both places, which is precisely what `ProviderAllowlist` and `ThreadAllowlist`
+already do — `DefaultConfiguration.java:418-419` and `struts-beans.xml:175-176`:
+
+```xml
+ <bean class="org.apache.struts2.ognl.SecurityMemberAccessConfig"/>
+```
+
+The `DefaultConfiguration` registration serves the bootstrap container (`DefaultConfiguration.java:360`) and the
+`XWorkTestCase` harness; the `struts-beans.xml` entry serves the real Dispatcher container. The bootstrap
+container carries only `BOOTSTRAP_CONSTANTS`, so most security constants are absent there, the
+`required = false` setters do not fire, and the bean falls back to defaults — exactly as a `SecurityMemberAccess`
+constructed in that container behaves today.
+
+This failure mode is loud, not silent: `useConfig` is a mandatory `@Inject`, so a container missing the binding
+throws at build time rather than running with empty exclusions.
The `TODO: SpringObjectFactoryTest fails when these are SINGLETON` comment at the top of `bootstrapFactories`
applies to the `*Factory` beans in the first block, not to this region, where singletons are already the norm.
(struts) 10/15: WW-5675 refactor(ognl): drop the lazy dev-mode flip from the access path
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit cb3dff2ff2df3e375dbb2832de88f0d5be0ded61
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:40:01 2026 +0200
WW-5675 refactor(ognl): drop the lazy dev-mode flip from the access path
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../apache/struts2/ognl/SecurityMemberAccess.java | 45 ----------------------
.../SecurityMemberAccessConfigSharingTest.java | 25 ++++++++++++
2 files changed, 25 insertions(+), 45 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
index 8ba19f540..b7ed69caa 100644
--- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
@@ -86,13 +86,6 @@ public class SecurityMemberAccess implements MemberAccess {
private Set<String> excludedPackageNames = emptySet();
private Set<String> excludedPackageExemptClasses = emptySet();
- private volatile boolean isDevModeInit;
- private boolean isDevMode;
- private Set<String> devModeExcludedClasses = Set.of(Object.class.getName());
- private Set<Pattern> devModeExcludedPackageNamePatterns = emptySet();
- private Set<String> devModeExcludedPackageNames = emptySet();
- private Set<String> devModeExcludedPackageExemptClasses = emptySet();
-
private boolean enforceAllowlistEnabled = false;
private Set<Class<?>> allowlistClasses = emptySet();
private Set<String> allowlistPackageNames = emptySet();
@@ -283,7 +276,6 @@ public class SecurityMemberAccess implements MemberAccess {
* @return {@code true} if member access is allowed
*/
protected boolean checkExclusionList(Object target, Member member) {
- useDevModeConfiguration();
Class<?> memberClass = member.getDeclaringClass();
if (isClassExcluded(memberClass)) {
LOG.warn("Declaring class of member type [{}] is excluded!", memberClass);
@@ -612,41 +604,4 @@ public class SecurityMemberAccess implements MemberAccess {
this.disallowDefaultPackageAccess = BooleanUtils.toBoolean(disallowDefaultPackageAccess);
}
- @Inject(StrutsConstants.STRUTS_DEVMODE)
- protected void useDevMode(String devMode) {
- this.isDevMode = BooleanUtils.toBoolean(devMode);
- }
-
- @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false)
- public void useDevModeExcludedClasses(String commaDelimitedClasses) {
- this.devModeExcludedClasses = toNewClassesSet(devModeExcludedClasses, commaDelimitedClasses);
- }
-
- @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
- public void useDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
- this.devModeExcludedPackageNamePatterns = toNewPatternsSet(devModeExcludedPackageNamePatterns, commaDelimitedPackagePatterns);
- }
-
- @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false)
- public void useDevModeExcludedPackageNames(String commaDelimitedPackageNames) {
- this.devModeExcludedPackageNames = toNewPackageNamesSet(devModeExcludedPackageNames, commaDelimitedPackageNames);
- }
-
- @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
- public void useDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) {
- this.devModeExcludedPackageExemptClasses = toClassesSet(commaDelimitedClasses);
- }
-
- private void useDevModeConfiguration() {
- if (!isDevMode || isDevModeInit) {
- return;
- }
- logWarningForFirstOccurrence("devMode", LOG,
- "DevMode enabled, using DevMode excluded classes and packages for OGNL security enforcement!");
- isDevModeInit = true;
- excludedClasses = devModeExcludedClasses;
- excludedPackageNamePatterns = devModeExcludedPackageNamePatterns;
- excludedPackageNames = devModeExcludedPackageNames;
- excludedPackageExemptClasses = devModeExcludedPackageExemptClasses;
- }
}
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
index 5680e59fb..eb09c9fb4 100644
--- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
@@ -148,4 +148,29 @@ public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
super(providerAllowlist, threadAllowlist);
}
}
+
+ /**
+ * Dev-mode exclusions must be in force from the first access, with no lazy flip.
+ */
+ public void testDevModeExclusionsApplyWithoutAnAccess() throws Exception {
+ loadButSet(Map.of(
+ StrutsConstants.STRUTS_DEVMODE, "true",
+ StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, "java.lang.ProcessBuilder"));
+
+ SecurityMemberAccess sma = container.getInstance(SecurityMemberAccess.class);
+ Set<String> excluded = SecurityMemberAccessTest.reflectField(sma, "excludedClasses");
+
+ assertTrue("dev-mode exclusions were not applied at startup",
+ excluded.contains("java.lang.ProcessBuilder"));
+ }
+
+ public void testDevModeMethodsAreGone() throws Exception {
+ for (String name : new String[]{"useDevMode", "useDevModeExcludedClasses",
+ "useDevModeExcludedPackageNamePatterns", "useDevModeExcludedPackageNames",
+ "useDevModeExcludedPackageExemptClasses", "useDevModeConfiguration"}) {
+ for (java.lang.reflect.Method method : SecurityMemberAccess.class.getDeclaredMethods()) {
+ assertFalse("SecurityMemberAccess still declares " + name, method.getName().equals(name));
+ }
+ }
+ }
}
(struts) 13/15: WW-5675 test(ognl): cover the production registration of the config bean
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit ae41c5a8dbbcceac4f9af820f67c2022435a7fea
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:53:44 2026 +0200
WW-5675 test(ognl): cover the production registration of the config bean
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...mberAccessConfigProductionRegistrationTest.java | 43 ++++++++++++++++++++++
.../SecurityMemberAccessConfigSharingTest.java | 6 +++
2 files changed, 49 insertions(+)
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigProductionRegistrationTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigProductionRegistrationTest.java
new file mode 100644
index 000000000..cd1018d50
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigProductionRegistrationTest.java
@@ -0,0 +1,43 @@
+/*
+ * 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
+ *
+ * http://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.apache.struts2.ognl;
+
+import org.apache.struts2.StrutsInternalTestCase;
+
+/**
+ * Covers the {@code struts-beans.xml} registration of {@link SecurityMemberAccessConfig}, which
+ * {@link SecurityMemberAccessConfigSharingTest} cannot: that test extends {@link org.apache.struts2.XWorkTestCase}
+ * directly, whose container is built from {@code StrutsDefaultConfigurationProvider} alone and never loads
+ * {@code struts-beans.xml}. Production, via {@link org.apache.struts2.dispatcher.Dispatcher#init()}, never adds
+ * that provider and relies entirely on the {@code struts-beans.xml} entry.
+ * <p>
+ * {@link StrutsInternalTestCase} boots a real {@link org.apache.struts2.dispatcher.Dispatcher}, so its container
+ * is wired the way production's is. Without this test, the singleton scope of the {@code struts-beans.xml}
+ * entry — the entire point of WW-5675 sharing parsed configuration across {@link SecurityMemberAccess}
+ * instances — could regress to {@code scope="prototype"} with the whole suite staying green.
+ */
+public class SecurityMemberAccessConfigProductionRegistrationTest extends StrutsInternalTestCase {
+
+ public void testConfigBeanIsASingletonInTheProductionContainer() {
+ SecurityMemberAccessConfig first = container.getInstance(SecurityMemberAccessConfig.class);
+ assertNotNull("SecurityMemberAccessConfig is not registered in the production container", first);
+ assertSame("SecurityMemberAccessConfig is not a singleton in the production container",
+ first, container.getInstance(SecurityMemberAccessConfig.class));
+ }
+}
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
index eb09c9fb4..ffce445f8 100644
--- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
@@ -162,6 +162,12 @@ public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
assertTrue("dev-mode exclusions were not applied at startup",
excluded.contains("java.lang.ProcessBuilder"));
+ // The `contains` check above is non-vacuous only because the test container does not load
+ // struts-excluded-classes.xml, where java.lang.ProcessBuilder happens to sit in both the
+ // production and dev-mode excluded-classes sets. Asserting identity with the config bean's
+ // set keeps this test meaningful even if the harness starts loading that file.
+ assertSame("excludedClasses was not seeded from the dev-mode-resolved config",
+ container.getInstance(SecurityMemberAccessConfig.class).getExcludedClasses(), excluded);
}
public void testDevModeMethodsAreGone() throws Exception {
(struts) 02/15: WW-5675 docs(ognl): record that bootstrapFactories is on the production path
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 6216411b4def47b1ac76e374dab0f3db29d1e782
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 13:49:00 2026 +0200
WW-5675 docs(ognl): record that bootstrapFactories is on the production path
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...WW-5675-security-member-access-config-sharing-design.md | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
index b5435fe33..3523beb01 100644
--- a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
+++ b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
@@ -87,6 +87,20 @@ Concrete class, no interface, **not** aliased in `StrutsBeanSelectionProvider`.
the shape of `ProviderAllowlist` and `ThreadAllowlist` (`DefaultConfiguration.java:418-419`), not a user extension
point.
+`bootstrapFactories` is on the production path, not test-only: `ConfigurationManager.addDefaultContainerProviders`
+(`ConfigurationManager.java:94`) registers `StrutsDefaultConfigurationProvider`, which calls it at
+`StrutsDefaultConfigurationProvider.java:116`, and `Dispatcher` drives `ConfigurationManager`. It reads as
+test-oriented in a grep only because a dozen tests name the provider explicitly and `XWorkTestCaseHelper` — test
+scaffolding that lives in `core/src/main` — registers it too.
+
+The method serves both the bootstrap container (`DefaultConfiguration.java:360`) and the main container, so each
+gets its own configuration singleton. The bootstrap container carries only `BOOTSTRAP_CONSTANTS`, so most security
+constants are absent there, the `required = false` setters do not fire, and the bean falls back to defaults —
+exactly as a `SecurityMemberAccess` constructed in that container behaves today.
+
+The `TODO: SpringObjectFactoryTest fails when these are SINGLETON` comment at the top of `bootstrapFactories`
+applies to the `*Factory` beans in the first block, not to this region, where singletons are already the norm.
+
It takes over these sixteen `@Inject` setters from `SecurityMemberAccess`:
| Setter | Constant |
(struts) 04/15: WW-5675 docs(ognl): add implementation plan for sharing parsed security config
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit d8a4e3003cbb45ff0f7d0cbc58cdad3d174da9ce
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 13:56:51 2026 +0200
WW-5675 docs(ognl): add implementation plan for sharing parsed security config
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...14-WW-5675-share-parsed-ognl-security-config.md | 1096 ++++++++++++++++++++
1 file changed, 1096 insertions(+)
diff --git a/docs/superpowers/plans/2026-08-14-WW-5675-share-parsed-ognl-security-config.md b/docs/superpowers/plans/2026-08-14-WW-5675-share-parsed-ognl-security-config.md
new file mode 100644
index 000000000..4ce8dbc8e
--- /dev/null
+++ b/docs/superpowers/plans/2026-08-14-WW-5675-share-parsed-ognl-security-config.md
@@ -0,0 +1,1096 @@
+# WW-5675 Share Parsed OGNL Security Configuration — Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Parse the OGNL security configuration once per container instead of once per `SecurityMemberAccess` instantiation, without changing OGNL allow/deny semantics.
+
+**Architecture:** A new `Scope.SINGLETON` bean, `SecurityMemberAccessConfig`, takes over all sixteen `@Inject` configuration setters and does all parsing once per container, resolving dev-mode in `Initializable.init()`. `SecurityMemberAccess` stays `Scope.PROTOTYPE` and receives that bean through a single `@Inject` setter, copying immutable set references. The dev-mode lazy flip is deleted from the access path, and the allowlist two-set walk collapses into one precomputed union.
+
+**Tech Stack:** Java 17 (`maven.compiler.release=17`), Maven, JUnit 4 (`org.junit.Test`), AssertJ, Mockito, Log4j2, Caffeine.
+
+**Spec:** `docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md`
+
+## Global Constraints
+
+- **Branch:** `WW-5675-share-parsed-ognl-security-config`. Never push to `main`; finish via a PR.
+- **Commit format:** `WW-5675 <type>(<scope>): <description>`, e.g. `WW-5675 perf(ognl): share parsed config across instances`. Every commit ends with the `Co-Authored-By: Claude Opus 5 <no...@anthropic.com>` trailer.
+- **Never `git add -A` or `git add .`** in this repo — the tree carries roughly twenty long-lived untracked files. Stage explicit paths and verify with `git diff --cached --name-only` before every commit.
+- **Core tests are JUnit 4** (`org.junit.Test`, `org.junit.Before`) or extend `XWorkTestCase`. A JUnit 5 `@Test` added to these suites silently never runs.
+- **OGNL allow/deny semantics must not change.** No configuration may become more permissive. This is a security gate.
+- **Test command:** `mvn test -DskipAssembly -pl core -Dtest=ClassName#methodName`
+- **Full module suite:** `mvn test -DskipAssembly -pl core`
+- Target version 7.4.0.
+
+---
+
+## File Structure
+
+| File | Responsibility |
+|---|---|
+| `core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java` | Modify: hoist the whitespace `Pattern` to a constant |
+| `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java` | **Create**: owns all config parsing and dev-mode resolution, one instance per container |
+| `core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java` | Modify: register the new bean as `Scope.SINGLETON` |
+| `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java` | Modify: receive config by setter, deprecate eleven setters, delete dev-mode state, collapse the allowlist union |
+| `core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java` | Test: whitespace validation behaviour preserved |
+| `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java` | **Create**: differential parsing + dev-mode resolution |
+| `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java` | **Create**: sharing proof, instance isolation, subclass injection |
+
+---
+
+### Task 1: Hoist the whitespace pattern in `ConfigParseUtil`
+
+`validatePackageNames` currently calls `Pattern.compile("\\s")` once per package name — roughly 58 recompiles of a trivial pattern per `SecurityMemberAccess` instantiation under the default configuration.
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java:142-146`
+- Test: `core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java`
+
+**Interfaces:**
+- Consumes: nothing.
+- Produces: no signature change. `public static void validatePackageNames(Collection<String> packageNames)` keeps its exact behaviour and throws `ConfigurationException` on any whitespace.
+
+- [ ] **Step 1: Write the failing test**
+
+Create `core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java` if it does not exist; otherwise append these methods to the existing class. If creating it, use the standard ASF license header copied verbatim from `ConfigParseUtil.java` lines 1-18.
+
+```java
+package org.apache.struts2.util;
+
+import org.apache.struts2.config.ConfigurationException;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Set;
+
+import static org.junit.Assert.assertThrows;
+
+public class ConfigParseUtilTest {
+
+ @Test
+ public void validatePackageNamesAcceptsNamesWithoutWhitespace() {
+ ConfigParseUtil.validatePackageNames(Set.of("java.lang", "org.apache.struts2", ""));
+ }
+
+ @Test
+ public void validatePackageNamesRejectsSpace() {
+ assertThrows(ConfigurationException.class,
+ () -> ConfigParseUtil.validatePackageNames(Set.of("java.lang", "org.apache struts2")));
+ }
+
+ @Test
+ public void validatePackageNamesRejectsTab() {
+ assertThrows(ConfigurationException.class,
+ () -> ConfigParseUtil.validatePackageNames(Set.of("java\tlang")));
+ }
+
+ @Test
+ public void validatePackageNamesRejectsNewline() {
+ assertThrows(ConfigurationException.class,
+ () -> ConfigParseUtil.validatePackageNames(Set.of("java\nlang")));
+ }
+
+ @Test
+ public void validatePackageNamesAcceptsEmptyCollection() {
+ ConfigParseUtil.validatePackageNames(List.of());
+ }
+}
+```
+
+- [ ] **Step 2: Run the tests to verify they pass against the current implementation**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ConfigParseUtilTest`
+Expected: PASS. These tests characterise existing behaviour before the refactor — they are the guard, not a red test. Do not proceed if any fails; that would mean the characterisation is wrong.
+
+- [ ] **Step 3: Hoist the pattern**
+
+In `core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java`, add the constant next to the existing cache constants near line 45:
+
+```java
+ private static final Pattern WHITESPACE = Pattern.compile("\\s");
+```
+
+Then replace the body of `validatePackageNames`:
+
+```java
+ public static void validatePackageNames(Collection<String> packageNames) {
+ if (packageNames.stream().anyMatch(s -> WHITESPACE.matcher(s).find())) {
+ throw new ConfigurationException("Excluded package names could not be parsed due to erroneous whitespace characters: " + packageNames);
+ }
+ }
+```
+
+- [ ] **Step 4: Run the tests again**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ConfigParseUtilTest`
+Expected: PASS, identical results to Step 2.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java
+git diff --cached --name-only
+git commit -m "WW-5675 perf(config): hoist the whitespace pattern in validatePackageNames
+
+Co-Authored-By: Claude Opus 5 <no...@anthropic.com>"
+```
+
+---
+
+### Task 2: Create the `SecurityMemberAccessConfig` bean
+
+A standalone bean that owns all parsing. It does not touch `SecurityMemberAccess` yet, so it is independently testable.
+
+**Files:**
+- Create: `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java`
+- Test: `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java`
+
+**Interfaces:**
+- Consumes: `ConfigParseUtil.toClassesSet`, `toClassObjectsSet`, `toNewClassesSet`, `toNewPatternsSet`, `toNewPackageNamesSet`, `toPackageNamesSet` (all `public static`, unchanged).
+- Produces, relied on by Task 3:
+ - `boolean isAllowStaticFieldAccess()`
+ - `Set<String> getExcludedClasses()`
+ - `Set<Pattern> getExcludedPackageNamePatterns()`
+ - `Set<String> getExcludedPackageNames()`
+ - `Set<String> getExcludedPackageExemptClasses()`
+ - `boolean isEnforceAllowlistEnabled()`
+ - `Set<Class<?>> getAllowlistClasses()`
+ - `Set<String> getAllowlistPackageNames()`
+ - `boolean isDisallowProxyObjectAccess()`
+ - `boolean isDisallowProxyMemberAccess()`
+ - `boolean isDisallowDefaultPackageAccess()`
+
+ The four excluded-* getters return the **effective** sets, with dev-mode already applied.
+
+- [ ] **Step 1: Write the failing test**
+
+Create `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java` with the ASF license header copied verbatim from `SecurityMemberAccess.java` lines 1-18.
+
+The `legacy*` methods below are **frozen oracles**: verbatim copies of the accumulation logic they replace. They must never be deleted, nor rewritten to delegate to production code — that would make the differential vacuous. This mirrors the approach used in `SecurityMemberAccessPackageMatchingTest` for WW-5674.
+
+```java
+package org.apache.struts2.ognl;
+
+import org.junit.Test;
+
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import static org.apache.struts2.util.ConfigParseUtil.toNewClassesSet;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class SecurityMemberAccessConfigTest {
+
+ /**
+ * Frozen oracle: the accumulation SecurityMemberAccess performed before WW-5675.
+ * Never delete this, and never make it delegate to production code.
+ */
+ private static Set<String> legacyExcludedClassAccumulation(boolean allowStaticFieldAccess, String configured) {
+ Set<String> excludedClasses = Set.of(Object.class.getName());
+ if (!allowStaticFieldAccess) {
+ excludedClasses = toNewClassesSet(excludedClasses, Class.class.getName());
+ }
+ return toNewClassesSet(excludedClasses, configured);
+ }
+
+ private SecurityMemberAccessConfig configWith(boolean devMode, String excludedClasses, String devModeExcludedClasses) {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useDevMode(String.valueOf(devMode));
+ config.useExcludedClasses(excludedClasses);
+ config.useDevModeExcludedClasses(devModeExcludedClasses);
+ config.init();
+ return config;
+ }
+
+ @Test
+ public void excludedClassesMatchLegacyAccumulation() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder");
+ config.init();
+
+ assertEquals(legacyExcludedClassAccumulation(true, "java.lang.Runtime,java.lang.ProcessBuilder"),
+ config.getExcludedClasses());
+ }
+
+ @Test
+ public void disallowingStaticFieldAccessAddsClassToExclusions() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useAllowStaticFieldAccess("false");
+ config.useExcludedClasses("java.lang.Runtime");
+ config.init();
+
+ assertFalse(config.isAllowStaticFieldAccess());
+ assertEquals(legacyExcludedClassAccumulation(false, "java.lang.Runtime"), config.getExcludedClasses());
+ }
+
+ /**
+ * The container iterates getDeclaredMethods(), whose order the JDK leaves unspecified.
+ * The accumulation must therefore be commutative, as it was before WW-5675.
+ */
+ @Test
+ public void setterOrderDoesNotAffectExcludedClasses() {
+ SecurityMemberAccessConfig forward = new SecurityMemberAccessConfig();
+ forward.useAllowStaticFieldAccess("false");
+ forward.useExcludedClasses("java.lang.Runtime");
+ forward.init();
+
+ SecurityMemberAccessConfig reverse = new SecurityMemberAccessConfig();
+ reverse.useExcludedClasses("java.lang.Runtime");
+ reverse.useAllowStaticFieldAccess("false");
+ reverse.init();
+
+ assertEquals(forward.getExcludedClasses(), reverse.getExcludedClasses());
+ }
+
+ @Test
+ public void devModeDisabledPublishesNormalExclusions() {
+ SecurityMemberAccessConfig config = configWith(false, "java.lang.Runtime", "java.lang.ProcessBuilder");
+
+ assertTrue(config.getExcludedClasses().contains("java.lang.Runtime"));
+ assertFalse(config.getExcludedClasses().contains("java.lang.ProcessBuilder"));
+ }
+
+ @Test
+ public void devModeEnabledPublishesDevModeExclusions() {
+ SecurityMemberAccessConfig config = configWith(true, "java.lang.Runtime", "java.lang.ProcessBuilder");
+
+ assertTrue(config.getExcludedClasses().contains("java.lang.ProcessBuilder"));
+ assertFalse(config.getExcludedClasses().contains("java.lang.Runtime"));
+ }
+
+ @Test
+ public void packageNamesAreStrippedOfDots() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useExcludedPackageNames("java.io.,.java.net");
+ config.init();
+
+ assertTrue(config.getExcludedPackageNames().contains("java.io"));
+ assertTrue(config.getExcludedPackageNames().contains("java.net"));
+ }
+
+ @Test
+ public void patternsAreCompiledOnce() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useExcludedPackageNamePatterns("^java\\.lang\\..*");
+ config.init();
+
+ Set<Pattern> patterns = config.getExcludedPackageNamePatterns();
+ assertEquals(1, patterns.size());
+ assertTrue(patterns.iterator().next().matcher("java.lang.Runtime").matches());
+ }
+
+ /**
+ * A missing init() must fail closed: production exclusions, never the dev-mode ones.
+ */
+ @Test
+ public void withoutInitTheNormalExclusionsApply() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useDevMode("true");
+ config.useExcludedClasses("java.lang.Runtime");
+ config.useDevModeExcludedClasses("java.lang.ProcessBuilder");
+
+ assertTrue(config.getExcludedClasses().contains("java.lang.Runtime"));
+ }
+}
+```
+
+- [ ] **Step 2: Run the test to verify it fails**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessConfigTest`
+Expected: FAIL — compilation error, `SecurityMemberAccessConfig` does not exist.
+
+- [ ] **Step 3: Create the bean**
+
+Create `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java` with the ASF license header copied verbatim from `SecurityMemberAccess.java` lines 1-18.
+
+Note that `init()` overwrites the normal fields with the dev-mode ones, exactly mirroring the `useDevModeConfiguration()` method it replaces. This is deliberate: if `init()` never runs, the normal production exclusions remain in force, which fails closed.
+
+```java
+package org.apache.struts2.ognl;
+
+import org.apache.commons.lang3.BooleanUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.inject.Inject;
+import org.apache.struts2.inject.Initializable;
+
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import static java.util.Collections.emptySet;
+import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES;
+import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES;
+import static org.apache.struts2.util.ConfigParseUtil.toClassObjectsSet;
+import static org.apache.struts2.util.ConfigParseUtil.toClassesSet;
+import static org.apache.struts2.util.ConfigParseUtil.toNewClassesSet;
+import static org.apache.struts2.util.ConfigParseUtil.toNewPackageNamesSet;
+import static org.apache.struts2.util.ConfigParseUtil.toNewPatternsSet;
+import static org.apache.struts2.util.ConfigParseUtil.toPackageNamesSet;
+import static org.apache.struts2.util.DebugUtils.logWarningForFirstOccurrence;
+
+/**
+ * Holds the parsed OGNL security configuration for one container.
+ * <p>
+ * {@link SecurityMemberAccess} is a {@code Scope.PROTOTYPE} bean, constructed once per value stack and
+ * again for each OGNL context. Parsing the roughly ninety configuration entries on every one of those
+ * was the dominant cost identified by WW-5667. This bean is a {@code Scope.SINGLETON}, so the parsing
+ * happens once per container and each {@code SecurityMemberAccess} merely copies immutable references.
+ * <p>
+ * Dev-mode is resolved in {@link #init()} rather than in a setter, because the container iterates
+ * {@code getDeclaredMethods()}, whose order the JDK leaves unspecified. If {@code init()} never runs,
+ * the normal production exclusions stay in force, which fails closed.
+ *
+ * @since Struts 7.4.0
+ */
+public class SecurityMemberAccessConfig implements Initializable {
+
+ private static final Logger LOG = LogManager.getLogger(SecurityMemberAccessConfig.class);
+
+ private boolean allowStaticFieldAccess = true;
+
+ private Set<String> excludedClasses = Set.of(Object.class.getName());
+ private Set<Pattern> excludedPackageNamePatterns = emptySet();
+ private Set<String> excludedPackageNames = emptySet();
+ private Set<String> excludedPackageExemptClasses = emptySet();
+
+ private boolean isDevMode;
+ private Set<String> devModeExcludedClasses = Set.of(Object.class.getName());
+ private Set<Pattern> devModeExcludedPackageNamePatterns = emptySet();
+ private Set<String> devModeExcludedPackageNames = emptySet();
+ private Set<String> devModeExcludedPackageExemptClasses = emptySet();
+
+ private boolean enforceAllowlistEnabled = false;
+ private Set<Class<?>> allowlistClasses = emptySet();
+ private Set<String> allowlistPackageNames = emptySet();
+
+ private boolean disallowProxyObjectAccess = false;
+ private boolean disallowProxyMemberAccess = false;
+ private boolean disallowDefaultPackageAccess = false;
+
+ @Override
+ public void init() {
+ if (!isDevMode) {
+ return;
+ }
+ logWarningForFirstOccurrence("devMode", LOG,
+ "DevMode enabled, using DevMode excluded classes and packages for OGNL security enforcement!");
+ excludedClasses = devModeExcludedClasses;
+ excludedPackageNamePatterns = devModeExcludedPackageNamePatterns;
+ excludedPackageNames = devModeExcludedPackageNames;
+ excludedPackageExemptClasses = devModeExcludedPackageExemptClasses;
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, required = false)
+ public void useAllowStaticFieldAccess(String allowStaticFieldAccess) {
+ this.allowStaticFieldAccess = BooleanUtils.toBoolean(allowStaticFieldAccess);
+ if (!this.allowStaticFieldAccess) {
+ useExcludedClasses(Class.class.getName());
+ }
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_EXCLUDED_CLASSES, required = false)
+ public void useExcludedClasses(String commaDelimitedClasses) {
+ this.excludedClasses = toNewClassesSet(excludedClasses, commaDelimitedClasses);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
+ public void useExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
+ this.excludedPackageNamePatterns = toNewPatternsSet(excludedPackageNamePatterns, commaDelimitedPackagePatterns);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, required = false)
+ public void useExcludedPackageNames(String commaDelimitedPackageNames) {
+ this.excludedPackageNames = toNewPackageNamesSet(excludedPackageNames, commaDelimitedPackageNames);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
+ public void useExcludedPackageExemptClasses(String commaDelimitedClasses) {
+ this.excludedPackageExemptClasses = toClassesSet(commaDelimitedClasses);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_ALLOWLIST_ENABLE, required = false)
+ public void useEnforceAllowlistEnabled(String enforceAllowlistEnabled) {
+ this.enforceAllowlistEnabled = BooleanUtils.toBoolean(enforceAllowlistEnabled);
+ if (!this.enforceAllowlistEnabled) {
+ String msg = "OGNL allowlist is disabled!" +
+ " We strongly recommend keeping it enabled to protect against critical vulnerabilities." +
+ " Set the configuration `{}=true` to enable it." +
+ " Please refer to the Struts 7.0 migration guide and security documentation for further information.";
+ logWarningForFirstOccurrence("allowlist", LOG, msg, StrutsConstants.STRUTS_ALLOWLIST_ENABLE);
+ }
+ }
+
+ @Inject(value = STRUTS_ALLOWLIST_CLASSES, required = false)
+ public void useAllowlistClasses(String commaDelimitedClasses) {
+ this.allowlistClasses = toClassObjectsSet(commaDelimitedClasses);
+ }
+
+ @Inject(value = STRUTS_ALLOWLIST_PACKAGE_NAMES, required = false)
+ public void useAllowlistPackageNames(String commaDelimitedPackageNames) {
+ this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, required = false)
+ public void useDisallowProxyObjectAccess(String disallowProxyObjectAccess) {
+ this.disallowProxyObjectAccess = BooleanUtils.toBoolean(disallowProxyObjectAccess);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, required = false)
+ public void useDisallowProxyMemberAccess(String disallowProxyMemberAccess) {
+ this.disallowProxyMemberAccess = BooleanUtils.toBoolean(disallowProxyMemberAccess);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS, required = false)
+ public void useDisallowDefaultPackageAccess(String disallowDefaultPackageAccess) {
+ this.disallowDefaultPackageAccess = BooleanUtils.toBoolean(disallowDefaultPackageAccess);
+ }
+
+ @Inject(StrutsConstants.STRUTS_DEVMODE)
+ public void useDevMode(String devMode) {
+ this.isDevMode = BooleanUtils.toBoolean(devMode);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false)
+ public void useDevModeExcludedClasses(String commaDelimitedClasses) {
+ this.devModeExcludedClasses = toNewClassesSet(devModeExcludedClasses, commaDelimitedClasses);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
+ public void useDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
+ this.devModeExcludedPackageNamePatterns = toNewPatternsSet(devModeExcludedPackageNamePatterns, commaDelimitedPackagePatterns);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false)
+ public void useDevModeExcludedPackageNames(String commaDelimitedPackageNames) {
+ this.devModeExcludedPackageNames = toNewPackageNamesSet(devModeExcludedPackageNames, commaDelimitedPackageNames);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
+ public void useDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) {
+ this.devModeExcludedPackageExemptClasses = toClassesSet(commaDelimitedClasses);
+ }
+
+ public boolean isAllowStaticFieldAccess() {
+ return allowStaticFieldAccess;
+ }
+
+ public Set<String> getExcludedClasses() {
+ return excludedClasses;
+ }
+
+ public Set<Pattern> getExcludedPackageNamePatterns() {
+ return excludedPackageNamePatterns;
+ }
+
+ public Set<String> getExcludedPackageNames() {
+ return excludedPackageNames;
+ }
+
+ public Set<String> getExcludedPackageExemptClasses() {
+ return excludedPackageExemptClasses;
+ }
+
+ public boolean isEnforceAllowlistEnabled() {
+ return enforceAllowlistEnabled;
+ }
+
+ public Set<Class<?>> getAllowlistClasses() {
+ return allowlistClasses;
+ }
+
+ public Set<String> getAllowlistPackageNames() {
+ return allowlistPackageNames;
+ }
+
+ public boolean isDisallowProxyObjectAccess() {
+ return disallowProxyObjectAccess;
+ }
+
+ public boolean isDisallowProxyMemberAccess() {
+ return disallowProxyMemberAccess;
+ }
+
+ public boolean isDisallowDefaultPackageAccess() {
+ return disallowDefaultPackageAccess;
+ }
+}
+```
+
+- [ ] **Step 4: Run the test to verify it passes**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessConfigTest`
+Expected: PASS, 9 tests.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java
+git diff --cached --name-only
+git commit -m "WW-5675 feat(ognl): add a container-singleton OGNL security config bean
+
+Co-Authored-By: Claude Opus 5 <no...@anthropic.com>"
+```
+
+---
+
+### Task 3: Register the bean and wire it into `SecurityMemberAccess`
+
+`SecurityMemberAccess` starts reading the shared configuration. Its own setters lose `@Inject` and become deprecated, but keep mutating the instance so the roughly 110 existing direct call sites behave identically.
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java:416-420`
+- Modify: `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java:473-536`
+- Test: `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java`
+
+**Interfaces:**
+- Consumes: every getter from Task 2.
+- Produces: `public void useConfig(SecurityMemberAccessConfig config)` on `SecurityMemberAccess`, annotated `@Inject`. Task 4 removes the dev-mode setters; Task 5 changes the allowlist walk.
+
+- [ ] **Step 1: Write the failing test**
+
+Create `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java` with the ASF license header copied verbatim from `SecurityMemberAccess.java` lines 1-18.
+
+`XWorkTestCase` lives in `core/src/main/java/org/apache/struts2/XWorkTestCase.java` and exposes `protected Container container`. It is a JUnit 3 style `TestCase`, so test methods must be named `testXxx` — an `@Test` annotation alone will not run them.
+
+```java
+package org.apache.struts2.ognl;
+
+import org.apache.struts2.XWorkTestCase;
+
+import java.util.Set;
+
+public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
+
+ /**
+ * Reference identity proves no re-parsing occurred: any re-parse necessarily
+ * allocates a fresh set.
+ */
+ public void testConfigDerivedSetsAreSharedAcrossInstances() throws Exception {
+ SecurityMemberAccess first = container.getInstance(SecurityMemberAccess.class);
+ SecurityMemberAccess second = container.getInstance(SecurityMemberAccess.class);
+
+ assertNotSame("expected a prototype bean", first, second);
+
+ Set<String> firstExcluded = SecurityMemberAccessTest.reflectField(first, "excludedClasses");
+ Set<String> secondExcluded = SecurityMemberAccessTest.reflectField(second, "excludedClasses");
+ assertSame("excluded classes were re-parsed per instance", firstExcluded, secondExcluded);
+
+ Set<String> firstPackages = SecurityMemberAccessTest.reflectField(first, "excludedPackageNames");
+ Set<String> secondPackages = SecurityMemberAccessTest.reflectField(second, "excludedPackageNames");
+ assertSame("excluded package names were re-parsed per instance", firstPackages, secondPackages);
+ }
+
+ public void testConfigBeanIsASingleton() {
+ assertSame(container.getInstance(SecurityMemberAccessConfig.class),
+ container.getInstance(SecurityMemberAccessConfig.class));
+ }
+
+ /**
+ * The shared sets must not be perturbed by a deprecated setter call on one instance.
+ */
+ public void testDeprecatedSetterDoesNotLeakToSiblings() throws Exception {
+ SecurityMemberAccess mutated = container.getInstance(SecurityMemberAccess.class);
+ SecurityMemberAccess untouched = container.getInstance(SecurityMemberAccess.class);
+ SecurityMemberAccessConfig config = container.getInstance(SecurityMemberAccessConfig.class);
+
+ Set<String> before = SecurityMemberAccessTest.reflectField(untouched, "excludedClasses");
+ mutated.useExcludedClasses("java.lang.Runtime");
+ Set<String> after = SecurityMemberAccessTest.reflectField(untouched, "excludedClasses");
+
+ assertSame("a sibling instance was affected", before, after);
+ assertFalse("the shared config was mutated", config.getExcludedClasses().contains("java.lang.Runtime"));
+
+ Set<String> mutatedSet = SecurityMemberAccessTest.reflectField(mutated, "excludedClasses");
+ assertTrue("the setter did not affect its own instance", mutatedSet.contains("java.lang.Runtime"));
+ }
+
+ /**
+ * Guards the fail-open hole avoided by using setter rather than constructor injection:
+ * a subclass calling the two-argument super constructor must still receive the config.
+ */
+ public void testSubclassReceivesConfigThroughInheritedSetter() throws Exception {
+ SubclassedSecurityMemberAccess subclassed = new SubclassedSecurityMemberAccess(
+ container.getInstance(ProviderAllowlist.class),
+ container.getInstance(ThreadAllowlist.class));
+
+ container.inject(subclassed);
+
+ Set<String> excluded = SecurityMemberAccessTest.reflectField(subclassed, "excludedClasses");
+ assertSame("subclass did not receive the shared config",
+ container.getInstance(SecurityMemberAccessConfig.class).getExcludedClasses(), excluded);
+ }
+
+ static class SubclassedSecurityMemberAccess extends SecurityMemberAccess {
+ SubclassedSecurityMemberAccess(ProviderAllowlist providerAllowlist, ThreadAllowlist threadAllowlist) {
+ super(providerAllowlist, threadAllowlist);
+ }
+ }
+}
+```
+
+`Container.inject(Object)` is declared at `core/src/main/java/org/apache/struts2/inject/Container.java:83`, so the call above drives the real injection path — including `ContainerImpl.addInjectors`, which recurses into superclasses at `ContainerImpl.java:97`. That recursion is exactly what this test exists to protect.
+
+- [ ] **Step 2: Run the test to verify it fails**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessConfigSharingTest`
+Expected: FAIL — `SecurityMemberAccessConfig` is not registered in the container, and the sets are still parsed per instance so `assertSame` fails.
+
+- [ ] **Step 3: Register the bean**
+
+In `core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java`, inside `bootstrapFactories`, add the registration immediately after the `ThreadAllowlist` line:
+
+```java
+ .factory(ProviderAllowlist.class, Scope.SINGLETON)
+ .factory(ThreadAllowlist.class, Scope.SINGLETON)
+ .factory(SecurityMemberAccessConfig.class, Scope.SINGLETON)
+```
+
+Add the import alongside the existing OGNL imports near line 123:
+
+```java
+import org.apache.struts2.ognl.SecurityMemberAccessConfig;
+```
+
+- [ ] **Step 4: Add the config setter to `SecurityMemberAccess`**
+
+In `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java`, add this method immediately after `setProxyService` (around line 113):
+
+```java
+ /**
+ * Copies the shared, already-parsed configuration into this instance. This is the only injected
+ * member that touches the configuration fields, so the unspecified order in which the container
+ * iterates {@code getDeclaredMethods()} cannot affect the result.
+ *
+ * @since Struts 7.4.0
+ */
+ @Inject
+ public void useConfig(SecurityMemberAccessConfig config) {
+ this.allowStaticFieldAccess = config.isAllowStaticFieldAccess();
+ this.excludedClasses = config.getExcludedClasses();
+ this.excludedPackageNamePatterns = config.getExcludedPackageNamePatterns();
+ this.excludedPackageNames = config.getExcludedPackageNames();
+ this.excludedPackageExemptClasses = config.getExcludedPackageExemptClasses();
+ this.enforceAllowlistEnabled = config.isEnforceAllowlistEnabled();
+ this.allowlistClasses = config.getAllowlistClasses();
+ this.allowlistPackageNames = config.getAllowlistPackageNames();
+ this.disallowProxyObjectAccess = config.isDisallowProxyObjectAccess();
+ this.disallowProxyMemberAccess = config.isDisallowProxyMemberAccess();
+ this.disallowDefaultPackageAccess = config.isDisallowDefaultPackageAccess();
+ }
+```
+
+- [ ] **Step 5: Deprecate the eleven remaining setters**
+
+Still in `SecurityMemberAccess.java`, for each of these methods remove the `@Inject(...)` annotation and add `@Deprecated` plus a Javadoc `@deprecated` tag. Leave every method body exactly as it is.
+
+Apply to: `useAllowStaticFieldAccess`, `useExcludedClasses`, `useExcludedPackageNamePatterns`, `useExcludedPackageNames`, `useExcludedPackageExemptClasses`, `useEnforceAllowlistEnabled`, `useAllowlistClasses`, `useAllowlistPackageNames`, `useDisallowProxyObjectAccess`, `useDisallowProxyMemberAccess`, `useDisallowDefaultPackageAccess`.
+
+The pattern for each, shown for `useExcludedClasses`:
+
+```java
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
+ public void useExcludedClasses(String commaDelimitedClasses) {
+ this.excludedClasses = toNewClassesSet(excludedClasses, commaDelimitedClasses);
+ }
+```
+
+Do **not** annotate `useDevMode` or the four `useDevModeExcluded*` methods — Task 4 deletes those. Do **not** touch `useAcceptProperties` or `useExcludeProperties`; they carry per-request state, not configuration, and are not deprecated.
+
+Deprecation is by annotation only. Do not add runtime warnings — roughly 110 test call sites would flood the build output.
+
+- [ ] **Step 6: Run the new test**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessConfigSharingTest`
+Expected: PASS, 4 tests.
+
+- [ ] **Step 7: Run the existing security suites**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest='SecurityMemberAccessTest,ExternalSecurityMemberAccessTest,OgnlUtilTest,OgnlValueStackTest'`
+Expected: PASS. Surefire needs comma separation; `+` is not valid.
+
+If `SecurityMemberAccessTest` fails, the deprecated setters have not kept their exact semantics — fix the setter, not the test.
+
+- [ ] **Step 8: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
+git diff --cached --name-only
+git commit -m "WW-5675 perf(ognl): share parsed config across SecurityMemberAccess instances
+
+Co-Authored-By: Claude Opus 5 <no...@anthropic.com>"
+```
+
+---
+
+### Task 4: Delete the dev-mode state from `SecurityMemberAccess`
+
+The lazy dev-mode flip runs on the access path today. With dev-mode resolved once by the config bean, all of it goes.
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java:89-94, 264, 538-574`
+- Test: `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java`
+
+**Interfaces:**
+- Consumes: `SecurityMemberAccessConfig` getters, which already publish dev-mode-resolved sets.
+- Produces: removal only. `useDevMode`, `useDevModeExcludedClasses`, `useDevModeExcludedPackageNamePatterns`, `useDevModeExcludedPackageNames`, `useDevModeExcludedPackageExemptClasses` and `useDevModeConfiguration` no longer exist on `SecurityMemberAccess`.
+
+- [ ] **Step 1: Write the failing test**
+
+Append to `SecurityMemberAccessConfigSharingTest`:
+
+```java
+ /**
+ * Dev-mode exclusions must be in force from the first access, with no lazy flip.
+ */
+ public void testDevModeExclusionsApplyWithoutAnAccess() throws Exception {
+ loadConfigurationProviders(new StubConfigurationProvider() {
+ @Override
+ public void register(ContainerBuilder builder, LocatableProperties props) {
+ props.setProperty(StrutsConstants.STRUTS_DEVMODE, "true");
+ props.setProperty(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, "java.lang.ProcessBuilder");
+ }
+ });
+
+ SecurityMemberAccess sma = container.getInstance(SecurityMemberAccess.class);
+ Set<String> excluded = SecurityMemberAccessTest.reflectField(sma, "excludedClasses");
+
+ assertTrue("dev-mode exclusions were not applied at startup",
+ excluded.contains("java.lang.ProcessBuilder"));
+ }
+
+ public void testDevModeMethodsAreGone() throws Exception {
+ for (String name : new String[]{"useDevMode", "useDevModeExcludedClasses",
+ "useDevModeExcludedPackageNamePatterns", "useDevModeExcludedPackageNames",
+ "useDevModeExcludedPackageExemptClasses", "useDevModeConfiguration"}) {
+ for (java.lang.reflect.Method method : SecurityMemberAccess.class.getDeclaredMethods()) {
+ assertFalse("SecurityMemberAccess still declares " + name, method.getName().equals(name));
+ }
+ }
+ }
+```
+
+Add these imports to the test file:
+
+```java
+import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.inject.ContainerBuilder;
+import org.apache.struts2.test.StubConfigurationProvider;
+import org.apache.struts2.util.location.LocatableProperties;
+```
+
+Note `LocatableProperties` is in `org.apache.struts2.util.location`, not `org.apache.struts2.config.entities`.
+
+- [ ] **Step 2: Run the test to verify it fails**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessConfigSharingTest#testDevModeMethodsAreGone`
+Expected: FAIL — the methods still exist.
+
+- [ ] **Step 3: Delete the dev-mode state**
+
+In `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java`:
+
+Delete these fields (lines 89-94):
+
+```java
+ private volatile boolean isDevModeInit;
+ private boolean isDevMode;
+ private Set<String> devModeExcludedClasses = Set.of(Object.class.getName());
+ private Set<Pattern> devModeExcludedPackageNamePatterns = emptySet();
+ private Set<String> devModeExcludedPackageNames = emptySet();
+ private Set<String> devModeExcludedPackageExemptClasses = emptySet();
+```
+
+Delete the `useDevModeConfiguration()` method entirely, and its call at the top of `checkExclusionList` so that the method begins:
+
+```java
+ protected boolean checkExclusionList(Object target, Member member) {
+ Class<?> memberClass = member.getDeclaringClass();
+```
+
+Delete the five dev-mode setters: `useDevMode`, `useDevModeExcludedClasses`, `useDevModeExcludedPackageNamePatterns`, `useDevModeExcludedPackageNames`, `useDevModeExcludedPackageExemptClasses`.
+
+- [ ] **Step 4: Run the tests**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessConfigSharingTest`
+Expected: PASS, 6 tests.
+
+- [ ] **Step 5: Run the existing security suites**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest='SecurityMemberAccessTest,ExternalSecurityMemberAccessTest'`
+Expected: PASS. If a test called a dev-mode setter directly, the earlier survey was wrong — stop and report rather than deleting the test.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
+git diff --cached --name-only
+git commit -m "WW-5675 refactor(ognl): drop the lazy dev-mode flip from the access path
+
+Co-Authored-By: Claude Opus 5 <no...@anthropic.com>"
+```
+
+---
+
+### Task 5: Collapse the allowlist two-set walk into one precomputed union
+
+WW-5674 merged the two allowlist walks with a three-argument helper. With the configuration shared, the union can be precomputed, so the helper reverts to a single set.
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java:257-263, 392-441`
+- Test: `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java`
+
+**Interfaces:**
+- Consumes: `SecurityMemberAccessConfig.getAllowlistPackageNames()`.
+- Produces: `static boolean isPackageBelongsToPackages(String packageName, Set<String> matchingPackages)` — two arguments, replacing the three-argument form. The three-argument `isClassBelongsToPackages(Class, Set, Set)` overload is deleted.
+
+- [ ] **Step 1: Write the failing test**
+
+Append to `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java`. Do not modify or delete the frozen oracles already in that file.
+
+```java
+ /**
+ * The union must never lose ALLOWLIST_REQUIRED_PACKAGES. Dropping them would be a silent
+ * fail-open: Struts' own components would stop being allowlisted with nothing failing loudly.
+ */
+ @Test
+ public void allowlistUnionRetainsRequiredPackagesAfterSetterCall() throws Exception {
+ SecurityMemberAccess sma = new SecurityMemberAccess(null, null);
+ sma.useAllowlistPackageNames("com.example.app");
+
+ Set<String> union = SecurityMemberAccessTest.reflectField(sma, "allowlistPackageNamesUnion");
+
+ assertTrue("configured package missing", union.contains("com.example.app"));
+ assertTrue("required Struts package dropped from the allowlist",
+ union.contains("org.apache.struts2.components"));
+ }
+
+ @Test
+ public void allowlistUnionContainsRequiredPackagesByDefault() throws Exception {
+ SecurityMemberAccess sma = new SecurityMemberAccess(null, null);
+
+ Set<String> union = SecurityMemberAccessTest.reflectField(sma, "allowlistPackageNamesUnion");
+
+ assertTrue(union.contains("org.apache.struts2.components"));
+ assertTrue(union.contains("org.apache.struts2.views.jsp"));
+ assertTrue(union.contains("org.apache.struts2.validator.validators"));
+ }
+
+ @Test
+ public void singleSetWalkMatchesTheFrozenOracle() {
+ for (String packageName : PACKAGE_NAMES) {
+ for (Set<String> candidates : CANDIDATE_SETS) {
+ assertEquals("mismatch for " + packageName + " against " + candidates,
+ legacyPrefixMatch(packageName, candidates),
+ SecurityMemberAccess.isPackageBelongsToPackages(packageName, candidates));
+ }
+ }
+ }
+```
+
+- [ ] **Step 2: Run the test to verify it fails**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessPackageMatchingTest`
+Expected: FAIL — `allowlistPackageNamesUnion` does not exist and `isPackageBelongsToPackages` still takes three arguments.
+
+- [ ] **Step 3: Add the union field and its single computation site**
+
+In `SecurityMemberAccess.java`, replace the allowlist field declarations (around line 97-98):
+
+```java
+ private Set<Class<?>> allowlistClasses = emptySet();
+ private Set<String> allowlistPackageNames = emptySet();
+ private Set<String> allowlistPackageNamesUnion = ALLOWLIST_REQUIRED_PACKAGES;
+```
+
+The union defaults to `ALLOWLIST_REQUIRED_PACKAGES` so that an instance which never receives configuration — the eight direct `new SecurityMemberAccess(null, null)` test constructions — still allowlists Struts' own packages, exactly as before.
+
+Add the single computation site and its helper:
+
+```java
+ /**
+ * The only place the allowlist union is computed. Both the injected configuration and the
+ * deprecated setter route through here; splitting this in two would risk silently dropping
+ * {@code ALLOWLIST_REQUIRED_PACKAGES}, which fails open.
+ */
+ private void applyAllowlistPackageNames(Set<String> packageNames) {
+ this.allowlistPackageNames = packageNames;
+ this.allowlistPackageNamesUnion = union(ALLOWLIST_REQUIRED_PACKAGES, packageNames);
+ }
+
+ private static Set<String> union(Set<String> required, Set<String> configured) {
+ if (configured.isEmpty()) {
+ return required;
+ }
+ Set<String> union = new HashSet<>(required);
+ union.addAll(configured);
+ return unmodifiableSet(union);
+ }
+```
+
+Add these imports:
+
+```java
+import java.util.HashSet;
+
+import static java.util.Collections.unmodifiableSet;
+```
+
+- [ ] **Step 4: Route both callers through it**
+
+In `useConfig`, replace the direct assignment:
+
+```java
+ this.allowlistPackageNames = config.getAllowlistPackageNames();
+```
+
+with:
+
+```java
+ applyAllowlistPackageNames(config.getAllowlistPackageNames());
+```
+
+In the deprecated `useAllowlistPackageNames`, replace the body:
+
+```java
+ @Deprecated
+ public void useAllowlistPackageNames(String commaDelimitedPackageNames) {
+ applyAllowlistPackageNames(toPackageNamesSet(commaDelimitedPackageNames));
+ }
+```
+
+- [ ] **Step 5: Collapse the walk**
+
+Replace the last clause of `isClassAllowlisted`:
+
+```java
+ || isClassBelongsToPackages(clazz, allowlistPackageNamesUnion);
+```
+
+Delete the three-argument `isClassBelongsToPackages(Class, Set, Set)` overload entirely, and rewrite the remaining pair:
+
+```java
+ public static boolean isClassBelongsToPackages(Class<?> clazz, Set<String> matchingPackages) {
+ return isPackageBelongsToPackages(toPackageName(clazz), matchingPackages);
+ }
+
+ /**
+ * Tests whether the given package name, or any of its parent packages, is present in the set.
+ * Walks the name in place rather than building the full prefix list, since this runs on the OGNL
+ * member-access path. Shortest prefix first, so broad entries such as {@code java.io}
+ * short-circuit earliest.
+ *
+ * <p>
+ * The package name must not end in {@code '.'}. Such a name is probed one prefix more than by the
+ * implementation this replaced, which matches more broadly — tightening exclusion but
+ * <em>loosening</em> the allowlist. {@link Class#getPackageName()} cannot produce a trailing dot,
+ * and {@code ConfigParseUtil.toPackageNamesSet} strips them from configured names, so every
+ * current caller is safe; route any other string through here only after confirming the same.
+ *
+ * @param packageName the package name to test, empty for the default package, never ending in {@code '.'}
+ * @param matchingPackages the package names to match against
+ * @return {@code true} if the package or any parent package is in the set
+ */
+ static boolean isPackageBelongsToPackages(String packageName, Set<String> matchingPackages) {
+ if (matchingPackages.isEmpty()) {
+ return false;
+ }
+ int idx = packageName.indexOf('.');
+ while (idx != -1) {
+ if (matchingPackages.contains(packageName.substring(0, idx))) {
+ return true;
+ }
+ idx = packageName.indexOf('.', idx + 1);
+ }
+ return matchingPackages.contains(packageName);
+ }
+```
+
+Remove the now-unused `emptySet` static import only if no other usage remains — check with `grep -n "emptySet" core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java`.
+
+- [ ] **Step 6: Run the package-matching tests**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessPackageMatchingTest`
+Expected: PASS, 12 tests.
+
+- [ ] **Step 7: Run the security suites**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest='SecurityMemberAccessTest,ExternalSecurityMemberAccessTest,SecurityMemberAccessConfigSharingTest,SecurityMemberAccessConfigTest'`
+Expected: PASS.
+
+- [ ] **Step 8: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java
+git diff --cached --name-only
+git commit -m "WW-5675 perf(ognl): precompute the allowlist package union
+
+Co-Authored-By: Claude Opus 5 <no...@anthropic.com>"
+```
+
+---
+
+### Task 6: Verify the whole module and the plugins
+
+**Files:** none modified unless a failure is found.
+
+**Interfaces:** none.
+
+- [ ] **Step 1: Run the full core suite**
+
+Run: `mvn test -DskipAssembly -pl core`
+Expected: PASS with zero failures and zero errors. For reference, the suite stood at 3158 tests when WW-5674 merged; this plan adds roughly 15.
+
+Record the actual counts. Do not describe the work as complete without this output in hand.
+
+- [ ] **Step 2: Run the plugin suites that touch `SecurityMemberAccess`**
+
+Run: `mvn test -DskipAssembly -pl plugins/spring,plugins/cdi`
+Expected: PASS. Both modules construct `new SecurityMemberAccess(null, null)` directly in proxy tests, which exercises the un-injected path.
+
+- [ ] **Step 3: Confirm no stray `@Inject` remains on the deprecated setters**
+
+Run:
+
+```bash
+grep -n -B2 "public void use" core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java | grep -A2 "@Inject"
+```
+
+Expected: only `useConfig` and `setProxyService` appear. Any other hit means a setter kept its annotation and will still be injected per instance, silently defeating the change.
+
+- [ ] **Step 4: Confirm the dev-mode state is gone**
+
+Run:
+
+```bash
+grep -n "devMode\|DevMode" core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
+```
+
+Expected: no matches.
+
+- [ ] **Step 5: Commit any fixes, then push and open a draft PR**
+
+Only if Steps 1-4 are clean:
+
+```bash
+git push -u origin WW-5675-share-parsed-ognl-security-config
+```
+
+Open a draft PR titled `WW-5675 Share parsed OGNL security configuration across SecurityMemberAccess instances`, with `Fixes [WW-5675](https://issues.apache.org/jira/browse/WW-5675)` in the description.
+
+The PR description must state plainly that removing the five dev-mode setters is a source-breaking change in a minor release, why it was accepted, and that a Migration Guide entry is owed for 7.4.0.
+
+---
+
+## Follow-ups (not part of this plan)
+
+- File a Jira Improvement for 8.0.0 to remove the eleven deprecated setters, cross-referencing WW-5675 and WW-5678.
+- Add the Version Notes and Migration Guide entry for the dev-mode setter removal.
+- Update WW-5667 to record that this ticket, not WW-5674, is the one expected to move the reported 9%.
+- WW-5678's first item is resolved here by Task 5; the remaining visibility narrowing stays with that ticket.
(struts) 08/15: WW-5675 perf(ognl): share parsed config across SecurityMemberAccess instances
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit cac69c15c303a7d684d2007eaac45f800716b999
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:17:07 2026 +0200
WW-5675 perf(ognl): share parsed config across SecurityMemberAccess instances
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../struts2/config/impl/DefaultConfiguration.java | 2 +
.../apache/struts2/ognl/SecurityMemberAccess.java | 99 +++++++++++++++++++---
.../SecurityMemberAccessConfigSharingTest.java | 91 ++++++++++++++++++++
3 files changed, 181 insertions(+), 11 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java
index dcf4f1602..53bf20dc8 100644
--- a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java
+++ b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java
@@ -121,6 +121,7 @@ import org.apache.struts2.conversion.StrutsTypeConverterHolder;
import org.apache.struts2.factory.StrutsResultFactory;
import org.apache.struts2.ognl.OgnlGuard;
import org.apache.struts2.ognl.ProviderAllowlist;
+import org.apache.struts2.ognl.SecurityMemberAccessConfig;
import org.apache.struts2.ognl.StrutsOgnlGuard;
import org.apache.struts2.ognl.ThreadAllowlist;
@@ -417,6 +418,7 @@ public class DefaultConfiguration implements Configuration {
.factory(OgnlGuard.class, StrutsOgnlGuard.class, Scope.SINGLETON)
.factory(ProviderAllowlist.class, Scope.SINGLETON)
.factory(ThreadAllowlist.class, Scope.SINGLETON)
+ .factory(SecurityMemberAccessConfig.class, Scope.SINGLETON)
.factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON);
}
diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
index badad3dee..8ba19f540 100644
--- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
@@ -112,6 +112,28 @@ public class SecurityMemberAccess implements MemberAccess {
this.proxyService = proxyService;
}
+ /**
+ * Copies the shared, already-parsed configuration into this instance. This is the only injected
+ * member that touches the configuration fields, so the unspecified order in which the container
+ * iterates {@code getDeclaredMethods()} cannot affect the result.
+ *
+ * @since Struts 7.4.0
+ */
+ @Inject
+ public void useConfig(SecurityMemberAccessConfig config) {
+ this.allowStaticFieldAccess = config.isAllowStaticFieldAccess();
+ this.excludedClasses = config.getExcludedClasses();
+ this.excludedPackageNamePatterns = config.getExcludedPackageNamePatterns();
+ this.excludedPackageNames = config.getExcludedPackageNames();
+ this.excludedPackageExemptClasses = config.getExcludedPackageExemptClasses();
+ this.enforceAllowlistEnabled = config.isEnforceAllowlistEnabled();
+ this.allowlistClasses = config.getAllowlistClasses();
+ this.allowlistPackageNames = config.getAllowlistPackageNames();
+ this.disallowProxyObjectAccess = config.isDisallowProxyObjectAccess();
+ this.disallowProxyMemberAccess = config.isDisallowProxyMemberAccess();
+ this.disallowDefaultPackageAccess = config.isDisallowDefaultPackageAccess();
+ }
+
@Override
public Object setup(OgnlContext context, Object target, Member member, String propertyName) {
Object result = null;
@@ -470,7 +492,12 @@ public class SecurityMemberAccess implements MemberAccess {
this.acceptProperties = acceptedProperties;
}
- @Inject(value = StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useAllowStaticFieldAccess(String allowStaticFieldAccess) {
this.allowStaticFieldAccess = BooleanUtils.toBoolean(allowStaticFieldAccess);
if (!this.allowStaticFieldAccess) {
@@ -478,27 +505,52 @@ public class SecurityMemberAccess implements MemberAccess {
}
}
- @Inject(value = StrutsConstants.STRUTS_EXCLUDED_CLASSES, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useExcludedClasses(String commaDelimitedClasses) {
this.excludedClasses = toNewClassesSet(excludedClasses, commaDelimitedClasses);
}
- @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
this.excludedPackageNamePatterns = toNewPatternsSet(excludedPackageNamePatterns, commaDelimitedPackagePatterns);
}
- @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useExcludedPackageNames(String commaDelimitedPackageNames) {
this.excludedPackageNames = toNewPackageNamesSet(excludedPackageNames, commaDelimitedPackageNames);
}
- @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useExcludedPackageExemptClasses(String commaDelimitedClasses) {
this.excludedPackageExemptClasses = toClassesSet(commaDelimitedClasses);
}
- @Inject(value = StrutsConstants.STRUTS_ALLOWLIST_ENABLE, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useEnforceAllowlistEnabled(String enforceAllowlistEnabled) {
this.enforceAllowlistEnabled = BooleanUtils.toBoolean(enforceAllowlistEnabled);
if (!this.enforceAllowlistEnabled) {
@@ -510,27 +562,52 @@ public class SecurityMemberAccess implements MemberAccess {
}
}
- @Inject(value = STRUTS_ALLOWLIST_CLASSES, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useAllowlistClasses(String commaDelimitedClasses) {
this.allowlistClasses = toClassObjectsSet(commaDelimitedClasses);
}
- @Inject(value = STRUTS_ALLOWLIST_PACKAGE_NAMES, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useAllowlistPackageNames(String commaDelimitedPackageNames) {
this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames);
}
- @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useDisallowProxyObjectAccess(String disallowProxyObjectAccess) {
this.disallowProxyObjectAccess = BooleanUtils.toBoolean(disallowProxyObjectAccess);
}
- @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useDisallowProxyMemberAccess(String disallowProxyMemberAccess) {
this.disallowProxyMemberAccess = BooleanUtils.toBoolean(disallowProxyMemberAccess);
}
- @Inject(value = StrutsConstants.STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS, required = false)
+ /**
+ * @deprecated since 7.4.0, configuration is parsed once per container by
+ * {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
+ * tests and existing callers; it will be removed in Struts 8.0.0.
+ */
+ @Deprecated
public void useDisallowDefaultPackageAccess(String disallowDefaultPackageAccess) {
this.disallowDefaultPackageAccess = BooleanUtils.toBoolean(disallowDefaultPackageAccess);
}
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
new file mode 100644
index 000000000..b6af1db29
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
@@ -0,0 +1,91 @@
+/*
+ * 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
+ *
+ * http://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.apache.struts2.ognl;
+
+import org.apache.struts2.XWorkTestCase;
+
+import java.util.Set;
+
+public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
+
+ /**
+ * Reference identity proves no re-parsing occurred: any re-parse necessarily
+ * allocates a fresh set.
+ */
+ public void testConfigDerivedSetsAreSharedAcrossInstances() throws Exception {
+ SecurityMemberAccess first = container.getInstance(SecurityMemberAccess.class);
+ SecurityMemberAccess second = container.getInstance(SecurityMemberAccess.class);
+
+ assertNotSame("expected a prototype bean", first, second);
+
+ Set<String> firstExcluded = SecurityMemberAccessTest.reflectField(first, "excludedClasses");
+ Set<String> secondExcluded = SecurityMemberAccessTest.reflectField(second, "excludedClasses");
+ assertSame("excluded classes were re-parsed per instance", firstExcluded, secondExcluded);
+
+ Set<String> firstPackages = SecurityMemberAccessTest.reflectField(first, "excludedPackageNames");
+ Set<String> secondPackages = SecurityMemberAccessTest.reflectField(second, "excludedPackageNames");
+ assertSame("excluded package names were re-parsed per instance", firstPackages, secondPackages);
+ }
+
+ public void testConfigBeanIsASingleton() {
+ assertSame(container.getInstance(SecurityMemberAccessConfig.class),
+ container.getInstance(SecurityMemberAccessConfig.class));
+ }
+
+ /**
+ * The shared sets must not be perturbed by a deprecated setter call on one instance.
+ */
+ public void testDeprecatedSetterDoesNotLeakToSiblings() throws Exception {
+ SecurityMemberAccess mutated = container.getInstance(SecurityMemberAccess.class);
+ SecurityMemberAccess untouched = container.getInstance(SecurityMemberAccess.class);
+ SecurityMemberAccessConfig config = container.getInstance(SecurityMemberAccessConfig.class);
+
+ Set<String> before = SecurityMemberAccessTest.reflectField(untouched, "excludedClasses");
+ mutated.useExcludedClasses("java.lang.Runtime");
+ Set<String> after = SecurityMemberAccessTest.reflectField(untouched, "excludedClasses");
+
+ assertSame("a sibling instance was affected", before, after);
+ assertFalse("the shared config was mutated", config.getExcludedClasses().contains("java.lang.Runtime"));
+
+ Set<String> mutatedSet = SecurityMemberAccessTest.reflectField(mutated, "excludedClasses");
+ assertTrue("the setter did not affect its own instance", mutatedSet.contains("java.lang.Runtime"));
+ }
+
+ /**
+ * Guards the fail-open hole avoided by using setter rather than constructor injection:
+ * a subclass calling the two-argument super constructor must still receive the config.
+ */
+ public void testSubclassReceivesConfigThroughInheritedSetter() throws Exception {
+ SubclassedSecurityMemberAccess subclassed = new SubclassedSecurityMemberAccess(
+ container.getInstance(ProviderAllowlist.class),
+ container.getInstance(ThreadAllowlist.class));
+
+ container.inject(subclassed);
+
+ Set<String> excluded = SecurityMemberAccessTest.reflectField(subclassed, "excludedClasses");
+ assertSame("subclass did not receive the shared config",
+ container.getInstance(SecurityMemberAccessConfig.class).getExcludedClasses(), excluded);
+ }
+
+ static class SubclassedSecurityMemberAccess extends SecurityMemberAccess {
+ SubclassedSecurityMemberAccess(ProviderAllowlist providerAllowlist, ThreadAllowlist threadAllowlist) {
+ super(providerAllowlist, threadAllowlist);
+ }
+ }
+}
(struts) 15/15: WW-5675 fix(ognl): move the allowlist union onto the config bean
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 571014c98ccc6d8f6141a35c60fc9c5abef364b9
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 15:21:40 2026 +0200
WW-5675 fix(ognl): move the allowlist union onto the config bean
Final-review cleanup: SecurityMemberAccess.applyAllowlistPackageNames was
still allocating a fresh HashSet per instantiation, landing precisely on
deployments that configure struts.allowlist.packageNames. Move
ALLOWLIST_REQUIRED_PACKAGES and union(...) onto SecurityMemberAccessConfig,
which now precomputes allowlistPackageNamesUnion once per container;
useConfig copies the reference, and the deprecated setter path reuses the
same static union() method, so there remains exactly one computation site.
Also: mark the eleven deprecated SecurityMemberAccess setters with
since/forRemoval per repo convention, document union()'s Set.of(...)
aliasing contract, pin allowlistPackageNamesUnion into the immutability and
dev-mode-field-removal tests, restore alphabetical import order in
ConfigParseUtilTest, switch the sharing test off the Map.of ten-pair
ceiling, and correct the design doc's bootstrap-container wiring claim and
drop its unimplemented counting-probe promise.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../apache/struts2/ognl/SecurityMemberAccess.java | 55 ++++++++------------
.../struts2/ognl/SecurityMemberAccessConfig.java | 41 +++++++++++++++
.../SecurityMemberAccessConfigSharingTest.java | 44 ++++++++++------
.../ognl/SecurityMemberAccessConfigTest.java | 21 ++++++++
.../struts2/ognl/SecurityMemberAccessTest.java | 1 +
.../apache/struts2/util/ConfigParseUtilTest.java | 2 +-
...security-member-access-config-sharing-design.md | 58 +++++++++++++++++-----
7 files changed, 159 insertions(+), 63 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
index 80c36f05c..fcc337a63 100644
--- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
@@ -32,14 +32,12 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Modifier;
-import java.util.HashSet;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static java.text.MessageFormat.format;
import static java.util.Collections.emptySet;
-import static java.util.Collections.unmodifiableSet;
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES;
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES;
import static org.apache.struts2.util.ConfigParseUtil.toClassObjectsSet;
@@ -58,12 +56,6 @@ public class SecurityMemberAccess implements MemberAccess {
private static final Logger LOG = LogManager.getLogger(SecurityMemberAccess.class);
- private static final Set<String> ALLOWLIST_REQUIRED_PACKAGES = Set.of(
- "org.apache.struts2.validator.validators",
- "org.apache.struts2.components",
- "org.apache.struts2.views.jsp"
- );
-
private static final Set<Class<?>> ALLOWLIST_REQUIRED_CLASSES = Set.of(
java.lang.Enum.class,
java.lang.String.class,
@@ -91,7 +83,7 @@ public class SecurityMemberAccess implements MemberAccess {
private boolean enforceAllowlistEnabled = false;
private Set<Class<?>> allowlistClasses = emptySet();
private Set<String> allowlistPackageNames = emptySet();
- private Set<String> allowlistPackageNamesUnion = ALLOWLIST_REQUIRED_PACKAGES;
+ private Set<String> allowlistPackageNamesUnion = SecurityMemberAccessConfig.ALLOWLIST_REQUIRED_PACKAGES;
private boolean disallowProxyObjectAccess = false;
private boolean disallowProxyMemberAccess = false;
@@ -124,29 +116,24 @@ public class SecurityMemberAccess implements MemberAccess {
this.excludedPackageExemptClasses = config.getExcludedPackageExemptClasses();
this.enforceAllowlistEnabled = config.isEnforceAllowlistEnabled();
this.allowlistClasses = config.getAllowlistClasses();
- applyAllowlistPackageNames(config.getAllowlistPackageNames());
+ this.allowlistPackageNames = config.getAllowlistPackageNames();
+ this.allowlistPackageNamesUnion = config.getAllowlistPackageNamesUnion();
this.disallowProxyObjectAccess = config.isDisallowProxyObjectAccess();
this.disallowProxyMemberAccess = config.isDisallowProxyMemberAccess();
this.disallowDefaultPackageAccess = config.isDisallowDefaultPackageAccess();
}
/**
- * The only place the allowlist union is computed. Both the injected configuration and the
- * deprecated setter route through here; splitting this in two would risk silently dropping
- * {@code ALLOWLIST_REQUIRED_PACKAGES}, which fails open.
+ * Used only by the deprecated {@link #useAllowlistPackageNames(String)} setter path. The injected
+ * configuration path seeds both fields directly from {@link SecurityMemberAccessConfig}, which
+ * precomputes the union exactly once per container; both routes call
+ * {@link SecurityMemberAccessConfig#union(Set, Set)}, so there remains exactly one place in the
+ * codebase that computes the union, and {@code ALLOWLIST_REQUIRED_PACKAGES} cannot be silently
+ * dropped from either.
*/
private void applyAllowlistPackageNames(Set<String> packageNames) {
this.allowlistPackageNames = packageNames;
- this.allowlistPackageNamesUnion = union(ALLOWLIST_REQUIRED_PACKAGES, packageNames);
- }
-
- private static Set<String> union(Set<String> required, Set<String> configured) {
- if (configured.isEmpty()) {
- return required;
- }
- Set<String> union = new HashSet<>(required);
- union.addAll(configured);
- return unmodifiableSet(union);
+ this.allowlistPackageNamesUnion = SecurityMemberAccessConfig.union(SecurityMemberAccessConfig.ALLOWLIST_REQUIRED_PACKAGES, packageNames);
}
@Override
@@ -495,7 +482,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useAllowStaticFieldAccess(String allowStaticFieldAccess) {
this.allowStaticFieldAccess = BooleanUtils.toBoolean(allowStaticFieldAccess);
if (!this.allowStaticFieldAccess) {
@@ -508,7 +495,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useExcludedClasses(String commaDelimitedClasses) {
this.excludedClasses = toNewClassesSet(excludedClasses, commaDelimitedClasses);
}
@@ -518,7 +505,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
this.excludedPackageNamePatterns = toNewPatternsSet(excludedPackageNamePatterns, commaDelimitedPackagePatterns);
}
@@ -528,7 +515,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useExcludedPackageNames(String commaDelimitedPackageNames) {
this.excludedPackageNames = toNewPackageNamesSet(excludedPackageNames, commaDelimitedPackageNames);
}
@@ -538,7 +525,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useExcludedPackageExemptClasses(String commaDelimitedClasses) {
this.excludedPackageExemptClasses = toClassesSet(commaDelimitedClasses);
}
@@ -548,7 +535,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useEnforceAllowlistEnabled(String enforceAllowlistEnabled) {
this.enforceAllowlistEnabled = BooleanUtils.toBoolean(enforceAllowlistEnabled);
if (!this.enforceAllowlistEnabled) {
@@ -565,7 +552,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useAllowlistClasses(String commaDelimitedClasses) {
this.allowlistClasses = toClassObjectsSet(commaDelimitedClasses);
}
@@ -575,7 +562,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useAllowlistPackageNames(String commaDelimitedPackageNames) {
applyAllowlistPackageNames(toPackageNamesSet(commaDelimitedPackageNames));
}
@@ -585,7 +572,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useDisallowProxyObjectAccess(String disallowProxyObjectAccess) {
this.disallowProxyObjectAccess = BooleanUtils.toBoolean(disallowProxyObjectAccess);
}
@@ -595,7 +582,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useDisallowProxyMemberAccess(String disallowProxyMemberAccess) {
this.disallowProxyMemberAccess = BooleanUtils.toBoolean(disallowProxyMemberAccess);
}
@@ -605,7 +592,7 @@ public class SecurityMemberAccess implements MemberAccess {
* {@link SecurityMemberAccessConfig}. This method still mutates this instance and is retained for
* tests and existing callers; it will be removed in Struts 8.0.0.
*/
- @Deprecated
+ @Deprecated(since = "7.4.0", forRemoval = true)
public void useDisallowDefaultPackageAccess(String disallowDefaultPackageAccess) {
this.disallowDefaultPackageAccess = BooleanUtils.toBoolean(disallowDefaultPackageAccess);
}
diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java
index 600973830..59aef3c03 100644
--- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java
+++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java
@@ -25,10 +25,12 @@ import org.apache.struts2.StrutsConstants;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.inject.Initializable;
+import java.util.HashSet;
import java.util.Set;
import java.util.regex.Pattern;
import static java.util.Collections.emptySet;
+import static java.util.Collections.unmodifiableSet;
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES;
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES;
import static org.apache.struts2.util.ConfigParseUtil.toClassObjectsSet;
@@ -57,6 +59,20 @@ public class SecurityMemberAccessConfig implements Initializable {
private static final Logger LOG = LogManager.getLogger(SecurityMemberAccessConfig.class);
+ /**
+ * Struts' own component packages, which must always be allowlisted regardless of what an
+ * application configures via {@code struts.allowlist.packageNames}. Lives here, alongside
+ * {@link #union(Set, Set)}, because this is the single place that computes
+ * {@code allowlistPackageNamesUnion}; {@link SecurityMemberAccess} references both statically for
+ * its default field value and its deprecated {@code useAllowlistPackageNames} setter, so the
+ * computation is never duplicated.
+ */
+ static final Set<String> ALLOWLIST_REQUIRED_PACKAGES = Set.of(
+ "org.apache.struts2.validator.validators",
+ "org.apache.struts2.components",
+ "org.apache.struts2.views.jsp"
+ );
+
private boolean allowStaticFieldAccess = true;
private Set<String> excludedClasses = Set.of(Object.class.getName());
@@ -73,6 +89,7 @@ public class SecurityMemberAccessConfig implements Initializable {
private boolean enforceAllowlistEnabled = false;
private Set<Class<?>> allowlistClasses = emptySet();
private Set<String> allowlistPackageNames = emptySet();
+ private Set<String> allowlistPackageNamesUnion = ALLOWLIST_REQUIRED_PACKAGES;
private boolean disallowProxyObjectAccess = false;
private boolean disallowProxyMemberAccess = false;
@@ -139,6 +156,26 @@ public class SecurityMemberAccessConfig implements Initializable {
@Inject(value = STRUTS_ALLOWLIST_PACKAGE_NAMES, required = false)
public void useAllowlistPackageNames(String commaDelimitedPackageNames) {
this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames);
+ this.allowlistPackageNamesUnion = union(ALLOWLIST_REQUIRED_PACKAGES, allowlistPackageNames);
+ }
+
+ /**
+ * The only place in the codebase that computes the allowlist package union. Both
+ * {@link #useAllowlistPackageNames(String)} above and {@link SecurityMemberAccess}'s deprecated
+ * setter path call this method, so {@code ALLOWLIST_REQUIRED_PACKAGES} can never silently drop out
+ * of the union through a second, drifted implementation.
+ * <p>
+ * The early return aliases {@code required} directly into the result, which is safe only because
+ * every caller passes an immutable {@code Set.of(...)} for that argument; a mutable set must not be
+ * passed as {@code required}.
+ */
+ static Set<String> union(Set<String> required, Set<String> configured) {
+ if (configured.isEmpty()) {
+ return required;
+ }
+ Set<String> union = new HashSet<>(required);
+ union.addAll(configured);
+ return unmodifiableSet(union);
}
@Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, required = false)
@@ -213,6 +250,10 @@ public class SecurityMemberAccessConfig implements Initializable {
return allowlistPackageNames;
}
+ public Set<String> getAllowlistPackageNamesUnion() {
+ return allowlistPackageNamesUnion;
+ }
+
public boolean isDisallowProxyObjectAccess() {
return disallowProxyObjectAccess;
}
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
index ffce445f8..9d74bc637 100644
--- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
@@ -24,6 +24,8 @@ import org.apache.struts2.XWorkTestCase;
import java.util.Map;
import java.util.Set;
+import static org.assertj.core.api.Assertions.assertThat;
+
public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
/**
@@ -46,17 +48,17 @@ public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
* when {@code useConfig} actually ran.
*/
public void testConfigDerivedSetsAreSharedAcrossInstances() throws Exception {
- loadButSet(Map.of(
- StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, "false",
- StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, "^org\\.apache\\.struts2\\.ognl\\.testpkg\\..*",
- StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, "org.apache.struts2.ognl.testpkg",
- StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, "java.lang.String",
- StrutsConstants.STRUTS_ALLOWLIST_ENABLE, "true",
- StrutsConstants.STRUTS_ALLOWLIST_CLASSES, "java.lang.String",
- StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES, "org.apache.struts2.ognl.testpkg",
- StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, "true",
- StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, "true",
- StrutsConstants.STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS, "true"));
+ loadButSet(Map.ofEntries(
+ Map.entry(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, "false"),
+ Map.entry(StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, "^org\\.apache\\.struts2\\.ognl\\.testpkg\\..*"),
+ Map.entry(StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, "org.apache.struts2.ognl.testpkg"),
+ Map.entry(StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, "java.lang.String"),
+ Map.entry(StrutsConstants.STRUTS_ALLOWLIST_ENABLE, "true"),
+ Map.entry(StrutsConstants.STRUTS_ALLOWLIST_CLASSES, "java.lang.String"),
+ Map.entry(StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES, "org.apache.struts2.ognl.testpkg"),
+ Map.entry(StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, "true"),
+ Map.entry(StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, "true"),
+ Map.entry(StrutsConstants.STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS, "true")));
SecurityMemberAccess first = container.getInstance(SecurityMemberAccess.class);
SecurityMemberAccess second = container.getInstance(SecurityMemberAccess.class);
@@ -84,6 +86,10 @@ public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
config.getAllowlistClasses(), SecurityMemberAccessTest.reflectField(first, "allowlistClasses"));
assertSame("allowlistPackageNames not seeded from config",
config.getAllowlistPackageNames(), SecurityMemberAccessTest.reflectField(first, "allowlistPackageNames"));
+ Set<String> firstAllowlistPackageNamesUnion = SecurityMemberAccessTest.reflectField(first, "allowlistPackageNamesUnion");
+ assertSame("allowlistPackageNamesUnion not seeded from config",
+ config.getAllowlistPackageNamesUnion(), firstAllowlistPackageNamesUnion);
+ assertThat(firstAllowlistPackageNamesUnion).contains("org.apache.struts2.ognl.testpkg", "org.apache.struts2.components");
boolean firstAllowStaticFieldAccess = SecurityMemberAccessTest.reflectField(first, "allowStaticFieldAccess");
assertEquals("allowStaticFieldAccess not seeded from config",
@@ -171,12 +177,18 @@ public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
}
public void testDevModeMethodsAreGone() throws Exception {
- for (String name : new String[]{"useDevMode", "useDevModeExcludedClasses",
+ Set<String> removedMethods = Set.of("useDevMode", "useDevModeExcludedClasses",
"useDevModeExcludedPackageNamePatterns", "useDevModeExcludedPackageNames",
- "useDevModeExcludedPackageExemptClasses", "useDevModeConfiguration"}) {
- for (java.lang.reflect.Method method : SecurityMemberAccess.class.getDeclaredMethods()) {
- assertFalse("SecurityMemberAccess still declares " + name, method.getName().equals(name));
- }
+ "useDevModeExcludedPackageExemptClasses", "useDevModeConfiguration");
+ for (java.lang.reflect.Method method : SecurityMemberAccess.class.getDeclaredMethods()) {
+ assertFalse("SecurityMemberAccess still declares " + method.getName(), removedMethods.contains(method.getName()));
+ }
+
+ Set<String> removedFields = Set.of("isDevModeInit", "isDevMode", "devModeExcludedClasses",
+ "devModeExcludedPackageNamePatterns", "devModeExcludedPackageNames",
+ "devModeExcludedPackageExemptClasses");
+ for (java.lang.reflect.Field field : SecurityMemberAccess.class.getDeclaredFields()) {
+ assertFalse("SecurityMemberAccess still declares field " + field.getName(), removedFields.contains(field.getName()));
}
}
}
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java
index e95e99e08..652d26d8c 100644
--- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java
@@ -140,4 +140,25 @@ public class SecurityMemberAccessConfigTest {
assertTrue(config.getExcludedClasses().contains("java.lang.Runtime"));
}
+
+ @Test
+ public void allowlistPackageNamesUnionDefaultsToRequiredPackagesOnly() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+
+ assertEquals(Set.of("org.apache.struts2.validator.validators",
+ "org.apache.struts2.components",
+ "org.apache.struts2.views.jsp"),
+ config.getAllowlistPackageNamesUnion());
+ }
+
+ @Test
+ public void allowlistPackageNamesUnionRetainsRequiredPackagesWhenConfigured() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useAllowlistPackageNames("com.example.app");
+
+ assertTrue(config.getAllowlistPackageNamesUnion().contains("com.example.app"));
+ assertTrue(config.getAllowlistPackageNamesUnion().contains("org.apache.struts2.components"));
+ assertTrue(config.getAllowlistPackageNamesUnion().contains("org.apache.struts2.validator.validators"));
+ assertTrue(config.getAllowlistPackageNamesUnion().contains("org.apache.struts2.views.jsp"));
+ }
}
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java
index a9b7b8c12..84c11a6dd 100644
--- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java
@@ -112,6 +112,7 @@ public class SecurityMemberAccessTest {
"excludedPackageExemptClasses",
"allowlistClasses",
"allowlistPackageNames",
+ "allowlistPackageNamesUnion",
"excludeProperties",
"acceptProperties");
for (String field : fields) {
diff --git a/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java
index 6e9c79727..18567bd89 100644
--- a/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java
+++ b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java
@@ -33,9 +33,9 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
-import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
diff --git a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
index 319ce54ec..8dbdb7122 100644
--- a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
+++ b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
@@ -96,7 +96,16 @@ suite failed with 1579 errors during implementation.
(`ConfigurationManager.java:78-80`). `Dispatcher.init()` (`Dispatcher.java:711-719`) installs its own provider
list — including `StrutsBeanSelectionProvider` via `init_AliasStandardObjects` — so the list is never empty and
`StrutsDefaultConfigurationProvider` is never added. The production container is built from
-`StrutsBeanSelectionProvider` plus `struts-beans.xml`, and `bootstrapFactories` is not on that path at all.
+`StrutsBeanSelectionProvider` plus `struts-beans.xml`, and `bootstrapFactories` is not on the path of that *main
+Dispatcher* container.
+
+It is, however, on a different, load-bearing path: `DefaultConfiguration.reloadContainer` builds a **bootstrap**
+container from `bootstrapFactories` (`DefaultConfiguration.java:283`, via `createBootstrapContainer` at
+`DefaultConfiguration.java:348-373`), then calls `setContext(bootstrap)` (`DefaultConfiguration.java:307`), which
+calls `bootstrap.getInstance(ValueStackFactory.class).createValueStack()` — and building a value stack instantiates
+`SecurityMemberAccess` through `CompoundRootAccessor`/`RootAccessor`. So the bootstrap container's registration
+of `SecurityMemberAccessConfig` is not a fallback for some other, unused path: it is exercised on every
+`reloadContainer()` call, before the main Dispatcher container even exists.
The registration therefore goes in both places, which is precisely what `ProviderAllowlist` and `ThreadAllowlist`
already do — `DefaultConfiguration.java:418-419` and `struts-beans.xml:175-176`:
@@ -106,10 +115,11 @@ already do — `DefaultConfiguration.java:418-419` and `struts-beans.xml:175-176
```
The `DefaultConfiguration` registration serves the bootstrap container (`DefaultConfiguration.java:360`) and the
-`XWorkTestCase` harness; the `struts-beans.xml` entry serves the real Dispatcher container. The bootstrap
-container carries only `BOOTSTRAP_CONSTANTS`, so most security constants are absent there, the
-`required = false` setters do not fire, and the bean falls back to defaults — exactly as a `SecurityMemberAccess`
-constructed in that container behaves today.
+`XWorkTestCase` harness; the `struts-beans.xml` entry serves the real Dispatcher container. **Both registrations
+are load-bearing** — production would throw at startup without either, since `useConfig` is a mandatory `@Inject`
+on `SecurityMemberAccess`. The bootstrap container carries only `BOOTSTRAP_CONSTANTS`, so most security constants
+are absent there, the `required = false` setters do not fire, and the bean falls back to defaults — exactly as a
+`SecurityMemberAccess` constructed in that container behaves today.
This failure mode is loud, not silent: `useConfig` is a mandatory `@Inject`, so a container missing the binding
throws at build time rather than running with empty exclusions.
@@ -230,16 +240,40 @@ This deletes the three-argument `isClassBelongsToPackages` overload and the two-
The ticket flagged this as a fail-open hazard: if the union were computed in two places — once seeded from
configuration, once when the deprecated `useAllowlistPackageNames` setter fires — the two could drift, silently
-dropping `ALLOWLIST_REQUIRED_PACKAGES` from the allowlist with nothing failing loudly. Both routes therefore
-funnel through one private method, so exactly one line in the codebase computes the union:
+dropping `ALLOWLIST_REQUIRED_PACKAGES` from the allowlist with nothing failing loudly. It is also a fail-open
+hazard if the union is *re-computed* per instance: that reintroduces exactly the per-instantiation `HashSet`
+allocation this ticket exists to remove, and lands on the deployments that configure the allowlist properly,
+inverting the ticket's intent.
+
+Both hazards are avoided by moving `ALLOWLIST_REQUIRED_PACKAGES` and the `union(...)` helper onto
+`SecurityMemberAccessConfig`, which precomputes `allowlistPackageNamesUnion` once, inside its own
+`useAllowlistPackageNames` setter, when the constant fires during container construction:
```java
-private void applyAllowlistPackageNames(Set<String> names) {
- this.allowlistPackageNames = names;
- this.allowlistPackageNamesUnion = union(ALLOWLIST_REQUIRED_PACKAGES, names);
+// SecurityMemberAccessConfig
+static final Set<String> ALLOWLIST_REQUIRED_PACKAGES = Set.of(
+ "org.apache.struts2.validator.validators",
+ "org.apache.struts2.components",
+ "org.apache.struts2.views.jsp"
+);
+
+public void useAllowlistPackageNames(String commaDelimitedPackageNames) {
+ this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames);
+ this.allowlistPackageNamesUnion = union(ALLOWLIST_REQUIRED_PACKAGES, allowlistPackageNames);
}
+
+static Set<String> union(Set<String> required, Set<String> configured) { … }
```
+`SecurityMemberAccess.useConfig` copies the precomputed reference (`config.getAllowlistPackageNamesUnion()`) —
+no allocation on the hot instantiation path. Its deprecated `useAllowlistPackageNames` setter, which still mutates
+a single instance directly and has no `SecurityMemberAccessConfig` to read from, calls the same
+`SecurityMemberAccessConfig.union(...)` static method. Both routes therefore funnel through the one method, so
+exactly one line in the codebase computes the union, and `ALLOWLIST_REQUIRED_PACKAGES` cannot drift out of it
+through a second implementation. The constant and helper live on the config bean — the class that owns computing
+and exposing configuration-derived state — rather than being duplicated onto `SecurityMemberAccess`, whose
+deprecated setter merely calls back into it.
+
`isPackageBelongsToPackages` currently early-returns on `first.isEmpty() && second.isEmpty()`. Since
`ALLOWLIST_REQUIRED_PACKAGES` is never empty, that guard simply stops firing on the allowlist path; the exclusion
path, where both sets genuinely can be empty, keeps it. The guard was only ever an optimization, so this is not a
@@ -277,8 +311,8 @@ Core tests are JUnit 4 or extend `XWorkTestCase`. A JUnit 5 `@Test` added to the
1. **Sharing proof.** Request several `SecurityMemberAccess` instances from one container and assert their
configuration-derived sets are reference-identical (`assertSame`, not `assertEquals`). Reference identity is a
- dependency-free proof that no re-parsing occurred, since any re-parse necessarily produces a fresh set. Backed
- by a counting probe asserting exactly one parse per container.
+ dependency-free proof that no re-parsing occurred, since any re-parse necessarily produces a fresh set; this is
+ the sound substitute for a counting probe and is what the implementation actually asserts.
2. **Instance isolation.** Calling a deprecated setter on one instance must not perturb a sibling instance or the
singleton. The sets are `unmodifiableSet`, so an in-place mutation bug would throw rather than corrupt
silently, but this invariant deserves an explicit assertion.
(struts) 07/15: WW-5675 feat(ognl): add a container-singleton OGNL security config bean
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 32ceb19246cd905d7b685b084f0625c78abad4a8
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:11:32 2026 +0200
WW-5675 feat(ognl): add a container-singleton OGNL security config bean
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../struts2/ognl/SecurityMemberAccessConfig.java | 227 +++++++++++++++++++++
.../ognl/SecurityMemberAccessConfigTest.java | 143 +++++++++++++
2 files changed, 370 insertions(+)
diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java
new file mode 100644
index 000000000..600973830
--- /dev/null
+++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccessConfig.java
@@ -0,0 +1,227 @@
+/*
+ * 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
+ *
+ * http://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.apache.struts2.ognl;
+
+import org.apache.commons.lang3.BooleanUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.inject.Inject;
+import org.apache.struts2.inject.Initializable;
+
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import static java.util.Collections.emptySet;
+import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES;
+import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES;
+import static org.apache.struts2.util.ConfigParseUtil.toClassObjectsSet;
+import static org.apache.struts2.util.ConfigParseUtil.toClassesSet;
+import static org.apache.struts2.util.ConfigParseUtil.toNewClassesSet;
+import static org.apache.struts2.util.ConfigParseUtil.toNewPackageNamesSet;
+import static org.apache.struts2.util.ConfigParseUtil.toNewPatternsSet;
+import static org.apache.struts2.util.ConfigParseUtil.toPackageNamesSet;
+import static org.apache.struts2.util.DebugUtils.logWarningForFirstOccurrence;
+
+/**
+ * Holds the parsed OGNL security configuration for one container.
+ * <p>
+ * {@link SecurityMemberAccess} is a {@code Scope.PROTOTYPE} bean, constructed once per value stack and
+ * again for each OGNL context. Parsing the roughly ninety configuration entries on every one of those
+ * was the dominant cost identified by WW-5667. This bean is a {@code Scope.SINGLETON}, so the parsing
+ * happens once per container and each {@code SecurityMemberAccess} merely copies immutable references.
+ * <p>
+ * Dev-mode is resolved in {@link #init()} rather than in a setter, because the container iterates
+ * {@code getDeclaredMethods()}, whose order the JDK leaves unspecified. If {@code init()} never runs,
+ * the normal production exclusions stay in force, which fails closed.
+ *
+ * @since Struts 7.4.0
+ */
+public class SecurityMemberAccessConfig implements Initializable {
+
+ private static final Logger LOG = LogManager.getLogger(SecurityMemberAccessConfig.class);
+
+ private boolean allowStaticFieldAccess = true;
+
+ private Set<String> excludedClasses = Set.of(Object.class.getName());
+ private Set<Pattern> excludedPackageNamePatterns = emptySet();
+ private Set<String> excludedPackageNames = emptySet();
+ private Set<String> excludedPackageExemptClasses = emptySet();
+
+ private boolean isDevMode;
+ private Set<String> devModeExcludedClasses = Set.of(Object.class.getName());
+ private Set<Pattern> devModeExcludedPackageNamePatterns = emptySet();
+ private Set<String> devModeExcludedPackageNames = emptySet();
+ private Set<String> devModeExcludedPackageExemptClasses = emptySet();
+
+ private boolean enforceAllowlistEnabled = false;
+ private Set<Class<?>> allowlistClasses = emptySet();
+ private Set<String> allowlistPackageNames = emptySet();
+
+ private boolean disallowProxyObjectAccess = false;
+ private boolean disallowProxyMemberAccess = false;
+ private boolean disallowDefaultPackageAccess = false;
+
+ @Override
+ public void init() {
+ if (!isDevMode) {
+ return;
+ }
+ logWarningForFirstOccurrence("devMode", LOG,
+ "DevMode enabled, using DevMode excluded classes and packages for OGNL security enforcement!");
+ excludedClasses = devModeExcludedClasses;
+ excludedPackageNamePatterns = devModeExcludedPackageNamePatterns;
+ excludedPackageNames = devModeExcludedPackageNames;
+ excludedPackageExemptClasses = devModeExcludedPackageExemptClasses;
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, required = false)
+ public void useAllowStaticFieldAccess(String allowStaticFieldAccess) {
+ this.allowStaticFieldAccess = BooleanUtils.toBoolean(allowStaticFieldAccess);
+ if (!this.allowStaticFieldAccess) {
+ useExcludedClasses(Class.class.getName());
+ }
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_EXCLUDED_CLASSES, required = false)
+ public void useExcludedClasses(String commaDelimitedClasses) {
+ this.excludedClasses = toNewClassesSet(excludedClasses, commaDelimitedClasses);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
+ public void useExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
+ this.excludedPackageNamePatterns = toNewPatternsSet(excludedPackageNamePatterns, commaDelimitedPackagePatterns);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, required = false)
+ public void useExcludedPackageNames(String commaDelimitedPackageNames) {
+ this.excludedPackageNames = toNewPackageNamesSet(excludedPackageNames, commaDelimitedPackageNames);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
+ public void useExcludedPackageExemptClasses(String commaDelimitedClasses) {
+ this.excludedPackageExemptClasses = toClassesSet(commaDelimitedClasses);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_ALLOWLIST_ENABLE, required = false)
+ public void useEnforceAllowlistEnabled(String enforceAllowlistEnabled) {
+ this.enforceAllowlistEnabled = BooleanUtils.toBoolean(enforceAllowlistEnabled);
+ if (!this.enforceAllowlistEnabled) {
+ String msg = "OGNL allowlist is disabled!" +
+ " We strongly recommend keeping it enabled to protect against critical vulnerabilities." +
+ " Set the configuration `{}=true` to enable it." +
+ " Please refer to the Struts 7.0 migration guide and security documentation for further information.";
+ logWarningForFirstOccurrence("allowlist", LOG, msg, StrutsConstants.STRUTS_ALLOWLIST_ENABLE);
+ }
+ }
+
+ @Inject(value = STRUTS_ALLOWLIST_CLASSES, required = false)
+ public void useAllowlistClasses(String commaDelimitedClasses) {
+ this.allowlistClasses = toClassObjectsSet(commaDelimitedClasses);
+ }
+
+ @Inject(value = STRUTS_ALLOWLIST_PACKAGE_NAMES, required = false)
+ public void useAllowlistPackageNames(String commaDelimitedPackageNames) {
+ this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, required = false)
+ public void useDisallowProxyObjectAccess(String disallowProxyObjectAccess) {
+ this.disallowProxyObjectAccess = BooleanUtils.toBoolean(disallowProxyObjectAccess);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, required = false)
+ public void useDisallowProxyMemberAccess(String disallowProxyMemberAccess) {
+ this.disallowProxyMemberAccess = BooleanUtils.toBoolean(disallowProxyMemberAccess);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS, required = false)
+ public void useDisallowDefaultPackageAccess(String disallowDefaultPackageAccess) {
+ this.disallowDefaultPackageAccess = BooleanUtils.toBoolean(disallowDefaultPackageAccess);
+ }
+
+ @Inject(StrutsConstants.STRUTS_DEVMODE)
+ public void useDevMode(String devMode) {
+ this.isDevMode = BooleanUtils.toBoolean(devMode);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false)
+ public void useDevModeExcludedClasses(String commaDelimitedClasses) {
+ this.devModeExcludedClasses = toNewClassesSet(devModeExcludedClasses, commaDelimitedClasses);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
+ public void useDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
+ this.devModeExcludedPackageNamePatterns = toNewPatternsSet(devModeExcludedPackageNamePatterns, commaDelimitedPackagePatterns);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false)
+ public void useDevModeExcludedPackageNames(String commaDelimitedPackageNames) {
+ this.devModeExcludedPackageNames = toNewPackageNamesSet(devModeExcludedPackageNames, commaDelimitedPackageNames);
+ }
+
+ @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false)
+ public void useDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) {
+ this.devModeExcludedPackageExemptClasses = toClassesSet(commaDelimitedClasses);
+ }
+
+ public boolean isAllowStaticFieldAccess() {
+ return allowStaticFieldAccess;
+ }
+
+ public Set<String> getExcludedClasses() {
+ return excludedClasses;
+ }
+
+ public Set<Pattern> getExcludedPackageNamePatterns() {
+ return excludedPackageNamePatterns;
+ }
+
+ public Set<String> getExcludedPackageNames() {
+ return excludedPackageNames;
+ }
+
+ public Set<String> getExcludedPackageExemptClasses() {
+ return excludedPackageExemptClasses;
+ }
+
+ public boolean isEnforceAllowlistEnabled() {
+ return enforceAllowlistEnabled;
+ }
+
+ public Set<Class<?>> getAllowlistClasses() {
+ return allowlistClasses;
+ }
+
+ public Set<String> getAllowlistPackageNames() {
+ return allowlistPackageNames;
+ }
+
+ public boolean isDisallowProxyObjectAccess() {
+ return disallowProxyObjectAccess;
+ }
+
+ public boolean isDisallowProxyMemberAccess() {
+ return disallowProxyMemberAccess;
+ }
+
+ public boolean isDisallowDefaultPackageAccess() {
+ return disallowDefaultPackageAccess;
+ }
+}
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java
new file mode 100644
index 000000000..e95e99e08
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigTest.java
@@ -0,0 +1,143 @@
+/*
+ * 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
+ *
+ * http://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.apache.struts2.ognl;
+
+import org.junit.Test;
+
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import static org.apache.struts2.util.ConfigParseUtil.toNewClassesSet;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class SecurityMemberAccessConfigTest {
+
+ /**
+ * Frozen oracle: the accumulation SecurityMemberAccess performed before WW-5675.
+ * Never delete this, and never make it delegate to production code.
+ */
+ private static Set<String> legacyExcludedClassAccumulation(boolean allowStaticFieldAccess, String configured) {
+ Set<String> excludedClasses = Set.of(Object.class.getName());
+ if (!allowStaticFieldAccess) {
+ excludedClasses = toNewClassesSet(excludedClasses, Class.class.getName());
+ }
+ return toNewClassesSet(excludedClasses, configured);
+ }
+
+ private SecurityMemberAccessConfig configWith(boolean devMode, String excludedClasses, String devModeExcludedClasses) {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useDevMode(String.valueOf(devMode));
+ config.useExcludedClasses(excludedClasses);
+ config.useDevModeExcludedClasses(devModeExcludedClasses);
+ config.init();
+ return config;
+ }
+
+ @Test
+ public void excludedClassesMatchLegacyAccumulation() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder");
+ config.init();
+
+ assertEquals(legacyExcludedClassAccumulation(true, "java.lang.Runtime,java.lang.ProcessBuilder"),
+ config.getExcludedClasses());
+ }
+
+ @Test
+ public void disallowingStaticFieldAccessAddsClassToExclusions() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useAllowStaticFieldAccess("false");
+ config.useExcludedClasses("java.lang.Runtime");
+ config.init();
+
+ assertFalse(config.isAllowStaticFieldAccess());
+ assertEquals(legacyExcludedClassAccumulation(false, "java.lang.Runtime"), config.getExcludedClasses());
+ }
+
+ /**
+ * The container iterates getDeclaredMethods(), whose order the JDK leaves unspecified.
+ * The accumulation must therefore be commutative, as it was before WW-5675.
+ */
+ @Test
+ public void setterOrderDoesNotAffectExcludedClasses() {
+ SecurityMemberAccessConfig forward = new SecurityMemberAccessConfig();
+ forward.useAllowStaticFieldAccess("false");
+ forward.useExcludedClasses("java.lang.Runtime");
+ forward.init();
+
+ SecurityMemberAccessConfig reverse = new SecurityMemberAccessConfig();
+ reverse.useExcludedClasses("java.lang.Runtime");
+ reverse.useAllowStaticFieldAccess("false");
+ reverse.init();
+
+ assertEquals(forward.getExcludedClasses(), reverse.getExcludedClasses());
+ }
+
+ @Test
+ public void devModeDisabledPublishesNormalExclusions() {
+ SecurityMemberAccessConfig config = configWith(false, "java.lang.Runtime", "java.lang.ProcessBuilder");
+
+ assertTrue(config.getExcludedClasses().contains("java.lang.Runtime"));
+ assertFalse(config.getExcludedClasses().contains("java.lang.ProcessBuilder"));
+ }
+
+ @Test
+ public void devModeEnabledPublishesDevModeExclusions() {
+ SecurityMemberAccessConfig config = configWith(true, "java.lang.Runtime", "java.lang.ProcessBuilder");
+
+ assertTrue(config.getExcludedClasses().contains("java.lang.ProcessBuilder"));
+ assertFalse(config.getExcludedClasses().contains("java.lang.Runtime"));
+ }
+
+ @Test
+ public void packageNamesAreStrippedOfDots() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useExcludedPackageNames("java.io.,.java.net");
+ config.init();
+
+ assertTrue(config.getExcludedPackageNames().contains("java.io"));
+ assertTrue(config.getExcludedPackageNames().contains("java.net"));
+ }
+
+ @Test
+ public void patternsAreCompiledOnce() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useExcludedPackageNamePatterns("^java\\.lang\\..*");
+ config.init();
+
+ Set<Pattern> patterns = config.getExcludedPackageNamePatterns();
+ assertEquals(1, patterns.size());
+ assertTrue(patterns.iterator().next().matcher("java.lang.Runtime").matches());
+ }
+
+ /**
+ * A missing init() must fail closed: production exclusions, never the dev-mode ones.
+ */
+ @Test
+ public void withoutInitTheNormalExclusionsApply() {
+ SecurityMemberAccessConfig config = new SecurityMemberAccessConfig();
+ config.useDevMode("true");
+ config.useExcludedClasses("java.lang.Runtime");
+ config.useDevModeExcludedClasses("java.lang.ProcessBuilder");
+
+ assertTrue(config.getExcludedClasses().contains("java.lang.Runtime"));
+ }
+}
(struts) 14/15: WW-5675 perf(ognl): precompute the allowlist package union
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 6028e9dc86b43c8467daadb2a015950a893bc0e9
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:59:23 2026 +0200
WW-5675 perf(ognl): precompute the allowlist package union
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../apache/struts2/ognl/SecurityMemberAccess.java | 76 ++++++++++++----------
.../SecurityMemberAccessPackageMatchingTest.java | 46 ++++++++-----
2 files changed, 70 insertions(+), 52 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
index b7ed69caa..80c36f05c 100644
--- a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
@@ -32,12 +32,14 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Modifier;
+import java.util.HashSet;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static java.text.MessageFormat.format;
import static java.util.Collections.emptySet;
+import static java.util.Collections.unmodifiableSet;
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES;
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES;
import static org.apache.struts2.util.ConfigParseUtil.toClassObjectsSet;
@@ -89,6 +91,7 @@ public class SecurityMemberAccess implements MemberAccess {
private boolean enforceAllowlistEnabled = false;
private Set<Class<?>> allowlistClasses = emptySet();
private Set<String> allowlistPackageNames = emptySet();
+ private Set<String> allowlistPackageNamesUnion = ALLOWLIST_REQUIRED_PACKAGES;
private boolean disallowProxyObjectAccess = false;
private boolean disallowProxyMemberAccess = false;
@@ -121,12 +124,31 @@ public class SecurityMemberAccess implements MemberAccess {
this.excludedPackageExemptClasses = config.getExcludedPackageExemptClasses();
this.enforceAllowlistEnabled = config.isEnforceAllowlistEnabled();
this.allowlistClasses = config.getAllowlistClasses();
- this.allowlistPackageNames = config.getAllowlistPackageNames();
+ applyAllowlistPackageNames(config.getAllowlistPackageNames());
this.disallowProxyObjectAccess = config.isDisallowProxyObjectAccess();
this.disallowProxyMemberAccess = config.isDisallowProxyMemberAccess();
this.disallowDefaultPackageAccess = config.isDisallowDefaultPackageAccess();
}
+ /**
+ * The only place the allowlist union is computed. Both the injected configuration and the
+ * deprecated setter route through here; splitting this in two would risk silently dropping
+ * {@code ALLOWLIST_REQUIRED_PACKAGES}, which fails open.
+ */
+ private void applyAllowlistPackageNames(Set<String> packageNames) {
+ this.allowlistPackageNames = packageNames;
+ this.allowlistPackageNamesUnion = union(ALLOWLIST_REQUIRED_PACKAGES, packageNames);
+ }
+
+ private static Set<String> union(Set<String> required, Set<String> configured) {
+ if (configured.isEmpty()) {
+ return required;
+ }
+ Set<String> union = new HashSet<>(required);
+ union.addAll(configured);
+ return unmodifiableSet(union);
+ }
+
@Override
public Object setup(OgnlContext context, Object target, Member member, String propertyName) {
Object result = null;
@@ -269,7 +291,7 @@ public class SecurityMemberAccess implements MemberAccess {
|| ALLOWLIST_REQUIRED_CLASSES.contains(clazz)
|| (providerAllowlist != null && providerAllowlist.getProviderAllowlist().contains(clazz))
|| (threadAllowlist != null && threadAllowlist.getAllowlist().contains(clazz))
- || isClassBelongsToPackages(clazz, ALLOWLIST_REQUIRED_PACKAGES, allowlistPackageNames);
+ || isClassBelongsToPackages(clazz, allowlistPackageNamesUnion);
}
/**
@@ -404,54 +426,38 @@ public class SecurityMemberAccess implements MemberAccess {
}
public static boolean isClassBelongsToPackages(Class<?> clazz, Set<String> matchingPackages) {
- return isClassBelongsToPackages(clazz, matchingPackages, emptySet());
- }
-
- /**
- * Tests the class's package against two sets in a single walk. Equivalent to calling
- * {@link #isClassBelongsToPackages(Class, Set)} once per set and OR-ing the results, but
- * walks the package name only once.
- *
- * @param clazz the class whose package is tested
- * @param first the first set of package names to match against
- * @param second the second set of package names to match against
- * @return {@code true} if the class's package or any parent package is in either set
- */
- static boolean isClassBelongsToPackages(Class<?> clazz, Set<String> first, Set<String> second) {
- return isPackageBelongsToPackages(toPackageName(clazz), first, second);
+ return isPackageBelongsToPackages(toPackageName(clazz), matchingPackages);
}
/**
- * Tests whether the given package name, or any of its parent packages, is present in either
- * set. Walks the name in place rather than building the full prefix list, since this runs on
- * the OGNL member-access path. Shortest prefix first, so broad entries such as {@code java.io}
+ * Tests whether the given package name, or any of its parent packages, is present in the set.
+ * Walks the name in place rather than building the full prefix list, since this runs on the OGNL
+ * member-access path. Shortest prefix first, so broad entries such as {@code java.io}
* short-circuit earliest.
*
* <p>
- * The package name must not end in {@code '.'}. Such a name is probed one prefix more than by
- * the implementation this replaced, which matches more broadly — tightening exclusion but
- * <em>loosening</em> the allowlist. {@link Class#getPackageName()} cannot produce a trailing
- * dot, so every current caller is safe; route any other string through here only after
- * confirming the same.
+ * The package name must not end in {@code '.'}. Such a name is probed one prefix more than by the
+ * implementation this replaced, which matches more broadly — tightening exclusion but
+ * <em>loosening</em> the allowlist. {@link Class#getPackageName()} cannot produce a trailing dot,
+ * and {@code ConfigParseUtil.toPackageNamesSet} strips them from configured names, so every
+ * current caller is safe; route any other string through here only after confirming the same.
*
- * @param packageName the package name to test, empty for the default package, never ending in {@code '.'}
- * @param first the first set of package names to match against
- * @param second the second set of package names to match against
- * @return {@code true} if the package or any parent package is in either set
+ * @param packageName the package name to test, empty for the default package, never ending in {@code '.'}
+ * @param matchingPackages the package names to match against
+ * @return {@code true} if the package or any parent package is in the set
*/
- static boolean isPackageBelongsToPackages(String packageName, Set<String> first, Set<String> second) {
- if (first.isEmpty() && second.isEmpty()) {
+ static boolean isPackageBelongsToPackages(String packageName, Set<String> matchingPackages) {
+ if (matchingPackages.isEmpty()) {
return false;
}
int idx = packageName.indexOf('.');
while (idx != -1) {
- String prefix = packageName.substring(0, idx);
- if (first.contains(prefix) || second.contains(prefix)) {
+ if (matchingPackages.contains(packageName.substring(0, idx))) {
return true;
}
idx = packageName.indexOf('.', idx + 1);
}
- return first.contains(packageName) || second.contains(packageName);
+ return matchingPackages.contains(packageName);
}
protected boolean isClassExcluded(Class<?> clazz) {
@@ -571,7 +577,7 @@ public class SecurityMemberAccess implements MemberAccess {
*/
@Deprecated
public void useAllowlistPackageNames(String commaDelimitedPackageNames) {
- this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames);
+ applyAllowlistPackageNames(toPackageNamesSet(commaDelimitedPackageNames));
}
/**
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java
index 413b0c669..3dfa540dd 100644
--- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java
@@ -117,21 +117,21 @@ public class SecurityMemberAccessPackageMatchingTest {
public void siblingPackageWithSharedCharacterPrefixDoesNotMatch() {
Set<String> excluded = Set.of("org.apache.struts2");
- assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2x", excluded, emptySet()))
+ assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2x", excluded))
.as("a sibling package sharing a character prefix must not match (production)")
.isFalse();
assertThat(legacyPrefixMatch("org.apache.struts2x", excluded))
.as("a sibling package sharing a character prefix must not match (legacy oracle)")
.isFalse();
- assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2", excluded, emptySet()))
+ assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2", excluded))
.as("an exact match must match (production)")
.isTrue();
assertThat(legacyPrefixMatch("org.apache.struts2", excluded))
.as("an exact match must match (legacy oracle)")
.isTrue();
- assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2.ognl", excluded, emptySet()))
+ assertThat(SecurityMemberAccess.isPackageBelongsToPackages("org.apache.struts2.ognl", excluded))
.as("a sub-package must match (production)")
.isTrue();
assertThat(legacyPrefixMatch("org.apache.struts2.ognl", excluded))
@@ -192,7 +192,7 @@ public class SecurityMemberAccessPackageMatchingTest {
public void indexWalkMatchesLegacyAcrossPackageNameShapes() {
for (String packageName : PACKAGE_NAMES) {
for (Set<String> candidates : CANDIDATE_SETS) {
- assertThat(SecurityMemberAccess.isPackageBelongsToPackages(packageName, candidates, emptySet()))
+ assertThat(SecurityMemberAccess.isPackageBelongsToPackages(packageName, candidates))
.as("packageName=[%s] candidates=%s", packageName, candidates)
.isEqualTo(legacyPrefixMatch(packageName, candidates));
}
@@ -200,25 +200,37 @@ public class SecurityMemberAccessPackageMatchingTest {
}
@Test
- public void bothSetsEmptyShortCircuitsToFalse() {
+ public void emptyCandidateSetShortCircuitsToFalse() {
for (String packageName : PACKAGE_NAMES) {
- assertThat(SecurityMemberAccess.isPackageBelongsToPackages(packageName, emptySet(), emptySet()))
+ assertThat(SecurityMemberAccess.isPackageBelongsToPackages(packageName, emptySet()))
.as("packageName=[%s] with no configured packages", packageName)
.isFalse();
}
}
+ /**
+ * The union must never lose ALLOWLIST_REQUIRED_PACKAGES. Dropping them would be a silent
+ * fail-open: Struts' own components would stop being allowlisted with nothing failing loudly.
+ */
@Test
- public void twoSetOverloadEqualsDisjunctionOfSingleSetCalls() throws Exception {
- for (Class<?> clazz : classShapes()) {
- for (Set<String> first : CANDIDATE_SETS) {
- for (Set<String> second : CANDIDATE_SETS) {
- assertThat(isClassBelongsToPackages(clazz, first, second))
- .as("clazz=[%s] first=%s second=%s", clazz.getName(), first, second)
- .isEqualTo(isClassBelongsToPackages(clazz, first)
- || isClassBelongsToPackages(clazz, second));
- }
- }
- }
+ public void allowlistUnionRetainsRequiredPackagesAfterSetterCall() throws Exception {
+ SecurityMemberAccess sma = new SecurityMemberAccess(null, null);
+ sma.useAllowlistPackageNames("com.example.app");
+
+ Set<String> union = SecurityMemberAccessTest.reflectField(sma, "allowlistPackageNamesUnion");
+
+ assertThat(union).contains("com.example.app", "org.apache.struts2.components");
+ }
+
+ @Test
+ public void allowlistUnionContainsRequiredPackagesByDefault() throws Exception {
+ SecurityMemberAccess sma = new SecurityMemberAccess(null, null);
+
+ Set<String> union = SecurityMemberAccessTest.reflectField(sma, "allowlistPackageNamesUnion");
+
+ assertThat(union).contains(
+ "org.apache.struts2.components",
+ "org.apache.struts2.views.jsp",
+ "org.apache.struts2.validator.validators");
}
}
(struts) 01/15: WW-5675 docs(ognl): add design for sharing parsed OGNL security config
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit b10154e1702f88cccc4b9107bb6fc9e52dcb1e6b
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 13:45:30 2026 +0200
WW-5675 docs(ognl): add design for sharing parsed OGNL security config
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...security-member-access-config-sharing-design.md | 286 +++++++++++++++++++++
1 file changed, 286 insertions(+)
diff --git a/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
new file mode 100644
index 000000000..b5435fe33
--- /dev/null
+++ b/docs/superpowers/specs/2026-08-14-WW-5675-security-member-access-config-sharing-design.md
@@ -0,0 +1,286 @@
+# WW-5675 — Share parsed OGNL security configuration across `SecurityMemberAccess` instances
+
+**Ticket:** [WW-5675](https://issues.apache.org/jira/browse/WW-5675) (sub-task of [WW-5667](https://issues.apache.org/jira/browse/WW-5667))
+**Target:** 7.4.0
+**Date:** 2026-08-14
+**Status:** Design approved, pending implementation plan
+**Open decision:** whether the five dev-mode setters are deleted or deprecated — see "`SecurityMemberAccess`
+changes"
+
+## Problem
+
+`SecurityMemberAccess` is a `Scope.PROTOTYPE` bean, registered in two places:
+
+- `DefaultConfiguration.java:416` — `.factory(SecurityMemberAccess.class, Scope.PROTOTYPE)`
+- `StrutsBeanSelectionProvider.java:456` — aliased to `STRUTS_MEMBER_ACCESS`, making it user-overridable
+
+Every `container.getInstance(SecurityMemberAccess.class)` therefore constructs a fresh instance and re-runs all
+sixteen `@Inject` configuration setters, each of which re-parses a raw comma-delimited string from scratch. With
+the stock `struts-excluded-classes.xml` that is roughly 90 configuration entries per instantiation: comma
+splitting, `strip`, classloader validation, `Pattern.compile`, and `HashSet` accumulation.
+
+New instances are created on the request path from at least:
+
+- `OgnlValueStackFactory.createValueStack(...)` — once per value stack, and `ParametersInterceptor` creates an
+ additional stack per request
+- `OgnlUtil.createDefaultContext(Object, ClassResolver)` at `OgnlUtil.java:738` — reached from `setProperties`,
+ `copy`, `getBeanMap` and friends. Note `OgnlUtil.copy` calls it **twice** (`OgnlUtil.java:551-552`), so a single
+ copy costs two full configuration rebuilds.
+
+This is the dominant half of the parent report. The sibling ticket WW-5674 (merged as `81b34c295`) addressed the
+per-*access* allocations; this ticket addresses the per-*instantiation* cost, which is where the reported 9% lives.
+
+The fix proposed on the parent ticket — caching the parsed set in a `SecurityMemberAccess` field — cannot work,
+because the instance holding the field is itself discarded and rebuilt each time.
+
+### Also in scope
+
+`ConfigParseUtil.validatePackageNames` (`ConfigParseUtil.java:143`) evaluates `Pattern.compile("\\s")` once per
+package name rather than once overall — roughly 58 recompiles of a trivial pattern per instantiation under the
+default configuration. Hoist it to a static constant.
+
+## Goals
+
+- Parse the OGNL security configuration once per container instead of once per `SecurityMemberAccess`.
+- Preserve OGNL allow/deny semantics exactly. No configuration may become more permissive.
+- Keep source compatibility for 7.4.0: existing subclasses and direct setter callers must continue to compile and
+ behave identically.
+- Collapse the two-set allowlist walk introduced by WW-5674 into a single precomputed set.
+
+## Non-goals
+
+- Changing array/primitive package-resolution semantics — that is WW-5676, deliberately separate because it is a
+ security-semantics decision rather than a performance fix.
+- Removing the residual per-access `getPackage()` lookups — that is WW-5677.
+- Removing the deprecated setters. They are scheduled for 8.0.0 (see Follow-ups).
+- Adding JMH or any benchmarking infrastructure to the build.
+
+## Approach
+
+Introduce a container-singleton configuration bean that owns all parsing. `SecurityMemberAccess` stays
+`Scope.PROTOTYPE` and copies immutable set *references* out of that bean.
+
+Two alternatives were considered and rejected:
+
+**Memoize parsing inside `ConfigParseUtil`** (keyed by raw config string, following the existing Caffeine
+precedent in that file). Smallest possible diff and no API change, but it recovers the least: every instantiation
+still invokes sixteen setters, still builds the accumulated `HashSet` copies, and still runs the lazy dev-mode
+flip. It also does not unblock the allowlist union collapse.
+
+**Revert `SecurityMemberAccess` to `Scope.SINGLETON`**, relocating `acceptProperties`/`excludeProperties` into the
+OGNL context. Largest theoretical win, but it reverses a deliberate WW-5343 decision, converts two fields into
+shared mutable state requiring thread-safety on the OGNL security gate, and changes the `MemberAccessValueStack`
+contract that `ParametersInterceptor` depends on. Under the chosen approach the per-instantiation cost is already
+about a dozen reference copies, so this buys very little for substantially more risk.
+
+## Design
+
+### New bean: `SecurityMemberAccessConfig`
+
+Registered in `DefaultConfiguration` beside the existing internal singletons:
+
+```java
+.factory(SecurityMemberAccessConfig.class, Scope.SINGLETON)
+```
+
+Concrete class, no interface, **not** aliased in `StrutsBeanSelectionProvider`. It is internal plumbing, following
+the shape of `ProviderAllowlist` and `ThreadAllowlist` (`DefaultConfiguration.java:418-419`), not a user extension
+point.
+
+It takes over these sixteen `@Inject` setters from `SecurityMemberAccess`:
+
+| Setter | Constant |
+|---|---|
+| `useAllowStaticFieldAccess` | `STRUTS_ALLOW_STATIC_FIELD_ACCESS` |
+| `useExcludedClasses` | `STRUTS_EXCLUDED_CLASSES` |
+| `useExcludedPackageNamePatterns` | `STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS` |
+| `useExcludedPackageNames` | `STRUTS_EXCLUDED_PACKAGE_NAMES` |
+| `useExcludedPackageExemptClasses` | `STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES` |
+| `useEnforceAllowlistEnabled` | `STRUTS_ALLOWLIST_ENABLE` |
+| `useAllowlistClasses` | `STRUTS_ALLOWLIST_CLASSES` |
+| `useAllowlistPackageNames` | `STRUTS_ALLOWLIST_PACKAGE_NAMES` |
+| `useDisallowProxyObjectAccess` | `STRUTS_DISALLOW_PROXY_OBJECT_ACCESS` |
+| `useDisallowProxyMemberAccess` | `STRUTS_DISALLOW_PROXY_MEMBER_ACCESS` |
+| `useDisallowDefaultPackageAccess` | `STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS` |
+| `useDevMode` | `STRUTS_DEVMODE` |
+| `useDevModeExcludedClasses` | `STRUTS_DEV_MODE_EXCLUDED_CLASSES` |
+| `useDevModeExcludedPackageNamePatterns` | `STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS` |
+| `useDevModeExcludedPackageNames` | `STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES` |
+| `useDevModeExcludedPackageExemptClasses` | `STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES` |
+
+`setProxyService` and the `@Inject` constructor stay on `SecurityMemberAccess` — those inject collaborators, not
+configuration.
+
+The bean implements `Initializable`. Dev-mode resolution cannot happen inside any individual setter, because
+`ContainerImpl.addInjectorsForMembers` iterates `getDeclaredMethods()`, whose order the JDK explicitly leaves
+unspecified. `Initializable.init()` runs after the whole dependency graph is built
+(`InitializableFactory.wrapIfNeeded`, applied from `Scope` for singleton scope; `DefaultValidatorFactory` is the
+existing precedent). `init()` therefore:
+
+1. Selects the effective excluded sets — dev-mode variants when `struts.devMode=true`, otherwise the normal ones.
+2. Precomputes the allowlist package union.
+
+The bean exposes only immutable getters, and publishes the **effective** excluded sets with dev-mode already
+applied, so nothing downstream needs to know dev-mode exists.
+
+### `SecurityMemberAccess` changes
+
+Gains exactly one injected member:
+
+```java
+@Inject
+public void useConfig(SecurityMemberAccessConfig config) { … }
+```
+
+which seeds its fields by copying immutable set references — no parsing, no `HashSet` construction, no
+`Pattern.compile`.
+
+**Fields removed:** `isDevModeInit` (volatile), `isDevMode`, `devModeExcludedClasses`,
+`devModeExcludedPackageNamePatterns`, `devModeExcludedPackageNames`, `devModeExcludedPackageExemptClasses`.
+
+**Method removed:** `useDevModeConfiguration()`, along with its call from `checkExclusionList`
+(`SecurityMemberAccess.java:264`). The lazy dev-mode flip disappears from the access path entirely.
+
+**Field added:** `allowlistPackageNamesUnion`.
+
+The five dev-mode setters are proposed for **deletion outright rather than deprecation**. This is a deliberate
+deviation from the "additive and deprecate, no breakage in a minor" policy chosen for the rest of this change, and
+needs explicit sign-off.
+
+The case for deleting them: they are `public`, but only ever container-injected, with no direct caller anywhere in
+core, plugins, or tests. Preserving them faithfully would mean keeping `isDevMode` plus the four dev-mode set
+fields on the instance and reinstating some form of the lazy flip — that is, keeping precisely the code this
+change exists to delete, to serve a caller that does not demonstrably exist.
+
+The case against: they are public methods on a user-overridable bean, so a deployment could in principle call
+them, and removal in a minor release would break it at compile time. Deprecating them while preserving exact
+semantics is not cheap, because today's semantics are subtle — a manual `useDevModeExcludedClasses` call
+accumulates into the dev-mode set, which then *replaces* (not unions with) `excludedClasses` on first access. Any
+simplified retention would silently change that.
+
+The remaining eleven configuration setters stay as `@Deprecated` methods with their `@Inject` annotations removed.
+They keep mutating that instance exactly as they do now. Deprecation is by annotation only — no runtime warnings,
+which would flood test output given roughly 110 direct call sites across core and plugins.
+
+`useAllowStaticFieldAccess` retains its side effect of calling `useExcludedClasses(Class.class.getName())`, and
+the configuration bean must reproduce that accumulation exactly.
+
+### Why setter injection rather than constructor injection
+
+Constructor injection would be the obvious way to guarantee ordering, but it forces a constructor signature
+change. A user subclass calling `super(providerAllowlist, threadAllowlist)` — precisely the shape of the existing
+`ExternalSecurityMemberAccess` test fixture — would then either fail to compile, or, if a deprecated 2-arg
+overload were retained, compile cleanly and silently run with empty exclusions. **That is a fail-open hole**, and
+the kind that fails silently rather than loudly.
+
+Setter injection avoids it: `ContainerImpl.addInjectors` recurses into superclasses first
+(`ContainerImpl.java:97`), so inherited `@Inject` setters are injected on subclass instances. Existing subclasses
+keep compiling *and* receive the configuration.
+
+Injection ordering is safe by construction. Today the setters survive unspecified ordering only because they
+*accumulate* rather than assign, making them commutative — a subtlety that is easy to destroy accidentally. After
+this change `SecurityMemberAccess` has exactly one injected member touching those fields, so ordering stops
+mattering at all.
+
+A null configuration is also safe: the eight direct `new SecurityMemberAccess(null, null)` test sites never have
+the setter called, so their fields keep today's hardcoded defaults. Reads only ever touch fields, never the
+configuration object, so there is no null path on the access path.
+
+### Allowlist union
+
+With the sets precomputed per container, `isClassAllowlisted` collapses to a single set and a single walk:
+
+```java
+|| isClassBelongsToPackages(clazz, allowlistPackageNamesUnion);
+```
+
+This deletes the three-argument `isClassBelongsToPackages` overload and the two-set parameters on
+`isPackageBelongsToPackages`, resolving WW-5678's first item as a side effect.
+
+The ticket flagged this as a fail-open hazard: if the union were computed in two places — once seeded from
+configuration, once when the deprecated `useAllowlistPackageNames` setter fires — the two could drift, silently
+dropping `ALLOWLIST_REQUIRED_PACKAGES` from the allowlist with nothing failing loudly. Both routes therefore
+funnel through one private method, so exactly one line in the codebase computes the union:
+
+```java
+private void applyAllowlistPackageNames(Set<String> names) {
+ this.allowlistPackageNames = names;
+ this.allowlistPackageNamesUnion = union(ALLOWLIST_REQUIRED_PACKAGES, names);
+}
+```
+
+`isPackageBelongsToPackages` currently early-returns on `first.isEmpty() && second.isEmpty()`. Since
+`ALLOWLIST_REQUIRED_PACKAGES` is never empty, that guard simply stops firing on the allowlist path; the exclusion
+path, where both sets genuinely can be empty, keeps it. The guard was only ever an optimization, so this is not a
+semantic change.
+
+## Data flow
+
+| Tier | Frequency | Work |
+|---|---|---|
+| `SecurityMemberAccessConfig` construction | Once per container | All parsing, class validation, pattern compilation, dev-mode resolution, union precomputation |
+| `useConfig` | Once per `SecurityMemberAccess` | About a dozen immutable reference copies |
+| `ParametersInterceptor` | Once per request | Sets `acceptProperties`/`excludeProperties` on the instance (unchanged) |
+| `isAccessible` | Per OGNL member access | Field reads only |
+
+## Error handling
+
+Parsing failures — `ConfigurationException` for an unloadable class, an invalid regex, or whitespace in a package
+name — move from being thrown on every instantiation to being thrown once, when the singleton is first built.
+Still fatal, still loud, just earlier and once. Nothing degrades to a warning.
+
+The `struts.allowlist.enable=false` warning already dedupes via `logWarningForFirstOccurrence`; moving it to the
+configuration bean makes it a genuine once-per-container event.
+
+## Behaviour changes
+
+One, accepted during design review: the `"DevMode enabled, using DevMode excluded classes and packages for OGNL
+security enforcement!"` warning currently fires on the first OGNL access and will now fire when the configuration
+singleton is built. This makes it a deterministic startup signal rather than one contingent on traffic.
+
+No other externally visible behaviour changes. OGNL allow/deny semantics are identical.
+
+## Testing
+
+Core tests are JUnit 4 or extend `XWorkTestCase`. A JUnit 5 `@Test` added to these suites silently never runs.
+
+1. **Sharing proof.** Request several `SecurityMemberAccess` instances from one container and assert their
+ configuration-derived sets are reference-identical (`assertSame`, not `assertEquals`). Reference identity is a
+ dependency-free proof that no re-parsing occurred, since any re-parse necessarily produces a fresh set. Backed
+ by a counting probe asserting exactly one parse per container.
+2. **Instance isolation.** Calling a deprecated setter on one instance must not perturb a sibling instance or the
+ singleton. The sets are `unmodifiableSet`, so an in-place mutation bug would throw rather than corrupt
+ silently, but this invariant deserves an explicit assertion.
+3. **Subclass injection.** A subclass declaring the 2-arg constructor and calling
+ `super(providerAllowlist, threadAllowlist)` must receive the configuration through the inherited setter. This
+ is the test that would catch a future refactor to constructor injection reintroducing the fail-open hole.
+4. **Behaviour preservation.** Following WW-5674's differential pattern: for default, dev-mode, and custom
+ configurations, the sets the new bean publishes must equal what a legacy-style accumulation produces. This is
+ where the `useAllowStaticFieldAccess` → `useExcludedClasses` side effect gets pinned down.
+5. **Dev-mode.** With `struts.devMode=true` the effective sets are the dev-mode ones from the start, with no OGNL
+ access required to trigger the switch.
+
+The principal safety net is the existing suite. `SecurityMemberAccessTest` and its siblings drive these setters
+directly from roughly 110 call sites across core and plugins and must pass untouched. If the deprecated setters
+have kept their exact semantics, that suite cannot tell the difference — the strongest available evidence that
+OGNL allow/deny semantics are unchanged.
+
+## Risks
+
+| Risk | Mitigation |
+|---|---|
+| Shared sets mutated in place, poisoning every instance in the container | Sets are already `unmodifiableSet`; test 2 asserts isolation explicitly |
+| Allowlist union drifts from `ALLOWLIST_REQUIRED_PACKAGES` (fail-open) | Single computation site; test 4 covers custom allowlist configurations |
+| Configuration bean fails to reproduce the accumulate-not-assign semantics | Test 4 is differential against the legacy accumulation, not against hand-written expectations |
+| A future refactor moves configuration to constructor injection, reintroducing the silent fail-open | Test 3 encodes the subclass contract; the rationale is recorded above and in the class Javadoc |
+| `Initializable` is documented "should be only used internally" | The bean is internal and unaliased; `DefaultValidatorFactory` is the existing precedent |
+
+## Follow-ups
+
+- **8.0.0 — remove the deprecated configuration setters.** The eleven methods left on `SecurityMemberAccess`
+ should be removed once the major version allows it. To be filed as its own ticket, cross-referencing WW-5675 and
+ WW-5678.
+- **WW-5678** — its first item (the package-private overload sharing a name with a public method) is resolved for
+ free here by the union collapse. The remaining visibility narrowing stays with that ticket.
+- **WW-5667** — the parent should be updated to note that this ticket, not WW-5674, is the one expected to move
+ the reported 9%.
(struts) 06/15: WW-5675 perf(config): hoist the whitespace pattern in validatePackageNames
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 75b162f9613abc55b6d8db8c250fefc80e9f5e18
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:07:39 2026 +0200
WW-5675 perf(config): hoist the whitespace pattern in validatePackageNames
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../org/apache/struts2/util/ConfigParseUtil.java | 4 ++-
.../apache/struts2/util/ConfigParseUtilTest.java | 30 ++++++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java b/core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java
index 116ca637d..06dc1a43c 100644
--- a/core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java
+++ b/core/src/main/java/org/apache/struts2/util/ConfigParseUtil.java
@@ -47,6 +47,8 @@ public class ConfigParseUtil {
.maximumSize(MAX_CLASSLOADER_CACHE_SIZE)
.build();
+ private static final Pattern WHITESPACE = Pattern.compile("\\s");
+
private ConfigParseUtil() {
}
@@ -140,7 +142,7 @@ public class ConfigParseUtil {
}
public static void validatePackageNames(Collection<String> packageNames) {
- if (packageNames.stream().anyMatch(s -> Pattern.compile("\\s").matcher(s).find())) {
+ if (packageNames.stream().anyMatch(s -> WHITESPACE.matcher(s).find())) {
throw new ConfigurationException("Excluded package names could not be parsed due to erroneous whitespace characters: " + packageNames);
}
}
diff --git a/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java
index 84cf9a0a3..6e9c79727 100644
--- a/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java
+++ b/core/src/test/java/org/apache/struts2/util/ConfigParseUtilTest.java
@@ -29,9 +29,11 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Map;
import java.util.Set;
+import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
@@ -187,6 +189,34 @@ public class ConfigParseUtilTest {
innerCache.estimatedSize() <= limit);
}
+ @Test
+ public void validatePackageNamesAcceptsNamesWithoutWhitespace() {
+ ConfigParseUtil.validatePackageNames(Set.of("java.lang", "org.apache.struts2", ""));
+ }
+
+ @Test
+ public void validatePackageNamesRejectsSpace() {
+ assertThrows(ConfigurationException.class,
+ () -> ConfigParseUtil.validatePackageNames(Set.of("java.lang", "org.apache struts2")));
+ }
+
+ @Test
+ public void validatePackageNamesRejectsTab() {
+ assertThrows(ConfigurationException.class,
+ () -> ConfigParseUtil.validatePackageNames(Set.of("java\tlang")));
+ }
+
+ @Test
+ public void validatePackageNamesRejectsNewline() {
+ assertThrows(ConfigurationException.class,
+ () -> ConfigParseUtil.validatePackageNames(Set.of("java\nlang")));
+ }
+
+ @Test
+ public void validatePackageNamesAcceptsEmptyCollection() {
+ ConfigParseUtil.validatePackageNames(List.of());
+ }
+
@SuppressWarnings("unchecked")
private static Cache<Object, Object> validatedClassCache() {
try {
(struts) 09/15: WW-5675 test(ognl): strengthen SecurityMemberAccessConfigSharingTest assertions
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 02fa6aa9e8248e6745951fbee629cf77849124c8
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:29:59 2026 +0200
WW-5675 test(ognl): strengthen SecurityMemberAccessConfigSharingTest assertions
Two review findings: (1) the config-derived-set assertions compared instance to
instance only, which is vacuous for every emptySet()-defaulted field since
Collections.emptySet() is a JVM-wide singleton shared by both the config bean's
own default and SecurityMemberAccess's own default; deleting a useConfig
assignment for such a field would still pass. Fixed by additionally asserting
each field directly against the shared SecurityMemberAccessConfig bean, with
the container reloaded to set every relevant constant away from its hardcoded
default so the comparison is not itself vacuous by coincidence. (2)
testConfigBeanIsASingleton passed on assertSame(null, null) when the bean
was not registered at all; added assertNotNull before the identity check.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../SecurityMemberAccessConfigSharingTest.java | 64 +++++++++++++++++++++-
1 file changed, 62 insertions(+), 2 deletions(-)
diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
index b6af1db29..5680e59fb 100644
--- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
+++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessConfigSharingTest.java
@@ -18,8 +18,10 @@
*/
package org.apache.struts2.ognl;
+import org.apache.struts2.StrutsConstants;
import org.apache.struts2.XWorkTestCase;
+import java.util.Map;
import java.util.Set;
public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
@@ -27,10 +29,38 @@ public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
/**
* Reference identity proves no re-parsing occurred: any re-parse necessarily
* allocates a fresh set.
+ * <p>
+ * The instance-to-instance {@code assertSame} calls below are necessary but not sufficient:
+ * for a field whose default is {@link java.util.Collections#emptySet()}, two independently
+ * <em>unseeded</em> instances would also compare same, since {@code emptySet()} returns a
+ * JVM-wide singleton. Only {@code excludedClasses}, whose default {@code Set.of(...)} allocates
+ * a fresh instance per object, is proven by the instance-to-instance form alone. Every field is
+ * therefore additionally compared directly against the shared {@link SecurityMemberAccessConfig}
+ * bean, which fails on omission regardless of the default's identity.
+ * <p>
+ * That direct comparison is itself vacuous unless the configured value actually differs from the
+ * hardcoded default: {@code SecurityMemberAccess} and {@code SecurityMemberAccessConfig} share the
+ * same hardcoded defaults, so an unseeded field and a config parsed from an all-default container
+ * would also compare equal/same by coincidence. The container is therefore reloaded here with every
+ * relevant constant set away from its default, so a config value only matches the instance's field
+ * when {@code useConfig} actually ran.
*/
public void testConfigDerivedSetsAreSharedAcrossInstances() throws Exception {
+ loadButSet(Map.of(
+ StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, "false",
+ StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, "^org\\.apache\\.struts2\\.ognl\\.testpkg\\..*",
+ StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, "org.apache.struts2.ognl.testpkg",
+ StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, "java.lang.String",
+ StrutsConstants.STRUTS_ALLOWLIST_ENABLE, "true",
+ StrutsConstants.STRUTS_ALLOWLIST_CLASSES, "java.lang.String",
+ StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES, "org.apache.struts2.ognl.testpkg",
+ StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, "true",
+ StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, "true",
+ StrutsConstants.STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS, "true"));
+
SecurityMemberAccess first = container.getInstance(SecurityMemberAccess.class);
SecurityMemberAccess second = container.getInstance(SecurityMemberAccess.class);
+ SecurityMemberAccessConfig config = container.getInstance(SecurityMemberAccessConfig.class);
assertNotSame("expected a prototype bean", first, second);
@@ -41,11 +71,41 @@ public class SecurityMemberAccessConfigSharingTest extends XWorkTestCase {
Set<String> firstPackages = SecurityMemberAccessTest.reflectField(first, "excludedPackageNames");
Set<String> secondPackages = SecurityMemberAccessTest.reflectField(second, "excludedPackageNames");
assertSame("excluded package names were re-parsed per instance", firstPackages, secondPackages);
+
+ assertSame("excludedClasses not seeded from config",
+ config.getExcludedClasses(), SecurityMemberAccessTest.reflectField(first, "excludedClasses"));
+ assertSame("excludedPackageNamePatterns not seeded from config",
+ config.getExcludedPackageNamePatterns(), SecurityMemberAccessTest.reflectField(first, "excludedPackageNamePatterns"));
+ assertSame("excludedPackageNames not seeded from config",
+ config.getExcludedPackageNames(), SecurityMemberAccessTest.reflectField(first, "excludedPackageNames"));
+ assertSame("excludedPackageExemptClasses not seeded from config",
+ config.getExcludedPackageExemptClasses(), SecurityMemberAccessTest.reflectField(first, "excludedPackageExemptClasses"));
+ assertSame("allowlistClasses not seeded from config",
+ config.getAllowlistClasses(), SecurityMemberAccessTest.reflectField(first, "allowlistClasses"));
+ assertSame("allowlistPackageNames not seeded from config",
+ config.getAllowlistPackageNames(), SecurityMemberAccessTest.reflectField(first, "allowlistPackageNames"));
+
+ boolean firstAllowStaticFieldAccess = SecurityMemberAccessTest.reflectField(first, "allowStaticFieldAccess");
+ assertEquals("allowStaticFieldAccess not seeded from config",
+ config.isAllowStaticFieldAccess(), firstAllowStaticFieldAccess);
+ boolean firstEnforceAllowlistEnabled = SecurityMemberAccessTest.reflectField(first, "enforceAllowlistEnabled");
+ assertEquals("enforceAllowlistEnabled not seeded from config",
+ config.isEnforceAllowlistEnabled(), firstEnforceAllowlistEnabled);
+ boolean firstDisallowProxyObjectAccess = SecurityMemberAccessTest.reflectField(first, "disallowProxyObjectAccess");
+ assertEquals("disallowProxyObjectAccess not seeded from config",
+ config.isDisallowProxyObjectAccess(), firstDisallowProxyObjectAccess);
+ boolean firstDisallowProxyMemberAccess = SecurityMemberAccessTest.reflectField(first, "disallowProxyMemberAccess");
+ assertEquals("disallowProxyMemberAccess not seeded from config",
+ config.isDisallowProxyMemberAccess(), firstDisallowProxyMemberAccess);
+ boolean firstDisallowDefaultPackageAccess = SecurityMemberAccessTest.reflectField(first, "disallowDefaultPackageAccess");
+ assertEquals("disallowDefaultPackageAccess not seeded from config",
+ config.isDisallowDefaultPackageAccess(), firstDisallowDefaultPackageAccess);
}
public void testConfigBeanIsASingleton() {
- assertSame(container.getInstance(SecurityMemberAccessConfig.class),
- container.getInstance(SecurityMemberAccessConfig.class));
+ SecurityMemberAccessConfig instance = container.getInstance(SecurityMemberAccessConfig.class);
+ assertNotNull("SecurityMemberAccessConfig is not registered in the container", instance);
+ assertSame(instance, container.getInstance(SecurityMemberAccessConfig.class));
}
/**
(struts) 05/15: WW-5675 docs(ognl): fix Task 5 to handle the tests the signature change breaks
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 66113f81d6fed3f89d2b583a1212772a04ae1cb0
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:05:55 2026 +0200
WW-5675 docs(ognl): fix Task 5 to handle the tests the signature change breaks
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...14-WW-5675-share-parsed-ognl-security-config.md | 60 +++++++++++++++-------
1 file changed, 41 insertions(+), 19 deletions(-)
diff --git a/docs/superpowers/plans/2026-08-14-WW-5675-share-parsed-ognl-security-config.md b/docs/superpowers/plans/2026-08-14-WW-5675-share-parsed-ognl-security-config.md
index 4ce8dbc8e..bb6361f02 100644
--- a/docs/superpowers/plans/2026-08-14-WW-5675-share-parsed-ognl-security-config.md
+++ b/docs/superpowers/plans/2026-08-14-WW-5675-share-parsed-ognl-security-config.md
@@ -852,7 +852,41 @@ WW-5674 merged the two allowlist walks with a three-argument helper. With the co
- [ ] **Step 1: Write the failing test**
-Append to `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java`. Do not modify or delete the frozen oracles already in that file.
+Work in `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackageMatchingTest.java`. **Do not modify or delete the frozen oracles** (`legacyPrefixMatch`, `legacyToPackageName`) or the `PACKAGE_NAMES` / `CANDIDATE_SETS` / `classShapes()` fixtures. This file uses AssertJ (`assertThat`), not JUnit assertions — match that style.
+
+Narrowing the signatures in this task breaks three tests already in the file. Handle them exactly as follows; do not improvise, and do not delete a test merely because it fails to compile.
+
+**a. Convert `indexWalkMatchesLegacyAcrossPackageNameShapes` to the two-argument walk:**
+
+```java
+ @Test
+ public void indexWalkMatchesLegacyAcrossPackageNameShapes() {
+ for (String packageName : PACKAGE_NAMES) {
+ for (Set<String> candidates : CANDIDATE_SETS) {
+ assertThat(SecurityMemberAccess.isPackageBelongsToPackages(packageName, candidates))
+ .as("packageName=[%s] candidates=%s", packageName, candidates)
+ .isEqualTo(legacyPrefixMatch(packageName, candidates));
+ }
+ }
+ }
+```
+
+**b. Convert `bothSetsEmptyShortCircuitsToFalse`, renaming it since there is now one set:**
+
+```java
+ @Test
+ public void emptyCandidateSetShortCircuitsToFalse() {
+ for (String packageName : PACKAGE_NAMES) {
+ assertThat(SecurityMemberAccess.isPackageBelongsToPackages(packageName, emptySet()))
+ .as("packageName=[%s] with no configured packages", packageName)
+ .isFalse();
+ }
+ }
+```
+
+**c. Delete `twoSetOverloadEqualsDisjunctionOfSingleSetCalls` entirely.** It characterises the equivalence of the three-argument overload against two single-set calls; that overload no longer exists, so the property it asserts is gone. This is the one test in this file you may remove.
+
+**d. Append these two new tests:**
```java
/**
@@ -866,9 +900,7 @@ Append to `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackag
Set<String> union = SecurityMemberAccessTest.reflectField(sma, "allowlistPackageNamesUnion");
- assertTrue("configured package missing", union.contains("com.example.app"));
- assertTrue("required Struts package dropped from the allowlist",
- union.contains("org.apache.struts2.components"));
+ assertThat(union).contains("com.example.app", "org.apache.struts2.components");
}
@Test
@@ -877,20 +909,10 @@ Append to `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessPackag
Set<String> union = SecurityMemberAccessTest.reflectField(sma, "allowlistPackageNamesUnion");
- assertTrue(union.contains("org.apache.struts2.components"));
- assertTrue(union.contains("org.apache.struts2.views.jsp"));
- assertTrue(union.contains("org.apache.struts2.validator.validators"));
- }
-
- @Test
- public void singleSetWalkMatchesTheFrozenOracle() {
- for (String packageName : PACKAGE_NAMES) {
- for (Set<String> candidates : CANDIDATE_SETS) {
- assertEquals("mismatch for " + packageName + " against " + candidates,
- legacyPrefixMatch(packageName, candidates),
- SecurityMemberAccess.isPackageBelongsToPackages(packageName, candidates));
- }
- }
+ assertThat(union).contains(
+ "org.apache.struts2.components",
+ "org.apache.struts2.views.jsp",
+ "org.apache.struts2.validator.validators");
}
```
@@ -1017,7 +1039,7 @@ Remove the now-unused `emptySet` static import only if no other usage remains
- [ ] **Step 6: Run the package-matching tests**
Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessPackageMatchingTest`
-Expected: PASS, 12 tests.
+Expected: PASS. The file had 9 tests; one is deleted and two are added, so expect 10.
- [ ] **Step 7: Run the security suites**
(struts) 11/15: WW-5675 fix(ognl): register SecurityMemberAccessConfig for the Dispatcher container
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5675-share-parsed-ognl-security-config
in repository https://gitbox.apache.org/repos/asf/struts.git
commit cb162c6316c08870b4992800b8e5b35a0494e8da
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Fri Aug 14 14:44:31 2026 +0200
WW-5675 fix(ognl): register SecurityMemberAccessConfig for the Dispatcher container
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
core/src/main/resources/struts-beans.xml | 1 +
1 file changed, 1 insertion(+)
diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml
index 2de4ebc40..84f0919dc 100644
--- a/core/src/main/resources/struts-beans.xml
+++ b/core/src/main/resources/struts-beans.xml
@@ -174,6 +174,7 @@
class="org.apache.struts2.ognl.StrutsOgnlGuard"/>
<bean class="org.apache.struts2.ognl.ProviderAllowlist"/>
<bean class="org.apache.struts2.ognl.ThreadAllowlist"/>
+ <bean class="org.apache.struts2.ognl.SecurityMemberAccessConfig"/>
<bean type="org.apache.struts2.util.TextParser" name="struts"
class="org.apache.struts2.util.OgnlTextParser" scope="singleton"/>