This is an automated email from the ASF dual-hosted git repository.

lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git


The following commit(s) were added to refs/heads/main by this push:
     new d906f2344 WW-3530 Fix visitor-validator cache-key collision under 
wildcard actions (#1811)
d906f2344 is described below

commit d906f23448cb56216a6c316ae01f0d288b68571b
Author: Lukasz Lenart <[email protected]>
AuthorDate: Wed Jul 29 07:55:22 2026 +0200

    WW-3530 Fix visitor-validator cache-key collision under wildcard actions 
(#1811)
    
    * WW-3530 docs: add design spec for visitor-validator cache-key fix
    
    Co-Authored-By: Claude Opus 4.8 <[email protected]>
    
    * WW-3530 docs: add implementation plan for visitor-validator cache-key fix
    
    Co-Authored-By: Claude Opus 4.8 <[email protected]>
    
    * WW-3530 test(core): cover visitor-validator cache-key context handling 
under wildcard actions
    
    Co-Authored-By: Claude Opus 4.8 <[email protected]>
    
    * WW-3530 fix(core): keep visitor-validator context in cache key under 
wildcard actions
    
    Apply the wildcard config-name substitution only when validating the 
action's
    own class. Visited objects carry a stable, explicit visitor context that 
must
    remain part of the cache key, otherwise two visitor validators on one field 
with
    different contexts collide and the second is silently dropped.
    
    Fixes https://issues.apache.org/jira/browse/WW-3530
    
    Co-Authored-By: Claude Opus 4.8 <[email protected]>
    
    * WW-3530 docs: document <s:form> render-path caching limitation
    
    Co-Authored-By: Claude Opus 4.8 <[email protected]>
    
    * WW-3530 docs: correct WW-2996 scope claim and note default-context 
visitor limitation
    
    Final-review finding: default-context visitor validators under wildcard 
actions
    key on the volatile resolved action name for the visited class, 
reintroducing
    bounded WW-2996-style cache growth (memory only; correct validators still 
load).
    Correct the 'WW-2996 untouched' wording to 'untouched for the action's own 
class',
    document the subpath as an accepted limitation folded into the follow-up 
ticket,
    and clarify that end-to-end visitor execution is covered by existing 
visitor suites.
    
    Co-Authored-By: Claude Opus 4.8 <[email protected]>
    
    * WW-3530 chore(core): add DEBUG logging for validator cache-key branch 
decision
    
    Log the built key together with clazz, context, validatingActionClass, 
wildcard,
    and the action config name, so the wildcard-vs-visited-object branch taken 
in
    buildValidatorKey can be diagnosed at runtime.
    
    Co-Authored-By: Claude Opus 4.8 <[email protected]>
    
    * WW-3530 test(core): use assertNotEquals for cache-key inequality; fix 
comment grammar
    
    Address SonarCloud S5785 (assertFalse+equals -> assertNotEquals) and a 
Copilot
    grammar nit in the WW-4536 comment. DEBUG logging kept as-is per author 
decision.
    
    Co-Authored-By: Claude Opus 4.8 <[email protected]>
    
    ---------
    
    Co-authored-by: Claude Opus 4.8 <[email protected]>
---
 .../AnnotationActionValidatorManager.java          |  24 ++-
 .../AnnotationActionValidatorManagerTest.java      |  41 ++++
 ...26-07-25-WW-3530-visitor-validator-cache-key.md | 224 +++++++++++++++++++++
 ...5-WW-3530-visitor-validator-cache-key-design.md | 164 +++++++++++++++
 4 files changed, 449 insertions(+), 4 deletions(-)

diff --git 
a/core/src/main/java/org/apache/struts2/validator/AnnotationActionValidatorManager.java
 
b/core/src/main/java/org/apache/struts2/validator/AnnotationActionValidatorManager.java
index c2d5ca1e1..a2932a379 100644
--- 
a/core/src/main/java/org/apache/struts2/validator/AnnotationActionValidatorManager.java
+++ 
b/core/src/main/java/org/apache/struts2/validator/AnnotationActionValidatorManager.java
@@ -23,6 +23,8 @@ import org.apache.struts2.ActionInvocation;
 import org.apache.struts2.ActionProxy;
 import org.apache.struts2.config.entities.ActionConfig;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -36,6 +38,8 @@ import java.util.List;
  */
 public class AnnotationActionValidatorManager extends 
DefaultActionValidatorManager {
 
+    private static final Logger LOG = 
LogManager.getLogger(AnnotationActionValidatorManager.class);
+
     @Override
     protected String buildValidatorKey(Class clazz, String context) {
         ActionInvocation invocation = 
ActionContext.getContext().getActionInvocation();
@@ -48,20 +52,32 @@ public class AnnotationActionValidatorManager extends 
DefaultActionValidatorMana
             sb.append(config.getPackageName());
             sb.append("/");
         }
+
+        Object action = invocation.getAction();
+        boolean validatingActionClass = action != null && 
clazz.equals(action.getClass());
+        String configName = config.getName();
+        boolean wildcard = configName.contains(ActionConfig.WILDCARD)
+                || (configName.contains("{") && configName.contains("}"));
+
         // WW-2996: key needs to use the name of the action from the config 
file, instead of the url,
         // so wildcard actions will have the same validator
         // WW-3753: Using the config name instead of the context only for 
wildcard actions to keep the flexibility
         // provided by the original design (such as mapping different contexts 
to the same action and method if desired)
-        // WW-4536: Using NamedVariablePatternMatcher allows defines actions 
with patterns enclosed with '{}'
-        String configName = config.getName();
-        if (configName.contains(ActionConfig.WILDCARD) || 
(configName.contains("{") && configName.contains("}"))) {
+        // WW-4536: Using NamedVariablePatternMatcher allows defining actions 
with patterns enclosed with '{}'
+        // WW-3530: the config-name substitution only makes sense for the 
action's own class; a visited object
+        // (visitor validator) carries a stable, explicit context that must 
remain part of the key
+        if (validatingActionClass && wildcard) {
             sb.append(configName);
             sb.append("|");
             sb.append(proxy.getMethod());
         } else {
             sb.append(context);
         }
-        return sb.toString();
+
+        String validatorKey = sb.toString();
+        LOG.debug("Built validator key [{}] for class [{}] and context [{}]: 
validatingActionClass={}, wildcard={}, action config [{}]",
+                validatorKey, clazz.getName(), context, validatingActionClass, 
wildcard, configName);
+        return validatorKey;
     }
 
     @Override
diff --git 
a/core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java
 
b/core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java
index 029606409..125f3afd4 100644
--- 
a/core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java
+++ 
b/core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java
@@ -47,6 +47,8 @@ import org.easymock.EasyMock;
 
 import java.util.List;
 
+import static org.junit.Assert.assertNotEquals;
+
 
 
 /**
@@ -92,11 +94,50 @@ public class AnnotationActionValidatorManagerTest extends 
XWorkTestCase {
         super.tearDown();
     }
 
+    private void installInvocation(String configName, Object action) {
+        ActionConfig config = new ActionConfig.Builder("packageName", 
configName, "").build();
+        ActionInvocation invocation = 
EasyMock.createNiceMock(ActionInvocation.class);
+        ActionProxy proxy = EasyMock.createNiceMock(ActionProxy.class);
+
+        EasyMock.expect(invocation.getProxy()).andReturn(proxy).anyTimes();
+        EasyMock.expect(invocation.getAction()).andReturn(action).anyTimes();
+        EasyMock.expect(proxy.getMethod()).andReturn("execute").anyTimes();
+        EasyMock.expect(proxy.getConfig()).andReturn(config).anyTimes();
+
+        EasyMock.replay(invocation, proxy);
+        ActionContext.getContext().withActionInvocation(invocation);
+    }
+
     public void testBuildValidatorKey() {
         String validatorKey = 
annotationActionValidatorManager.buildValidatorKey(SimpleAnnotationAction.class,
 "name");
         assertEquals(SimpleAnnotationAction.class.getName() + 
"/packageName/name", validatorKey);
     }
 
+    public void 
testBuildValidatorKeyKeepsContextForVisitedClassUnderWildcardAction() {
+        // Action is wildcard-mapped; the validated class is a visited model 
object
+        // (not the action class), so its explicit, stable context must be 
preserved.
+        installInvocation("*", new SimpleAnnotationAction());
+
+        String basicKey = 
annotationActionValidatorManager.buildValidatorKey(AnnotatedTestBean.class, 
"basic");
+        String additionalKey = 
annotationActionValidatorManager.buildValidatorKey(AnnotatedTestBean.class, 
"additional");
+
+        assertEquals(AnnotatedTestBean.class.getName() + "/packageName/basic", 
basicKey);
+        assertEquals(AnnotatedTestBean.class.getName() + 
"/packageName/additional", additionalKey);
+        assertNotEquals("visited-object keys must differ per context", 
additionalKey, basicKey);
+    }
+
+    public void 
testBuildValidatorKeyIgnoresContextForActionClassUnderWildcardAction() {
+        installInvocation("*", new SimpleAnnotationAction());
+
+        String key1 = 
annotationActionValidatorManager.buildValidatorKey(SimpleAnnotationAction.class,
 "foo");
+        String key2 = 
annotationActionValidatorManager.buildValidatorKey(SimpleAnnotationAction.class,
 "bar");
+
+        // WW-2996: a wildcard action's own validators share one cache entry 
across
+        // all resolved names, so the volatile context must NOT be part of the 
key.
+        assertEquals(key1, key2);
+        assertEquals(SimpleAnnotationAction.class.getName() + 
"/packageName/*|execute", key1);
+    }
+
     public void testBuildsValidatorsForAlias() {
         List validatorList = 
annotationActionValidatorManager.getValidators(SimpleAnnotationAction.class, 
alias);
 
diff --git 
a/docs/superpowers/plans/2026-07-25-WW-3530-visitor-validator-cache-key.md 
b/docs/superpowers/plans/2026-07-25-WW-3530-visitor-validator-cache-key.md
new file mode 100644
index 000000000..be6e22a13
--- /dev/null
+++ b/docs/superpowers/plans/2026-07-25-WW-3530-visitor-validator-cache-key.md
@@ -0,0 +1,224 @@
+# WW-3530 Visitor-Validator Cache-Key Fix 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:** Stop `AnnotationActionValidatorManager` from collapsing distinct 
visitor-validator `context` values into one cache key under wildcard actions, 
so multiple visitor validators on the same field run again.
+
+**Architecture:** Narrow the wildcard config-name substitution in 
`buildValidatorKey` so it fires only when the class being validated is the 
action's own class. Visited objects (whose `context` is a stable, explicit 
visitor param) always key on `context`, exactly like 
`DefaultActionValidatorManager`.
+
+**Tech Stack:** Java, Maven, JUnit 3-style `XWorkTestCase` (extends 
`junit.framework.TestCase`), EasyMock.
+
+## Global Constraints
+
+- Target version: **7.3.0** (SNAPSHOT is a placeholder; real version chosen at 
release).
+- Commit messages MUST be prefixed with the Jira ticket: `WW-3530 <type>: ...`.
+- Only `core` module is touched. Build/test with `mvn test -DskipAssembly -pl 
core`.
+- `core` tests are JUnit 3/4 — they extend `XWorkTestCase` and use `testXxx` 
method names with `junit.framework` assertions. An `@Test` annotation would 
silently never run. Do NOT convert to JUnit 5.
+- `ActionConfig.WILDCARD` == `"*"`. Named-pattern detection also matches names 
containing both `{` and `}`.
+- Change is behavior-preserving for: non-wildcard actions, and wildcard 
actions validating their own class (WW-2996 caching must stay intact).
+
+---
+
+## Task 1: Add failing tests for the cache-key logic
+
+**Files:**
+- Test: 
`core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java`
+
+**Interfaces:**
+- Consumes: `AnnotationActionValidatorManager.buildValidatorKey(Class clazz, 
String context)` (protected, same package as the test) — returns the cache key 
`String`.
+- Consumes: existing test classes already imported in the file — 
`SimpleAnnotationAction` (used as the action class) and `AnnotatedTestBean` 
(used as a visited/foreign class).
+- Produces: a private helper `installInvocation(String configName, Object 
action)` used by the new tests.
+
+- [ ] **Step 1: Add the mock-installer helper**
+
+Add this private method to `AnnotationActionValidatorManagerTest` (place it 
just after `tearDown`, before `testBuildValidatorKey`). It installs an 
`ActionInvocation` whose proxy/config drive `buildValidatorKey`, overriding the 
one from `setUp`:
+
+```java
+private void installInvocation(String configName, Object action) {
+    ActionConfig config = new ActionConfig.Builder("packageName", configName, 
"").build();
+    ActionInvocation invocation = 
EasyMock.createNiceMock(ActionInvocation.class);
+    ActionProxy proxy = EasyMock.createNiceMock(ActionProxy.class);
+
+    EasyMock.expect(invocation.getProxy()).andReturn(proxy).anyTimes();
+    EasyMock.expect(invocation.getAction()).andReturn(action).anyTimes();
+    EasyMock.expect(proxy.getMethod()).andReturn("execute").anyTimes();
+    EasyMock.expect(proxy.getConfig()).andReturn(config).anyTimes();
+
+    EasyMock.replay(invocation, proxy);
+    ActionContext.getContext().withActionInvocation(invocation);
+}
+```
+
+- [ ] **Step 2: Add the WW-3530 reproduction test (visited class under a 
wildcard action)**
+
+Add this test method to the class. Under a wildcard action, a *visited* 
object's key must include its stable context so `basic` and `additional` don't 
collide:
+
+```java
+public void 
testBuildValidatorKeyKeepsContextForVisitedClassUnderWildcardAction() {
+    // Action is wildcard-mapped; the validated class is a visited model object
+    // (not the action class), so its explicit, stable context must be 
preserved.
+    installInvocation("*", new SimpleAnnotationAction());
+
+    String basicKey = 
annotationActionValidatorManager.buildValidatorKey(AnnotatedTestBean.class, 
"basic");
+    String additionalKey = 
annotationActionValidatorManager.buildValidatorKey(AnnotatedTestBean.class, 
"additional");
+
+    assertEquals(AnnotatedTestBean.class.getName() + "/packageName/basic", 
basicKey);
+    assertEquals(AnnotatedTestBean.class.getName() + 
"/packageName/additional", additionalKey);
+    assertFalse("visited-object keys must differ per context", 
basicKey.equals(additionalKey));
+}
+```
+
+- [ ] **Step 3: Add the WW-2996 regression test (action's own class under a 
wildcard action)**
+
+Add this test method. When the validated class IS the action class under a 
wildcard, the key must ignore context and use the stable config name — 
preserving WW-2996 behavior:
+
+```java
+public void 
testBuildValidatorKeyIgnoresContextForActionClassUnderWildcardAction() {
+    installInvocation("*", new SimpleAnnotationAction());
+
+    String key1 = 
annotationActionValidatorManager.buildValidatorKey(SimpleAnnotationAction.class,
 "foo");
+    String key2 = 
annotationActionValidatorManager.buildValidatorKey(SimpleAnnotationAction.class,
 "bar");
+
+    // WW-2996: a wildcard action's own validators share one cache entry across
+    // all resolved names, so the volatile context must NOT be part of the key.
+    assertEquals(key1, key2);
+    assertEquals(SimpleAnnotationAction.class.getName() + 
"/packageName/*|execute", key1);
+}
+```
+
+- [ ] **Step 4: Run the new tests to verify they FAIL**
+
+Run:
+```bash
+mvn test -DskipAssembly -pl core 
-Dtest='AnnotationActionValidatorManagerTest#testBuildValidatorKeyKeepsContextForVisitedClassUnderWildcardAction+testBuildValidatorKeyIgnoresContextForActionClassUnderWildcardAction'
+```
+Expected: 
`testBuildValidatorKeyKeepsContextForVisitedClassUnderWildcardAction` FAILS — 
before the fix the visited-class key is `.../packageName/*|execute` for both 
contexts, so `basicKey` equals `additionalKey` and the 
`assertEquals(".../basic", ...)` assertion fails. 
`testBuildValidatorKeyIgnoresContextForActionClassUnderWildcardAction` PASSES 
already (documents the behavior to preserve).
+
+- [ ] **Step 5: Commit the tests**
+
+```bash
+git add 
core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java
+git commit -m "WW-3530 test(core): cover visitor-validator cache-key context 
handling under wildcard actions
+
+Co-Authored-By: Claude Opus 4.8 <[email protected]>"
+```
+
+---
+
+## Task 2: Narrow the wildcard substitution to the action class
+
+**Files:**
+- Modify: 
`core/src/main/java/org/apache/struts2/validator/AnnotationActionValidatorManager.java:40-65`
+- Test: 
`core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java`
 (from Task 1)
+
+**Interfaces:**
+- Produces: unchanged signature `protected String buildValidatorKey(Class 
clazz, String context)`. New behavior: config-name substitution applies only 
when `clazz.equals(invocation.getAction().getClass())`.
+
+- [ ] **Step 1: Replace the `buildValidatorKey` method body**
+
+Replace the whole method (currently lines 40-65) with:
+
+```java
+    @Override
+    protected String buildValidatorKey(Class clazz, String context) {
+        ActionInvocation invocation = 
ActionContext.getContext().getActionInvocation();
+        ActionProxy proxy = invocation.getProxy();
+        ActionConfig config = proxy.getConfig();
+
+        StringBuilder sb = new StringBuilder(clazz.getName());
+        sb.append("/");
+        if (StringUtils.isNotBlank(config.getPackageName())) {
+            sb.append(config.getPackageName());
+            sb.append("/");
+        }
+
+        Object action = invocation.getAction();
+        boolean validatingActionClass = action != null && 
clazz.equals(action.getClass());
+        String configName = config.getName();
+        boolean wildcard = configName.contains(ActionConfig.WILDCARD)
+                || (configName.contains("{") && configName.contains("}"));
+
+        // WW-2996: key needs to use the name of the action from the config 
file, instead of the url,
+        // so wildcard actions will have the same validator
+        // WW-3753: Using the config name instead of the context only for 
wildcard actions to keep the flexibility
+        // provided by the original design (such as mapping different contexts 
to the same action and method if desired)
+        // WW-4536: Using NamedVariablePatternMatcher allows defines actions 
with patterns enclosed with '{}'
+        // WW-3530: the config-name substitution only makes sense for the 
action's own class; a visited object
+        // (visitor validator) carries a stable, explicit context that must 
remain part of the key
+        if (validatingActionClass && wildcard) {
+            sb.append(configName);
+            sb.append("|");
+            sb.append(proxy.getMethod());
+        } else {
+            sb.append(context);
+        }
+        return sb.toString();
+    }
+```
+
+- [ ] **Step 2: Run the new tests to verify they PASS**
+
+Run:
+```bash
+mvn test -DskipAssembly -pl core 
-Dtest='AnnotationActionValidatorManagerTest#testBuildValidatorKeyKeepsContextForVisitedClassUnderWildcardAction+testBuildValidatorKeyIgnoresContextForActionClassUnderWildcardAction'
+```
+Expected: BUILD SUCCESS, both tests PASS.
+
+- [ ] **Step 3: Run the full validator test class for regressions**
+
+Run:
+```bash
+mvn test -DskipAssembly -pl core -Dtest='AnnotationActionValidatorManagerTest'
+```
+Expected: BUILD SUCCESS. In particular `testBuildValidatorKey` still passes 
(non-wildcard config `name`, null action → context branch → 
`<class>/packageName/name`).
+
+- [ ] **Step 4: Run the visitor-validator regression suites**
+
+Run:
+```bash
+mvn test -DskipAssembly -pl core 
-Dtest='VisitorFieldValidatorTest+VisitorFieldValidatorModelTest+DefaultActionValidatorManagerTest'
+```
+Expected: BUILD SUCCESS. Confirms non-wildcard visitor behavior and the 
`no-annotations` manager are unaffected.
+
+- [ ] **Step 5: Commit the fix**
+
+```bash
+git add 
core/src/main/java/org/apache/struts2/validator/AnnotationActionValidatorManager.java
+git commit -m "WW-3530 fix(core): keep visitor-validator context in cache key 
under wildcard actions
+
+Apply the wildcard config-name substitution only when validating the action's
+own class. Visited objects carry a stable, explicit visitor context that must
+remain part of the cache key, otherwise two visitor validators on one field 
with
+different contexts collide and the second is silently dropped.
+
+Fixes https://issues.apache.org/jira/browse/WW-3530
+
+Co-Authored-By: Claude Opus 4.8 <[email protected]>"
+```
+
+---
+
+## Task 3: Full core test run
+
+**Files:** none (verification only).
+
+- [ ] **Step 1: Run the full core module test suite**
+
+Run:
+```bash
+mvn test -DskipAssembly -pl core
+```
+Expected: BUILD SUCCESS with no new failures. This guards against any other 
caller of `buildValidatorKey` / `getValidators` relying on the old wildcard 
behavior for foreign classes (e.g. the `Form` component's client-side 
validation path).
+
+- [ ] **Step 2: If green, no commit needed**
+
+Verification-only task; nothing to commit if the suite passes.
+
+---
+
+## Self-Review Notes
+
+- **Test scope note:** Tasks 1-2 verify the fix at the `buildValidatorKey` 
unit level (distinct cache keys per context), a deterministic proxy for spec 
test #1. End-to-end "both visitor validators actually execute" is covered by 
the existing `VisitorFieldValidatorTest` / `VisitorFieldValidatorModelTest` 
suites (run as regression in Task 2 Step 4), not by a new integration test here.
+- **Spec coverage:** Root-cause narrowing → Task 2. Reproduction test 
(distinct cache entries per context for visited objects, spec test #1) → Task 1 
Step 2. WW-2996 regression guard (spec test #2) → Task 1 Step 3. Non-wildcard 
visitor + `DefaultActionValidatorManager` unchanged (spec test #3) → Task 2 
Step 4. Null-action guard (spec edge case) → `action != null` in Task 2 Step 1. 
Self-visiting wildcard limitation is intentionally not exercised (accepted 
known limitation per spec).
+- **Placeholder scan:** none.
+- **Type consistency:** `installInvocation(String, Object)`, 
`buildValidatorKey(Class, String)`, and 
`configName`/`wildcard`/`validatingActionClass` locals are consistent across 
tasks.
diff --git 
a/docs/superpowers/specs/2026-07-25-WW-3530-visitor-validator-cache-key-design.md
 
b/docs/superpowers/specs/2026-07-25-WW-3530-visitor-validator-cache-key-design.md
new file mode 100644
index 000000000..c12730723
--- /dev/null
+++ 
b/docs/superpowers/specs/2026-07-25-WW-3530-visitor-validator-cache-key-design.md
@@ -0,0 +1,164 @@
+# WW-3530: Fix visitor-validator cache-key collision under wildcard actions
+
+- **Jira:** [WW-3530](https://issues.apache.org/jira/browse/WW-3530)
+- **Component:** XML Validators
+- **Target version:** 7.3.0
+- **Date:** 2026-07-25
+
+## Problem
+
+When two visitor field validators are declared on the same field with different
+`context` params (e.g. `basic` and `additional`) and they visit an object of 
the
+same class, the second validator is silently ignored: both produce the same
+validator-cache key, so the first context's validators are returned for both 
and
+run twice.
+
+Originally reported in 2010 (v2.2.1) when the cache key omitted `context`
+entirely. Intervening changes (WW-2996, WW-3753, WW-4536) reworked the key so
+`context` is now included for normal actions — which fixed the report for
+non-wildcard actions. The defect still reproduces for **wildcard / 
named-pattern
+actions**, where the key intentionally drops `context`.
+
+### Root cause
+
+`AnnotationActionValidatorManager.buildValidatorKey(Class clazz, String 
context)`
+(`core/src/main/java/org/apache/struts2/validator/AnnotationActionValidatorManager.java`)
+substitutes the action's config name + method for `context` whenever the 
current
+action is a wildcard/named-pattern action:
+
+```java
+String configName = config.getName();
+if (configName.contains(ActionConfig.WILDCARD)
+        || (configName.contains("{") && configName.contains("}"))) {
+    sb.append(configName).append("|").append(proxy.getMethod());
+} else {
+    sb.append(context);
+}
+```
+
+The word "context" has two meanings:
+
+- **Action-level validation** (`ValidationInterceptor`): `clazz` is the action
+  class and `context` is the action name — for a wildcard action this is 
derived
+  from the URL and varies per request. Keying on it caused the WW-2996 memory
+  leak (one cache entry per resolved URL). WW-3753 correctly substitutes the
+  stable config name here.
+- **Visitor validation** (`VisitorFieldValidator`): `clazz` is the *visited
+  object's* class (not the action class) and `context` is the visitor's 
explicit,
+  hand-written `context` param — stable, and the only thing distinguishing
+  `basic` from `additional`.
+
+The bug is that the config-name substitution — designed for the action's own
+class under a wildcard — is applied **too broadly**, firing for visited objects
+too and wrongly discarding their stable `context`.
+
+## Fix
+
+Narrow the substitution to the case it was designed for: only swap in the 
config
+name when `clazz` is the action's own class. For any other class (a visited
+object), always key on `context`.
+
+```java
+protected String buildValidatorKey(Class clazz, String context) {
+    ActionInvocation invocation = 
ActionContext.getContext().getActionInvocation();
+    ActionProxy proxy = invocation.getProxy();
+    ActionConfig config = proxy.getConfig();
+
+    StringBuilder sb = new StringBuilder(clazz.getName());
+    sb.append("/");
+    if (StringUtils.isNotBlank(config.getPackageName())) {
+        sb.append(config.getPackageName()).append("/");
+    }
+
+    Object action = invocation.getAction();
+    boolean validatingActionClass = action != null && 
clazz.equals(action.getClass());
+    String configName = config.getName();
+    boolean wildcard = configName.contains(ActionConfig.WILDCARD)
+            || (configName.contains("{") && configName.contains("}"));
+
+    if (validatingActionClass && wildcard) {
+        // WW-2996/WW-3753/WW-4536: wildcard actions share validators across
+        // resolved names; key on the stable config name, not the volatile 
context.
+        sb.append(configName).append("|").append(proxy.getMethod());
+    } else {
+        // Normal actions AND all visited objects (WW-3530): context is stable.
+        sb.append(context);
+    }
+    return sb.toString();
+}
+```
+
+### Behavioral change
+
+The only difference: when `clazz` is **not** the action class (a visited object
+under a visitor validator), the key now includes `context` even if the current
+action is a wildcard. `basic` and `additional` therefore get distinct cache
+entries and both run. The wildcard-action caching path (WW-2996) is untouched
+**for the action's own class**, because the `configName|method` branch now 
fires
+only when `clazz == action.getClass()` — the request-validation hot path
+(`ValidationInterceptor` → `getValidators(action.getClass(), …)`) is fully
+preserved. See the default-context visitor note below for the one subpath where
+keying moves from stable to volatile.
+
+## Scope, edge cases, non-goals
+
+- **`DefaultActionValidatorManager`** (the `no-annotations` bean) already keys 
on
+  `clazz + "/" + context` unconditionally — no bug, no change.
+- **Null action guard:** if `invocation.getAction()` is null, fall through to 
the
+  `context` branch (safe default).
+- **Self-visiting wildcard action** (an action that visits an object of its 
*own*
+  class under a wildcard mapping, with two different contexts): still collides,
+  because `clazz == action.getClass()` takes the wildcard branch. Accepted 
known
+  limitation — extremely contrived, and resolving it would require 
reintroducing
+  the volatile-vs-stable-context ambiguity this fix avoids. Documented, not 
fixed.
+- **Default-context visitor under a wildcard action** (visitor validator with 
no
+  explicit `context` param): `VisitorFieldValidator` 
(`VisitorFieldValidator.java:141`)
+  defaults `visitorContext` to the *resolved* action name when `context == 
null`.
+  Under a wildcard action that name is volatile (per URL). Before this change 
the
+  visited object keyed on the stable `configName|method`; after it, keying on
+  `context` (= the volatile action name) creates one cache entry per resolved 
name
+  for that visited class — WW-2996-style cache growth. This is a 
**caching-efficiency**
+  regression only (correct validators still load; the growth is bounded by the
+  number of resolved wildcard names, narrower than the original WW-2996 leak), 
and
+  it disappears the moment the visitor validator declares an explicit 
`context`.
+  Accepted for this change; folded into the same follow-up ticket as the 
render-path
+  item (a clean fix needs the rejected "Approach C" — an explicit visitor 
signal
+  through the manager API).
+- **`<s:form>` client-side JS-validation render path** (`Form.getValidators`,
+  `Form.java:295`): this path resolves `actionClass` by *name* (via
+  `ServletUrlRenderer`), not from the live action instance. When the form 
targets
+  a different action than the one executing, or the live action is a 
Spring/CGLIB
+  proxy, `clazz.equals(action.getClass())` is false for the action's own 
top-level
+  lookup, so the key falls to `context` (= action name) instead of
+  `configName|method`. Effect is **caching efficiency only** — extra cache 
entries
+  per resolved action name on this render path; the correct validators still 
load,
+  because `buildValidatorConfigs` receives `context` directly, independent of 
the
+  key. The request-validation hot path (`ValidationInterceptor`) is unaffected:
+  there `clazz` is the live action instance's own class, so WW-2996 caching is
+  fully preserved. Accepted for this change; a follow-up ticket should refine 
the
+  discriminator for the render path (a robust fix needs an explicit visitor 
signal
+  threaded through the manager API — the rejected "Approach C").
+- **Non-goal:** no change to the `ActionValidatorManager` interface, to
+  `VisitorFieldValidator`, or to `DefaultActionValidatorManager`.
+
+## Testing
+
+Tests live in
+`core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java`
+(plus existing `VisitorFieldValidatorTest` / `VisitorFieldValidatorModelTest`
+for regression).
+
+1. **Bug reproduction (fails before, passes after):** a wildcard action with 
two
+   visitor validators on the same field, visiting the same class with different
+   contexts → assert **both** contexts' validators execute.
+2. **WW-2996 regression guard:** a wildcard action's own validators still 
resolve
+   to a single cache entry across two different resolved action names.
+3. **Existing coverage stays green:** non-wildcard visitor case and current
+   wildcard action-level validation behavior unchanged.
+
+## References
+
+- WW-3530 — this issue
+- WW-2996 — memory leak from keying on volatile wildcard action names
+- WW-3753 — introduced config-name substitution for wildcard actions
+- WW-4536 — extended it to `NamedVariablePatternMatcher` (`{...}`) actions

Reply via email to