This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feat/ognl-3.5-upgrade in repository https://gitbox.apache.org/repos/asf/struts.git
commit 803b5cbbd09e3f70b200c5fafbd6ede1a20eb194 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Apr 6 19:18:21 2026 +0200 WW-5326 docs: add Jira reference to spec and plan Co-Authored-By: Claude Opus 4.6 <[email protected]> --- ...04-04-hibernate-proxy-detection-optimization.md | 272 ++++++ .../plans/2026-04-06-ognl-3.5-upgrade.md | 1032 ++++++++++++++++++++ .../specs/2026-04-06-ognl-3.5-upgrade-design.md | 263 +++++ 3 files changed, 1567 insertions(+) diff --git a/docs/superpowers/plans/2026-04-04-hibernate-proxy-detection-optimization.md b/docs/superpowers/plans/2026-04-04-hibernate-proxy-detection-optimization.md new file mode 100644 index 000000000..cdc82a999 --- /dev/null +++ b/docs/superpowers/plans/2026-04-04-hibernate-proxy-detection-optimization.md @@ -0,0 +1,272 @@ +# Hibernate Proxy Detection Optimization + +> **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:** Eliminate `LinkageError` exceptions thrown when Hibernate is not on the classpath by detecting availability once at class-load time. + +**Architecture:** Add a static availability check in `StrutsProxyService` that probes for `org.hibernate.proxy.HibernateProxy` once during class initialization. All Hibernate-related methods short-circuit immediately when Hibernate is absent. Same pattern applied to deprecated `ProxyUtil`. + +**Tech Stack:** Java 17, JUnit 5, AssertJ, Mockito + +--- + +### Task 1: Add Hibernate Availability Check to StrutsProxyService + +**Files:** +- Modify: `core/src/main/java/org/apache/struts2/util/StrutsProxyService.java` +- Test: `core/src/test/java/org/apache/struts2/util/StrutsProxyServiceTest.java` + +- [ ] **Step 1: Write the failing test — verify no LinkageError is thrown when Hibernate classes are used** + +The existing tests already call `isHibernateProxy()` and `isHibernateProxyMember()` with non-Hibernate objects. We need a test that verifies the short-circuit behavior works correctly. Add this test to `StrutsProxyServiceTest.java`: + +```java +@Test +public void isHibernateProxyDoesNotThrowWhenCalledRepeatedly() { + // Verify that calling isHibernateProxy many times for different objects + // does not cause performance issues (no exceptions thrown internally) + for (int i = 0; i < 1000; i++) { + assertThat(proxyService.isHibernateProxy(new Object())).isFalse(); + } +} + +@Test +public void isHibernateProxyMemberDoesNotThrowWhenCalledRepeatedly() throws NoSuchMethodException { + Method method = Object.class.getMethod("toString"); + for (int i = 0; i < 1000; i++) { + assertThat(proxyService.isHibernateProxyMember(method)).isFalse(); + } +} +``` + +- [ ] **Step 2: Run tests to verify they pass (baseline — these pass even without the fix because Hibernate IS on the test classpath)** + +Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsProxyServiceTest#isHibernateProxyDoesNotThrowWhenCalledRepeatedly+isHibernateProxyMemberDoesNotThrowWhenCalledRepeatedly` +Expected: PASS + +- [ ] **Step 3: Add static Hibernate availability flag to StrutsProxyService** + +In `core/src/main/java/org/apache/struts2/util/StrutsProxyService.java`, add a static availability check at the top of the class and modify the three Hibernate methods to short-circuit: + +```java +// Add this field near the top of the class, after the class declaration: +private static final boolean HIBERNATE_AVAILABLE = isHibernateAvailable(); + +private static boolean isHibernateAvailable() { + try { + Class.forName("org.hibernate.proxy.HibernateProxy"); + return true; + } catch (ClassNotFoundException e) { + return false; + } +} +``` + +Then modify the three Hibernate methods to short-circuit: + +**`isHibernateProxy`** — change from: +```java +@Override +public boolean isHibernateProxy(Object object) { + try { + return object != null && HibernateProxy.class.isAssignableFrom(object.getClass()); + } catch (LinkageError ignored) { + return false; + } +} +``` +to: +```java +@Override +public boolean isHibernateProxy(Object object) { + if (!HIBERNATE_AVAILABLE || object == null) { + return false; + } + try { + return HibernateProxy.class.isAssignableFrom(object.getClass()); + } catch (LinkageError ignored) { + return false; + } +} +``` + +**`isHibernateProxyMember`** — change from: +```java +@Override +public boolean isHibernateProxyMember(Member member) { + try { + return hasMember(HibernateProxy.class, member); + } catch (LinkageError ignored) { + return false; + } +} +``` +to: +```java +@Override +public boolean isHibernateProxyMember(Member member) { + if (!HIBERNATE_AVAILABLE) { + return false; + } + try { + return hasMember(HibernateProxy.class, member); + } catch (LinkageError ignored) { + return false; + } +} +``` + +**`getHibernateProxyTarget`** — change from: +```java +@Override +public Object getHibernateProxyTarget(Object object) { + try { + return Hibernate.unproxy(object); + } catch (LinkageError ignored) { + return object; + } +} +``` +to: +```java +@Override +public Object getHibernateProxyTarget(Object object) { + if (!HIBERNATE_AVAILABLE) { + return object; + } + try { + return Hibernate.unproxy(object); + } catch (LinkageError ignored) { + return object; + } +} +``` + +- [ ] **Step 4: Run the full StrutsProxyService test suite** + +Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsProxyServiceTest` +Expected: All tests PASS + +- [ ] **Step 5: Run the Spring integration test suite** + +Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsProxyServiceSpringIntegrationTest` +Expected: All tests PASS + +- [ ] **Step 6: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/util/StrutsProxyService.java core/src/test/java/org/apache/struts2/util/StrutsProxyServiceTest.java +git commit -m "WW-5622 Optimize Hibernate proxy detection to avoid LinkageError exceptions + +Add static availability check for Hibernate classes in StrutsProxyService. +When Hibernate is not on the classpath, all Hibernate-related methods +short-circuit immediately without throwing/catching LinkageError. +This eliminates a significant performance penalty for applications +that don't use Hibernate." +``` + +--- + +### Task 2: Apply Same Fix to Deprecated ProxyUtil + +**Files:** +- Modify: `core/src/main/java/org/apache/struts2/util/ProxyUtil.java` + +- [ ] **Step 1: Add the same static availability check to ProxyUtil** + +In `core/src/main/java/org/apache/struts2/util/ProxyUtil.java`, add the same pattern: + +```java +// Add after the isProxyMemberCache field: +private static final boolean HIBERNATE_AVAILABLE = isHibernateAvailable(); + +private static boolean isHibernateAvailable() { + try { + Class.forName("org.hibernate.proxy.HibernateProxy"); + return true; + } catch (ClassNotFoundException e) { + return false; + } +} +``` + +Then modify the three Hibernate methods in ProxyUtil identically to Task 1: + +**`isHibernateProxy`**: +```java +@Deprecated(since = "7.2") +public static boolean isHibernateProxy(Object object) { + if (!HIBERNATE_AVAILABLE || object == null) { + return false; + } + try { + return HibernateProxy.class.isAssignableFrom(object.getClass()); + } catch (LinkageError ignored) { + return false; + } +} +``` + +**`isHibernateProxyMember`**: +```java +@Deprecated(since = "7.2") +public static boolean isHibernateProxyMember(Member member) { + if (!HIBERNATE_AVAILABLE) { + return false; + } + try { + return hasMember(HibernateProxy.class, member); + } catch (LinkageError ignored) { + return false; + } +} +``` + +**`getHibernateProxyTarget`**: +```java +@Deprecated(since = "7.2") +public static Object getHibernateProxyTarget(Object object) { + if (!HIBERNATE_AVAILABLE) { + return object; + } + try { + return Hibernate.unproxy(object); + } catch (LinkageError ignored) { + return object; + } +} +``` + +- [ ] **Step 2: Run existing ProxyUtil tests** + +Run: `mvn test -DskipAssembly -pl core -Dtest=ProxyUtilTest` +Expected: PASS (or if no dedicated test exists, run the SecurityMemberAccess tests which exercise ProxyUtil indirectly) + +Run: `mvn test -DskipAssembly -pl core -Dtest=SecurityMemberAccessTest` +Expected: PASS + +- [ ] **Step 3: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/util/ProxyUtil.java +git commit -m "WW-5622 Apply same Hibernate availability optimization to deprecated ProxyUtil" +``` + +--- + +### Task 3: Run Full Test Suite + +- [ ] **Step 1: Run all core tests** + +Run: `mvn test -DskipAssembly -pl core` +Expected: All tests PASS + +- [ ] **Step 2: Run spring plugin tests (exercises proxy detection heavily)** + +Run: `mvn test -DskipAssembly -pl plugins/spring` +Expected: All tests PASS + +- [ ] **Step 3: Run json plugin tests (StrutsJSONWriter has Hibernate-related class name checks)** + +Run: `mvn test -DskipAssembly -pl plugins/json` +Expected: All tests PASS diff --git a/docs/superpowers/plans/2026-04-06-ognl-3.5-upgrade.md b/docs/superpowers/plans/2026-04-06-ognl-3.5-upgrade.md new file mode 100644 index 000000000..ed90cb954 --- /dev/null +++ b/docs/superpowers/plans/2026-04-06-ognl-3.5-upgrade.md @@ -0,0 +1,1032 @@ +# OGNL 3.5.x Upgrade Implementation Plan + +> **Jira:** [WW-5326](https://issues.apache.org/jira/browse/WW-5326) +> +> **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:** Upgrade Struts from OGNL 3.4.10 to 3.5.0-BETA4, introducing `StrutsContext extends OgnlContext<StrutsContext>` and parameterizing all OGNL interface implementations with the new generic type. + +**Architecture:** Direct `StrutsContext` construction replaces `Ognl.createDefaultContext()`. All OGNL interfaces (`MemberAccess`, `PropertyAccessor`, `MethodAccessor`, `ClassResolver`, `TypeConverter`, `NullHandler`) gain `<StrutsContext>` type parameter. No behavioral changes — same security model, same accessor logic, same expression evaluation. + +**Tech Stack:** Java 17+, OGNL 3.5.0-BETA4, Maven, JUnit 5, AssertJ, Mockito + +--- + +## File Map + +### New Files +- `core/src/main/java/org/apache/struts2/ognl/StrutsContext.java` — Struts' OgnlContext subclass +- `core/src/test/java/org/apache/struts2/ognl/StrutsContextTest.java` — Unit tests for StrutsContext + +### Modified Files — Core Main (21 files) + +**Interfaces:** +- `core/src/main/java/org/apache/struts2/ognl/accessor/RootAccessor.java` — add `<StrutsContext>` to `PropertyAccessor`, `MethodAccessor`, `ClassResolver` + +**MemberAccess / TypeConverter / NullHandler:** +- `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java` — `implements MemberAccess<StrutsContext>`, change method params +- `core/src/main/java/org/apache/struts2/ognl/OgnlTypeConverterWrapper.java` — `implements ognl.TypeConverter<StrutsContext>`, change method params +- `core/src/main/java/org/apache/struts2/ognl/XWorkTypeConverterWrapper.java` — update `convertValue` to use `StrutsContext` +- `core/src/main/java/org/apache/struts2/ognl/OgnlNullHandlerWrapper.java` — `implements ognl.NullHandler<StrutsContext>`, change method params + +**PropertyAccessor implementations:** +- `core/src/main/java/org/apache/struts2/ognl/accessor/CompoundRootAccessor.java` — all method params `OgnlContext` → `StrutsContext` +- `core/src/main/java/org/apache/struts2/ognl/accessor/ObjectAccessor.java` — `extends ObjectPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/ObjectProxyPropertyAccessor.java` — `implements PropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/ParameterPropertyAccessor.java` — `extends ObjectPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/HttpParametersPropertyAccessor.java` — `extends ObjectPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkObjectPropertyAccessor.java` — `extends ObjectPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkCollectionPropertyAccessor.java` — `extends SetPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkEnumerationAccessor.java` — `extends EnumerationPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkIteratorPropertyAccessor.java` — `extends IteratorPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkListPropertyAccessor.java` — `extends ListPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMapPropertyAccessor.java` — `extends MapPropertyAccessor<StrutsContext>` +- `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessor.java` — `extends ObjectMethodAccessor<StrutsContext>` + +**Context creation / evaluation:** +- `core/src/main/java/org/apache/struts2/ognl/OgnlUtil.java` — `createDefaultContext` returns `StrutsContext`, `ensureOgnlContext` returns `StrutsContext`, `withRoot` takes `StrutsContext`, casts updated +- `core/src/main/java/org/apache/struts2/ognl/OgnlValueStack.java` — `setRoot` creates `StrutsContext` directly, field type `Map<String, Object> context` → `StrutsContext context` +- `core/src/main/java/org/apache/struts2/ognl/OgnlValueStackFactory.java` — update `OgnlRuntime.setPropertyAccessor`/`setMethodAccessor` calls (may need raw types at boundary) +- `core/src/main/java/org/apache/struts2/ognl/OgnlReflectionContextFactory.java` — deprecated, minimal change or remove + +### Modified Files — Core Main (non-OGNL package, 1 file) +- `core/src/main/java/org/apache/struts2/conversion/impl/DefaultTypeConverter.java` — imports `OgnlContext`, may need `StrutsContext` + +### Modified Files — POM (1 file) +- `pom.xml` — bump `ognl.version` from `3.4.10` to `3.5.0-BETA4` + +### Modified Files — Tiles Plugin Main (5 files) +- `plugins/tiles/src/main/java/org/apache/tiles/ognl/AnyScopePropertyAccessor.java` +- `plugins/tiles/src/main/java/org/apache/tiles/ognl/ScopePropertyAccessor.java` +- `plugins/tiles/src/main/java/org/apache/tiles/ognl/DelegatePropertyAccessor.java` +- `plugins/tiles/src/main/java/org/apache/tiles/ognl/NestedObjectDelegatePropertyAccessor.java` +- `plugins/tiles/src/main/java/org/apache/tiles/ognl/PropertyAccessorDelegateFactory.java` + +### Modified Files — Test (varies) +- Test files that create `OgnlContext` via `Ognl.createDefaultContext()` need updating +- Key tests: `SecurityMemberAccessTest`, `OgnlUtilTest`, `SecurityMemberAccessProxyTest` + +--- + +### Task 1: Bump OGNL Version + +**Files:** +- Modify: `pom.xml:126` + +- [ ] **Step 1: Update OGNL version property** + +In `pom.xml`, change the `ognl.version` property: + +```xml +<!-- Before --> +<ognl.version>3.4.10</ognl.version> + +<!-- After --> +<ognl.version>3.5.0-BETA4</ognl.version> +``` + +- [ ] **Step 2: Verify dependency resolves** + +Run: `mvn dependency:resolve -pl core -DskipAssembly 2>&1 | tail -20` + +Expected: OGNL 3.5.0-BETA4 resolves successfully. Build will NOT compile yet — that's expected. + +- [ ] **Step 3: Commit** + +```bash +git add pom.xml +git commit -m "WW-5326 build(deps): bump OGNL from 3.4.10 to 3.5.0-BETA4" +``` + +--- + +### Task 2: Create StrutsContext + +**Files:** +- Create: `core/src/main/java/org/apache/struts2/ognl/StrutsContext.java` +- Create: `core/src/test/java/org/apache/struts2/ognl/StrutsContextTest.java` + +- [ ] **Step 1: Write the test** + +```java +package org.apache.struts2.ognl; + +import ognl.ClassResolver; +import ognl.MemberAccess; +import ognl.TypeConverter; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +@SuppressWarnings("unchecked") +class StrutsContextTest { + + @Test + void shouldCreateContextWithRequiredMemberAccess() { + MemberAccess<StrutsContext> memberAccess = mock(MemberAccess.class); + var context = new StrutsContext(memberAccess); + + assertThat(context).isNotNull(); + assertThat(context.getMemberAccess()).isSameAs(memberAccess); + } + + @Test + void shouldCreateContextWithAllComponents() { + MemberAccess<StrutsContext> memberAccess = mock(MemberAccess.class); + ClassResolver<StrutsContext> classResolver = mock(ClassResolver.class); + TypeConverter<StrutsContext> typeConverter = mock(TypeConverter.class); + + var context = new StrutsContext(memberAccess, classResolver, typeConverter); + + assertThat(context.getMemberAccess()).isSameAs(memberAccess); + assertThat(context.getClassResolver()).isSameAs(classResolver); + assertThat(context.getTypeConverter()).isSameAs(typeConverter); + } + + @Test + void shouldSupportRootObject() { + MemberAccess<StrutsContext> memberAccess = mock(MemberAccess.class); + var root = new Object(); + var context = new StrutsContext(memberAccess); + context.withRoot(root); + + assertThat(context.getRoot()).isSameAs(root); + } + + @Test + void shouldImplementMapInterface() { + MemberAccess<StrutsContext> memberAccess = mock(MemberAccess.class); + var context = new StrutsContext(memberAccess); + + context.put("testKey", "testValue"); + assertThat(context.get("testKey")).isEqualTo("testValue"); + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsContextTest -Dsurefire.failIfNoSpecifiedTests=false 2>&1 | tail -10` + +Expected: FAIL — `StrutsContext` class does not exist yet. + +- [ ] **Step 3: Create StrutsContext class** + +```java +/* + * 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 ognl.ClassResolver; +import ognl.MemberAccess; +import ognl.OgnlContext; +import ognl.TypeConverter; + +/** + * Struts-specific OGNL evaluation context. Extends {@link OgnlContext} with the + * self-bounded generic parameter to enable type-safe access in all OGNL interface + * implementations ({@link MemberAccess}, {@link ognl.PropertyAccessor}, etc.). + * + * <p>Phase 1: minimal subclass delegating to super constructors. + * Future phases will promote stringly-typed map entries (e.g. {@code DENY_METHOD_EXECUTION}, + * {@code CREATE_NULL_OBJECTS}) to proper typed fields.</p> + * + * @since 7.2.0 + */ +public class StrutsContext extends OgnlContext<StrutsContext> { + + public StrutsContext(MemberAccess<StrutsContext> memberAccess) { + super(memberAccess); + } + + public StrutsContext(MemberAccess<StrutsContext> memberAccess, + ClassResolver<StrutsContext> classResolver) { + super(memberAccess, classResolver); + } + + public StrutsContext(MemberAccess<StrutsContext> memberAccess, + ClassResolver<StrutsContext> classResolver, + TypeConverter<StrutsContext> typeConverter) { + super(memberAccess, classResolver, typeConverter); + } +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsContextTest 2>&1 | tail -10` + +Expected: All 4 tests PASS. + +- [ ] **Step 5: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/ognl/StrutsContext.java core/src/test/java/org/apache/struts2/ognl/StrutsContextTest.java +git commit -m "WW-5326 feat(ognl): introduce StrutsContext extending OgnlContext<StrutsContext>" +``` + +--- + +### Task 3: Update Core Interfaces — RootAccessor, MemberAccess, TypeConverter, NullHandler + +These are the top-level interface/wrapper changes. Nothing compiles yet — that's fine, we're working through the type ripple. + +**Files:** +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/RootAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/OgnlTypeConverterWrapper.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/XWorkTypeConverterWrapper.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/OgnlNullHandlerWrapper.java` + +- [ ] **Step 1: Update RootAccessor interface** + +In `core/src/main/java/org/apache/struts2/ognl/accessor/RootAccessor.java`: + +```java +package org.apache.struts2.ognl.accessor; + +import ognl.ClassResolver; +import ognl.MethodAccessor; +import ognl.PropertyAccessor; +import org.apache.struts2.ognl.StrutsContext; + +/** + * @since 6.4.0 + */ +public interface RootAccessor extends PropertyAccessor<StrutsContext>, MethodAccessor<StrutsContext>, ClassResolver<StrutsContext> { +} +``` + +- [ ] **Step 2: Update SecurityMemberAccess** + +In `core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java`: + +Change the class declaration: + +```java +// Before +public class SecurityMemberAccess implements MemberAccess { +// After +public class SecurityMemberAccess implements MemberAccess<StrutsContext> { +``` + +Change the import from `import ognl.OgnlContext;` to `import org.apache.struts2.ognl.StrutsContext;` (remove the OgnlContext import if no longer needed). + +Change all three method signatures: + +```java +// Before +public Object setup(OgnlContext context, Object target, Member member, String propertyName) { +// After +public Object setup(StrutsContext context, Object target, Member member, String propertyName) { + +// Before +public void restore(OgnlContext context, Object target, Member member, String propertyName, Object state) { +// After +public void restore(StrutsContext context, Object target, Member member, String propertyName, Object state) { + +// Before +public boolean isAccessible(OgnlContext context, Object target, Member member, String propertyName) { +// After +public boolean isAccessible(StrutsContext context, Object target, Member member, String propertyName) { +``` + +- [ ] **Step 3: Update OgnlTypeConverterWrapper** + +In `core/src/main/java/org/apache/struts2/ognl/OgnlTypeConverterWrapper.java`: + +```java +// Before +public class OgnlTypeConverterWrapper implements ognl.TypeConverter { +// After +public class OgnlTypeConverterWrapper implements ognl.TypeConverter<StrutsContext> { + +// Before +public Object convertValue(OgnlContext context, Object target, Member member, String propertyName, Object value, Class<?> toType) { +// After +public Object convertValue(StrutsContext context, Object target, Member member, String propertyName, Object value, Class<?> toType) { +``` + +Replace `import ognl.OgnlContext;` with `// no longer needed` (remove it) since the method now takes `StrutsContext`. + +- [ ] **Step 4: Update XWorkTypeConverterWrapper** + +In `core/src/main/java/org/apache/struts2/ognl/XWorkTypeConverterWrapper.java`: + +The `convertValue` method takes `Map context` and casts to `OgnlContext`. After the upgrade, `ognl.TypeConverter<StrutsContext>.convertValue` passes `StrutsContext` directly. But `XWorkTypeConverterWrapper` implements the *Struts* `TypeConverter` interface (which takes `Map`), not the OGNL one. So this file's changes are minimal — just update the cast: + +```java +// Before +OgnlContext ognlContext = (context instanceof OgnlContext oc) ? oc : null; +if (ognlContext == null) { + throw new IllegalArgumentException("Context must be an OgnlContext for OGNL 3.4.8+"); +} +return typeConverter.convertValue(ognlContext, target, member, propertyName, value, toType); + +// After +StrutsContext strutsContext = (context instanceof StrutsContext sc) ? sc : null; +if (strutsContext == null) { + throw new IllegalArgumentException("Context must be a StrutsContext"); +} +return typeConverter.convertValue(strutsContext, target, member, propertyName, value, toType); +``` + +Add `import org.apache.struts2.ognl.StrutsContext;` and remove the `import ognl.OgnlContext;`. + +- [ ] **Step 5: Update OgnlNullHandlerWrapper** + +In `core/src/main/java/org/apache/struts2/ognl/OgnlNullHandlerWrapper.java`: + +```java +// Before +public class OgnlNullHandlerWrapper implements ognl.NullHandler { +// After +public class OgnlNullHandlerWrapper implements ognl.NullHandler<StrutsContext> { + +// Before +public Object nullMethodResult(OgnlContext context, Object target, String methodName, Object[] args) { +// After +public Object nullMethodResult(StrutsContext context, Object target, String methodName, Object[] args) { + +// Before +public Object nullPropertyValue(OgnlContext context, Object target, Object property) { +// After +public Object nullPropertyValue(StrutsContext context, Object target, Object property) { +``` + +Replace `import ognl.OgnlContext;` with `// removed`. + +- [ ] **Step 6: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/ognl/accessor/RootAccessor.java \ + core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java \ + core/src/main/java/org/apache/struts2/ognl/OgnlTypeConverterWrapper.java \ + core/src/main/java/org/apache/struts2/ognl/XWorkTypeConverterWrapper.java \ + core/src/main/java/org/apache/struts2/ognl/OgnlNullHandlerWrapper.java +git commit -m "WW-5326 refactor(ognl): parameterize core interfaces with StrutsContext" +``` + +--- + +### Task 4: Update PropertyAccessor Implementations + +All 13 accessor classes in `core/src/main/java/org/apache/struts2/ognl/accessor/`. The pattern is identical: add `<StrutsContext>` to the superclass/interface, change `OgnlContext` → `StrutsContext` in all method params. + +**Files:** +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/CompoundRootAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/ObjectAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/ObjectProxyPropertyAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/ParameterPropertyAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/HttpParametersPropertyAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkObjectPropertyAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkCollectionPropertyAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkEnumerationAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkIteratorPropertyAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkListPropertyAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMapPropertyAccessor.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/accessor/XWorkMethodAccessor.java` + +- [ ] **Step 1: Update CompoundRootAccessor** + +This is the largest accessor file. Change the class declaration (no change needed — it `implements RootAccessor` which is already parameterized). But all method signatures taking `OgnlContext` must change to `StrutsContext`: + +```java +// Add import +import org.apache.struts2.ognl.StrutsContext; + +// Change every method signature. Key methods: +public String getSourceAccessor(StrutsContext context, Object target, Object index) +public String getSourceSetter(StrutsContext context, Object target, Object index) +public void setProperty(StrutsContext context, Object target, Object name, Object value) throws OgnlException +public Object getProperty(StrutsContext context, Object target, Object name) throws OgnlException +public Object callMethod(StrutsContext context, Object target, String name, Object[] objects) throws MethodFailedException +public Object callStaticMethod(StrutsContext context, Class aClass, String s, Object[] objects) throws MethodFailedException +public Class classForName(String className, StrutsContext context) throws ClassNotFoundException +``` + +Remove `import ognl.OgnlContext;` if no longer used. + +- [ ] **Step 2: Update simple ObjectPropertyAccessor subclasses (6 files)** + +For each of these 6 files, apply the same pattern — add `<StrutsContext>` to the superclass and change `OgnlContext` → `StrutsContext` in method params: + +**ObjectAccessor.java:** +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class ObjectAccessor extends ObjectPropertyAccessor { +public class ObjectAccessor extends ObjectPropertyAccessor<StrutsContext> { +// Before: public Object getProperty(OgnlContext map, Object o, Object o1) throws OgnlException { +public Object getProperty(StrutsContext map, Object o, Object o1) throws OgnlException { +``` + +**ParameterPropertyAccessor.java:** +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class ParameterPropertyAccessor extends ObjectPropertyAccessor { +public class ParameterPropertyAccessor extends ObjectPropertyAccessor<StrutsContext> { +// Change both getProperty and setProperty params: OgnlContext → StrutsContext +``` + +**HttpParametersPropertyAccessor.java:** +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class HttpParametersPropertyAccessor extends ObjectPropertyAccessor { +public class HttpParametersPropertyAccessor extends ObjectPropertyAccessor<StrutsContext> { +// Change both getProperty and setProperty params: OgnlContext → StrutsContext +``` + +**XWorkObjectPropertyAccessor.java:** +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class XWorkObjectPropertyAccessor extends ObjectPropertyAccessor { +public class XWorkObjectPropertyAccessor extends ObjectPropertyAccessor<StrutsContext> { +// Change getProperty param: OgnlContext → StrutsContext +``` + +**XWorkEnumerationAccessor.java:** +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class XWorkEnumerationAccessor extends EnumerationPropertyAccessor { +public class XWorkEnumerationAccessor extends EnumerationPropertyAccessor<StrutsContext> { +// Change setProperty param: OgnlContext → StrutsContext +// Also change the ObjectPropertyAccessor import if needed +``` + +**XWorkIteratorPropertyAccessor.java:** +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class XWorkIteratorPropertyAccessor extends IteratorPropertyAccessor { +public class XWorkIteratorPropertyAccessor extends IteratorPropertyAccessor<StrutsContext> { +// Change setProperty param: OgnlContext → StrutsContext +// Also extends ObjectPropertyAccessor<StrutsContext> reference in import +``` + +- [ ] **Step 3: Update ObjectProxyPropertyAccessor** + +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class ObjectProxyPropertyAccessor implements PropertyAccessor { +public class ObjectProxyPropertyAccessor implements PropertyAccessor<StrutsContext> { + +// Change ALL method params: OgnlContext → StrutsContext +// getSourceAccessor, getSourceSetter, getProperty, setProperty, setupContext (private) +``` + +- [ ] **Step 4: Update XWorkCollectionPropertyAccessor** + +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class XWorkCollectionPropertyAccessor extends SetPropertyAccessor { +public class XWorkCollectionPropertyAccessor extends SetPropertyAccessor<StrutsContext> { + +// Change ALL method params: OgnlContext → StrutsContext +// getProperty, getSetMap, getPropertyThroughIteration, setProperty, getRealValue +``` + +- [ ] **Step 5: Update XWorkListPropertyAccessor** + +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class XWorkListPropertyAccessor extends ListPropertyAccessor { +public class XWorkListPropertyAccessor extends ListPropertyAccessor<StrutsContext> { + +// Change ALL method params: OgnlContext → StrutsContext +// getProperty, setProperty, getRealValue +// Also: the field type `XWorkCollectionPropertyAccessor _sAcc` and setter `setXWorkCollectionPropertyAccessor(PropertyAccessor acc)` — the PropertyAccessor param needs `<StrutsContext>` too +``` + +```java +// Before: public void setXWorkCollectionPropertyAccessor(PropertyAccessor acc) { +public void setXWorkCollectionPropertyAccessor(PropertyAccessor<StrutsContext> acc) { +``` + +- [ ] **Step 6: Update XWorkMapPropertyAccessor** + +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class XWorkMapPropertyAccessor extends MapPropertyAccessor { +public class XWorkMapPropertyAccessor extends MapPropertyAccessor<StrutsContext> { + +// Change ALL method params: OgnlContext → StrutsContext +// getProperty, setProperty, getValue (private), getKey (private) +``` + +- [ ] **Step 7: Update XWorkMethodAccessor** + +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class XWorkMethodAccessor extends ObjectMethodAccessor { +public class XWorkMethodAccessor extends ObjectMethodAccessor<StrutsContext> { + +// Change method params: OgnlContext → StrutsContext +// callMethod, callStaticMethod +``` + +- [ ] **Step 8: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/ognl/accessor/ +git commit -m "WW-5326 refactor(ognl): parameterize all accessor implementations with StrutsContext" +``` + +--- + +### Task 5: Update Context Creation — OgnlUtil, OgnlValueStack, OgnlValueStackFactory + +**Files:** +- Modify: `core/src/main/java/org/apache/struts2/ognl/OgnlUtil.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/OgnlValueStack.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/OgnlValueStackFactory.java` +- Modify: `core/src/main/java/org/apache/struts2/ognl/OgnlReflectionContextFactory.java` + +- [ ] **Step 1: Update OgnlUtil** + +Key changes in `core/src/main/java/org/apache/struts2/ognl/OgnlUtil.java`: + +Add import: +```java +import org.apache.struts2.ognl.StrutsContext; +``` + +Change `createDefaultContext` methods (lines 727-738): +```java +// Before +protected OgnlContext createDefaultContext(Object root) { + return createDefaultContext(root, null); +} + +protected OgnlContext createDefaultContext(Object root, ClassResolver resolver) { + if (resolver == null) { + resolver = container.getInstance(RootAccessor.class); + if (resolver == null) { + throw new IllegalStateException("Cannot find ClassResolver"); + } + } + return Ognl.createDefaultContext(root, container.getInstance(SecurityMemberAccess.class), resolver, defaultConverter); +} + +// After +protected StrutsContext createDefaultContext(Object root) { + return createDefaultContext(root, null); +} + +protected StrutsContext createDefaultContext(Object root, ClassResolver<StrutsContext> resolver) { + if (resolver == null) { + resolver = container.getInstance(RootAccessor.class); + if (resolver == null) { + throw new IllegalStateException("Cannot find ClassResolver"); + } + } + StrutsContext context = new StrutsContext( + container.getInstance(SecurityMemberAccess.class), resolver, defaultConverter); + context.withRoot(root); + return context; +} +``` + +Change `ensureOgnlContext` (lines 214-222): +```java +// Before +private OgnlContext ensureOgnlContext(Map<String, Object> context) { + if (context instanceof OgnlContext ognlContext) { + return ognlContext; + } + OgnlContext ognlContext = createDefaultContext(null); + ognlContext.putAll(context); + return ognlContext; +} + +// After +private StrutsContext ensureOgnlContext(Map<String, Object> context) { + if (context instanceof StrutsContext strutsContext) { + return strutsContext; + } + StrutsContext strutsContext = createDefaultContext(null); + strutsContext.putAll(context); + return strutsContext; +} +``` + +Change `withRoot` helper methods (lines 765-794) — `OgnlContext` → `StrutsContext`: +```java +// Before +private void withRoot(OgnlContext context, Object root, OgnlAction action) throws OgnlException { +// After +private void withRoot(StrutsContext context, Object root, OgnlAction action) throws OgnlException { + +// Before +private <T> T withRoot(OgnlContext context, Object root, OgnlSupplier<T> supplier) throws OgnlException { +// After +private <T> T withRoot(StrutsContext context, Object root, OgnlSupplier<T> supplier) throws OgnlException { +``` + +Change `ognlSet` and `ognlGet` casts (lines 427, 437): +```java +// Before +OgnlContext ognlContext = (OgnlContext) context; +// After +StrutsContext ognlContext = (StrutsContext) context; +``` + +Change `defaultConverter` field type (line 64): +```java +// Before +private TypeConverter defaultConverter; +// After — note: this is ognl.TypeConverter<StrutsContext> +private TypeConverter<StrutsContext> defaultConverter; +``` + +Update `setXWorkConverter` (line 92): +```java +// The OgnlTypeConverterWrapper now implements TypeConverter<StrutsContext>, so this is compatible +``` + +Update import: `import ognl.TypeConverter;` stays, but the `ClassResolver` import needs to be checked. Also change any remaining `OgnlContext` references in method signatures throughout the file to `StrutsContext`. + +- [ ] **Step 2: Update OgnlValueStack** + +Key changes in `core/src/main/java/org/apache/struts2/ognl/OgnlValueStack.java`: + +Change field type (line 71): +```java +// Before +protected transient Map<String, Object> context; +// After +protected transient StrutsContext context; +``` + +Add import: +```java +import org.apache.struts2.ognl.StrutsContext; +``` + +Change `setRoot` method (lines 121-130): +```java +// Before +protected void setRoot(XWorkConverter xworkConverter, RootAccessor accessor, CompoundRoot compoundRoot, SecurityMemberAccess securityMemberAccess) { + this.root = compoundRoot; + this.securityMemberAccess = securityMemberAccess; + OgnlContext ognlContext = Ognl.createDefaultContext(this.root, securityMemberAccess, accessor, new OgnlTypeConverterWrapper(xworkConverter)); + this.context = ognlContext; + this.converter = xworkConverter; + context.put(VALUE_STACK, this); + ognlContext.setTraceEvaluations(false); + ognlContext.setKeepLastEvaluation(false); +} + +// After +protected void setRoot(XWorkConverter xworkConverter, RootAccessor accessor, CompoundRoot compoundRoot, SecurityMemberAccess securityMemberAccess) { + this.root = compoundRoot; + this.securityMemberAccess = securityMemberAccess; + this.context = new StrutsContext(securityMemberAccess, accessor, new OgnlTypeConverterWrapper(xworkConverter)); + this.context.withRoot(this.root); + this.converter = xworkConverter; + context.put(VALUE_STACK, this); + context.setTraceEvaluations(false); + context.setKeepLastEvaluation(false); +} +``` + +Remove `import ognl.OgnlContext;` if no longer needed. Keep `import ognl.Ognl;` only if still used elsewhere in the file. + +Update `getContext()` return type if it currently returns `Map<String, Object>` — check the `ValueStack` interface. The `ValueStack.getContext()` returns `Map<String, Object>`, which `StrutsContext` satisfies (since `OgnlContext` implements `Map<String, Object>`). No interface change needed. + +- [ ] **Step 3: Update OgnlValueStackFactory** + +In `core/src/main/java/org/apache/struts2/ognl/OgnlValueStackFactory.java`: + +The `OgnlRuntime.setPropertyAccessor()` and `OgnlRuntime.setMethodAccessor()` calls now expect generic types. Since `OgnlRuntime` methods are static with their own generic parameter, and the accessors are now `PropertyAccessor<StrutsContext>` / `MethodAccessor<StrutsContext>`, this should work. But the `Container.getInstance(PropertyAccessor.class)` returns a raw type. Add `@SuppressWarnings("unchecked")` where needed: + +```java +// In registerPropertyAccessors(), the existing code should work since OgnlRuntime +// accepts the raw type at the boundary. If compiler errors occur, add: +@SuppressWarnings({"rawtypes", "unchecked"}) +``` + +- [ ] **Step 4: Update OgnlReflectionContextFactory** + +This class is `@Deprecated(forRemoval=true)`. Minimal change — keep it working with raw types: + +```java +@Override +public OgnlContext createDefaultContext(Object root) { + return Ognl.createDefaultContext(root); +} +``` + +This returns raw `OgnlContext` from the static OGNL method. Since the class is deprecated, this is acceptable. No change may even be needed if the raw type compiles. + +- [ ] **Step 5: Compile core module** + +Run: `mvn compile -DskipAssembly -pl core 2>&1 | tail -30` + +Expected: Core compiles. Fix any remaining type errors discovered here. + +- [ ] **Step 6: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/ognl/OgnlUtil.java \ + core/src/main/java/org/apache/struts2/ognl/OgnlValueStack.java \ + core/src/main/java/org/apache/struts2/ognl/OgnlValueStackFactory.java \ + core/src/main/java/org/apache/struts2/ognl/OgnlReflectionContextFactory.java +git commit -m "WW-5326 refactor(ognl): replace Ognl.createDefaultContext with direct StrutsContext construction" +``` + +--- + +### Task 6: Update Tiles Plugin + +The tiles plugin accessors use raw `PropertyAccessor` and take `OgnlContext` in method params. Since tiles runs inside Struts, parameterize with `StrutsContext`. + +**Files:** +- Modify: `plugins/tiles/src/main/java/org/apache/tiles/ognl/AnyScopePropertyAccessor.java` +- Modify: `plugins/tiles/src/main/java/org/apache/tiles/ognl/ScopePropertyAccessor.java` +- Modify: `plugins/tiles/src/main/java/org/apache/tiles/ognl/DelegatePropertyAccessor.java` +- Modify: `plugins/tiles/src/main/java/org/apache/tiles/ognl/NestedObjectDelegatePropertyAccessor.java` +- Modify: `plugins/tiles/src/main/java/org/apache/tiles/ognl/PropertyAccessorDelegateFactory.java` +- Modify: `plugins/tiles/src/main/java/org/apache/tiles/ognl/TilesContextPropertyAccessorDelegateFactory.java` + +- [ ] **Step 1: Update AnyScopePropertyAccessor** + +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class AnyScopePropertyAccessor implements PropertyAccessor { +public class AnyScopePropertyAccessor implements PropertyAccessor<StrutsContext> { + +// Change all 4 method params: OgnlContext → StrutsContext +``` + +- [ ] **Step 2: Update ScopePropertyAccessor** + +Same pattern as AnyScopePropertyAccessor: +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class ScopePropertyAccessor implements PropertyAccessor { +public class ScopePropertyAccessor implements PropertyAccessor<StrutsContext> { + +// Change all 4 method params: OgnlContext → StrutsContext +``` + +- [ ] **Step 3: Update PropertyAccessorDelegateFactory** + +```java +import ognl.PropertyAccessor; +import org.apache.struts2.ognl.StrutsContext; + +public interface PropertyAccessorDelegateFactory<T> { + // Before: PropertyAccessor getPropertyAccessor(String propertyName, T obj); + PropertyAccessor<StrutsContext> getPropertyAccessor(String propertyName, T obj); +} +``` + +- [ ] **Step 4: Update TilesContextPropertyAccessorDelegateFactory** + +Update field types and constructor params: +```java +import org.apache.struts2.ognl.StrutsContext; + +// Before: private final PropertyAccessor objectPropertyAccessor; +private final PropertyAccessor<StrutsContext> objectPropertyAccessor; +// Same for applicationContextPropertyAccessor, anyScopePropertyAccessor, scopePropertyAccessor + +// Constructor params: +public TilesContextPropertyAccessorDelegateFactory( + PropertyAccessor<StrutsContext> objectPropertyAccessor, + PropertyAccessor<StrutsContext> applicationContextPropertyAccessor, + PropertyAccessor<StrutsContext> anyScopePropertyAccessor, + PropertyAccessor<StrutsContext> scopePropertyAccessor) + +// Return type: +public PropertyAccessor<StrutsContext> getPropertyAccessor(String propertyName, Request request) +``` + +- [ ] **Step 5: Update DelegatePropertyAccessor** + +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class DelegatePropertyAccessor<T> implements PropertyAccessor { +public class DelegatePropertyAccessor<T> implements PropertyAccessor<StrutsContext> { + +// Before: private final PropertyAccessorDelegateFactory<T> factory; +// (no change — factory return type already updated) + +// Change all 4 method params: OgnlContext → StrutsContext +``` + +- [ ] **Step 6: Update NestedObjectDelegatePropertyAccessor** + +```java +import org.apache.struts2.ognl.StrutsContext; +// Before: public class NestedObjectDelegatePropertyAccessor<T> implements PropertyAccessor { +public class NestedObjectDelegatePropertyAccessor<T> implements PropertyAccessor<StrutsContext> { + +// Before: private final PropertyAccessor propertyAccessor; +private final PropertyAccessor<StrutsContext> propertyAccessor; + +// Constructor: +public NestedObjectDelegatePropertyAccessor(NestedObjectExtractor<T> nestedObjectExtractor, PropertyAccessor<StrutsContext> propertyAccessor) + +// Change all 4 method params: OgnlContext → StrutsContext +``` + +- [ ] **Step 7: Compile tiles plugin** + +Run: `mvn compile -DskipAssembly -pl plugins/tiles 2>&1 | tail -30` + +Expected: Compiles. Fix any remaining type issues. + +- [ ] **Step 8: Commit** + +```bash +git add plugins/tiles/src/main/java/org/apache/tiles/ognl/ +git commit -m "WW-5326 refactor(tiles): parameterize tiles OGNL accessors with StrutsContext" +``` + +--- + +### Task 7: Update Test Files + +Tests that create `OgnlContext` via `Ognl.createDefaultContext(null)` need to create `StrutsContext` instead. + +**Files:** +- Modify: `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java` +- Modify: `plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java` +- Modify: Other test files as needed (compile will reveal them) + +- [ ] **Step 1: Update SecurityMemberAccessTest** + +In `core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java`: + +Change field type: +```java +// Before +private OgnlContext context; +// After +private StrutsContext context; +``` + +Change setUp: +```java +// Before +context = ognl.Ognl.createDefaultContext(null); +// After +context = new StrutsContext(sma); +``` + +Note: `sma` is initialized in `assignNewSma` which is called after this line. Reorder if needed — create `sma` first, then context. Check the setUp flow and adjust accordingly. The `assignNewSma` method creates a new `SecurityMemberAccess`, so context may need to be created after that call. + +Add import: +```java +import org.apache.struts2.ognl.StrutsContext; +``` + +Remove `import ognl.OgnlContext;` if no longer used. + +- [ ] **Step 2: Update SecurityMemberAccessProxyTest** + +In `plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java`: + +Same pattern: +```java +// Before +private OgnlContext context; +... +context = ognl.Ognl.createDefaultContext(null); +// After +private StrutsContext context; +... +context = new StrutsContext(sma); +``` + +- [ ] **Step 3: Fix remaining test compilation errors** + +Run: `mvn test-compile -DskipAssembly -pl core 2>&1 | grep "error:" | head -30` + +Fix each error. Most will be `OgnlContext` → `StrutsContext` type changes or `Ognl.createDefaultContext()` calls that need replacing. + +Run: `mvn test-compile -DskipAssembly -pl plugins/tiles 2>&1 | grep "error:" | head -30` + +Fix tiles test compilation errors similarly. + +Run: `mvn test-compile -DskipAssembly -pl plugins/spring 2>&1 | grep "error:" | head -30` + +Fix spring test compilation errors. + +- [ ] **Step 4: Commit** + +```bash +git add -u +git commit -m "WW-5326 test(ognl): update tests for StrutsContext migration" +``` + +--- + +### Task 8: Run Full Test Suite and Fix Failures + +- [ ] **Step 1: Run core tests** + +Run: `mvn test -DskipAssembly -pl core 2>&1 | tail -30` + +Expected: All tests pass. If failures occur, investigate and fix. Common issues: +- `ClassCastException` if OGNL internally creates raw `OgnlContext` instead of preserving `StrutsContext` +- `OgnlRuntime` accessor registration warnings with generic types +- Test setup creating wrong context type + +- [ ] **Step 2: Run tiles plugin tests** + +Run: `mvn test -DskipAssembly -pl plugins/tiles 2>&1 | tail -30` + +Expected: All tests pass. + +- [ ] **Step 3: Run spring plugin tests** + +Run: `mvn test -DskipAssembly -pl plugins/spring 2>&1 | tail -30` + +Expected: All tests pass. + +- [ ] **Step 4: Run full build** + +Run: `mvn test -DskipAssembly 2>&1 | tail -50` + +Expected: Full build succeeds with all tests passing. This catches any transitive compilation issues in other modules. + +- [ ] **Step 5: Commit any remaining fixes** + +```bash +git add -u +git commit -m "WW-5326 fix(ognl): resolve test failures from OGNL 3.5.x migration" +``` + +--- + +### Task 9: Cleanup and Final Verification + +- [ ] **Step 1: Search for any remaining raw OgnlContext usage** + +Run grep to find leftover `OgnlContext` references that should be `StrutsContext`: + +```bash +# In core main sources (excluding deprecated OgnlReflectionContextFactory) +grep -rn "OgnlContext" core/src/main/java/ --include="*.java" | grep -v "OgnlReflectionContextFactory" | grep -v "import" +``` + +Any results (other than Javadoc or string literals) indicate missed conversions. Fix them. + +- [ ] **Step 2: Search for remaining `Ognl.createDefaultContext` calls** + +```bash +grep -rn "Ognl.createDefaultContext" core/src/main/java/ --include="*.java" | grep -v "OgnlReflectionContextFactory" +``` + +Expected: No results (except the deprecated factory). All context creation should go through `new StrutsContext(...)` or `OgnlUtil.createDefaultContext()`. + +- [ ] **Step 3: Run full build one final time** + +Run: `mvn clean test -DskipAssembly 2>&1 | tail -30` + +Expected: Clean build, all tests pass. + +- [ ] **Step 4: Commit any final cleanup** + +```bash +git add -u +git commit -m "WW-5326 chore(ognl): cleanup remaining OgnlContext references" +``` + +--- + +## Notes for the Implementer + +### Known Risk: OgnlRuntime Generic Boundary + +`OgnlValueStackFactory` registers accessors via `OgnlRuntime.setPropertyAccessor(Class, PropertyAccessor)`. Since these are now `PropertyAccessor<StrutsContext>` but the `OgnlRuntime` method is generic with its own `<C>`, you may need `@SuppressWarnings("unchecked")` at the registration boundary. This is acceptable — it's a type-erasure boundary between Struts' typed world and OGNL's static global registry. + +### Known Risk: OGNL Internal Context Preservation + +If any test fails with `ClassCastException` (trying to cast `OgnlContext` to `StrutsContext`), it means OGNL internally creates a new `OgnlContext` during evaluation instead of preserving the passed-in `StrutsContext`. **This is a bug in OGNL 3.5.x** that should be reported upstream. Workaround: use `instanceof` checks at the boundary. + +### Deprecated `Ognl.setRoot()` → `context.withRoot()` + +OGNL 3.5.x deprecates `Ognl.setRoot(context, root)` in favor of `context.withRoot(root)`. The `OgnlUtil.withRoot()` helper method currently uses `Ognl.setRoot()` / `Ognl.getRoot()`. Update to use `context.withRoot()` / `context.getRoot()` directly. + +### The `DefaultTypeConverter` File + +`core/src/main/java/org/apache/struts2/conversion/impl/DefaultTypeConverter.java` imports `ognl.OgnlContext` but only uses it in a conditional cast. Check whether this cast needs updating to `StrutsContext` or if it can use the `Map` interface. diff --git a/docs/superpowers/specs/2026-04-06-ognl-3.5-upgrade-design.md b/docs/superpowers/specs/2026-04-06-ognl-3.5-upgrade-design.md new file mode 100644 index 000000000..9358d8e9b --- /dev/null +++ b/docs/superpowers/specs/2026-04-06-ognl-3.5-upgrade-design.md @@ -0,0 +1,263 @@ +# OGNL 3.5.x Upgrade — Design Spec + +> **Jira:** [WW-5326](https://issues.apache.org/jira/browse/WW-5326) + +## Goal + +Upgrade Apache Struts from OGNL 3.4.10 to OGNL 3.5.0-BETA4+ and introduce `StrutsContext extends OgnlContext<StrutsContext>` as the framework's own OGNL evaluation context. This lays the foundation for treating OGNL as an execution sandbox with typed, Struts-specific context state. + +## Motivation + +- **Forward-looking maintenance**: stay current with OGNL development, avoid a larger migration later +- **Real-world validation**: Struts is the primary consumer of OGNL — upgrading validates the 3.5.x generic API +- **Java 17 baseline**: OGNL 3.5.x requires Java 17, aligning with Struts 7.x +- **Type safety**: self-bounded generics (`OgnlContext<C>`) enable Struts to have a properly typed context instead of stringly-typed map entries +- **Sandbox foundation**: `StrutsContext` is the first step toward isolated OGNL evaluation contexts (`OgnlRuntime` instance-based isolation is future work) + +## Non-Goals + +- Instance-based `OgnlRuntime` / true sandbox isolation (future OGNL work) +- Consuming new OGNL features (null-safe operator `?.`, dual-mode evaluation) — those come as separate follow-ups +- Behavioral changes to security model, accessor logic, or expression evaluation + +## Current State + +### OGNL Usage in Struts + +- **Version**: 3.4.10 (defined in root `pom.xml` as `ognl.version`) +- **Core dependency**: `core/pom.xml` depends on `ognl:ognl` +- **Context creation**: 3 call sites use `Ognl.createDefaultContext()`: + - `OgnlUtil.createDefaultContext()` (line 738) + - `OgnlValueStack.setRoot()` (line 124) + - `OgnlReflectionContextFactory.createDefaultContext()` (line 33) +- **No custom OgnlContext subclass**: Struts uses `OgnlContext` directly +- **Context state via map entries**: flags like `DENY_METHOD_EXECUTION`, `CREATE_NULL_OBJECTS`, `VALUE_STACK`, conversion state — all stored as stringly-typed map entries in `OgnlContext` and accessed via `ReflectionContextState` static methods + +### OGNL Interface Implementations in Struts + +| Interface | Struts Implementation | +|---|---| +| `MemberAccess` | `SecurityMemberAccess` | +| `TypeConverter` | `OgnlTypeConverterWrapper` | +| `ClassResolver` | `RootAccessor` (interface), `CompoundRootAccessor` (impl) | +| `PropertyAccessor` | `RootAccessor`, `CompoundRootAccessor`, `ObjectProxyPropertyAccessor`, + 8 classes extending `ObjectPropertyAccessor`/`ListPropertyAccessor`/`MapPropertyAccessor`/etc. | +| `MethodAccessor` | `RootAccessor`, `CompoundRootAccessor`, `XWorkMethodAccessor` | +| `NullHandler` | `OgnlNullHandlerWrapper` | + +### Tiles Plugin OGNL Usage + +6-8 files in `plugins/tiles` use OGNL directly: +- `ScopePropertyAccessor`, `AnyScopePropertyAccessor`, `NestedObjectDelegatePropertyAccessor`, `DelegatePropertyAccessor` +- `OGNLAttributeEvaluator`, `PropertyAccessorDelegateFactory`, `TilesContextPropertyAccessorDelegateFactory` +- Associated test files + +## OGNL 3.5.x Key API Changes + +### Self-Bounded Generics + +All core interfaces and classes are now generic with `<C extends OgnlContext<C>>`: + +```java +public class OgnlContext<C extends OgnlContext<C>> implements Map<String, Object> +public interface MemberAccess<C extends OgnlContext<C>> +public interface ClassResolver<C extends OgnlContext<C>> +public interface TypeConverter<C extends OgnlContext<C>> +public interface PropertyAccessor<C extends OgnlContext<C>> +public interface MethodAccessor<C extends OgnlContext<C>> +public interface NullHandler<C extends OgnlContext<C>> +public class ObjectPropertyAccessor<C extends OgnlContext<C>> implements PropertyAccessor<C> +// ... all base accessor classes similarly parameterized +``` + +### OgnlContext Constructor Changes + +```java +// New (memberAccess first, required non-null) +public OgnlContext(MemberAccess<C> memberAccess, ClassResolver<C> classResolver, TypeConverter<C> typeConverter) + +// Deprecated (old parameter order) +@Deprecated(forRemoval = true) +OgnlContext(ClassResolver<C> classResolver, TypeConverter<C> typeConverter, MemberAccess<C> memberAccess) +``` + +### OgnlContext.Builder + +```java +public static class Builder<C extends OgnlContext<C>> { + public Builder(Function<Builder<C>, C> provider) + public Builder<C> withMemberAccess(MemberAccess<C> memberAccess) + public Builder<C> withClassResolver(ClassResolver<C> classResolver) + public Builder<C> withTypeConverter(TypeConverter<C> converter) + public Builder<C> withRoot(Object value) + public C build() +} +``` + +### Other Changes + +- `SecurityManager` support removed +- Null-safe navigation operator (`?.`) added +- `setRoot()` deprecated in favor of `withRoot()` (fluent) +- Java 17 baseline + +### Unchanged + +- `Ognl` class remains abstract with only static methods (no instance-based evaluation) +- `OgnlRuntime` remains a static utility (global accessor/cache registration) + +## Design + +### Approach: Direct StrutsContext Construction + +Struts creates `StrutsContext` directly, bypassing `Ognl.createDefaultContext()`. This gives Struts full ownership of context lifecycle and avoids the global-state issues of `Ognl.withBuilderProvider()`. + +### StrutsContext + +```java +package org.apache.struts2.ognl; + +public class StrutsContext extends OgnlContext<StrutsContext> { + + // Phase 1: just the constructor, delegate to super + public StrutsContext(SecurityMemberAccess memberAccess, + RootAccessor resolver, + OgnlTypeConverterWrapper converter) { + super(memberAccess, resolver, converter); + } + + // Phase 2 (incremental): promote map entries to typed fields + // private ValueStack valueStack; + // private boolean reportErrorsOnNoProperty; + // private boolean throwExceptionOnFailure; + // private boolean createNullObjects; + // private boolean denyMethodExecution; + // private boolean denyIndexedAccessExecution; + // private String conversionPropertyFullName; + // private String currentPropertyPath; + // private Class<?> lastBeanClassAccessed; + // private String lastBeanPropertyAccessed; +} +``` + +Phase 1 introduces the class with zero behavioral change — it's just an `OgnlContext` subclass. The typed fields are a follow-up. + +### Generic Type Ripple + +All OGNL interface implementations parameterize with `<StrutsContext>`: + +```java +// Core interfaces +public class SecurityMemberAccess implements MemberAccess<StrutsContext> +public class OgnlTypeConverterWrapper implements ognl.TypeConverter<StrutsContext> +public interface RootAccessor extends PropertyAccessor<StrutsContext>, MethodAccessor<StrutsContext>, ClassResolver<StrutsContext> +public class OgnlNullHandlerWrapper implements ognl.NullHandler<StrutsContext> + +// Accessors (extend generic base classes) +public class CompoundRootAccessor implements RootAccessor +public class ObjectProxyPropertyAccessor implements PropertyAccessor<StrutsContext> +public class ObjectAccessor extends ObjectPropertyAccessor<StrutsContext> +public class ParameterPropertyAccessor extends ObjectPropertyAccessor<StrutsContext> +public class HttpParametersPropertyAccessor extends ObjectPropertyAccessor<StrutsContext> +public class XWorkObjectPropertyAccessor extends ObjectPropertyAccessor<StrutsContext> +public class XWorkEnumerationAccessor extends EnumerationPropertyAccessor<StrutsContext> // verify base class +public class XWorkIteratorPropertyAccessor extends IteratorPropertyAccessor<StrutsContext> // verify base class +public class XWorkCollectionPropertyAccessor extends ObjectPropertyAccessor<StrutsContext> +public class XWorkListPropertyAccessor extends ListPropertyAccessor<StrutsContext> +public class XWorkMapPropertyAccessor extends MapPropertyAccessor<StrutsContext> +public class XWorkMethodAccessor extends ObjectMethodAccessor<StrutsContext> +``` + +Method signatures change `OgnlContext` parameters to `StrutsContext` throughout. + +### Context Creation + +Replace `Ognl.createDefaultContext()` with direct construction: + +```java +// OgnlUtil.createDefaultContext() +protected StrutsContext createDefaultContext(Object root, ClassResolver<StrutsContext> resolver) { + if (resolver == null) { + resolver = container.getInstance(RootAccessor.class); + } + StrutsContext ctx = new StrutsContext( + container.getInstance(SecurityMemberAccess.class), resolver, defaultConverter); + ctx.withRoot(root); + return ctx; +} + +// OgnlValueStack.setRoot() +StrutsContext ognlContext = new StrutsContext(securityMemberAccess, accessor, + new OgnlTypeConverterWrapper(xworkConverter)); +ognlContext.withRoot(this.root); + +// OgnlReflectionContextFactory — already @Deprecated(forRemoval=true) since 6.8.0 +// Keep using Ognl.createDefaultContext(root) with raw type, or remove entirely +``` + +### Tiles Plugin + +The tiles plugin accessors operate on tiles-specific objects, not on `StrutsContext` directly. Options: +- Parameterize with raw `OgnlContext` (use `PropertyAccessor<OgnlContext>`) if OGNL allows it +- Use wildcard `PropertyAccessor<?>` if supported +- Parameterize with `StrutsContext` if tiles always runs within a Struts context + +Decision: determine during implementation based on what compiles cleanly. + +### XWorkTypeConverterWrapper + +Currently casts `Map` context to `OgnlContext`. After upgrade, `ognl.TypeConverter<StrutsContext>` passes `StrutsContext` directly — the cast goes away. Struts' own `TypeConverter` interface (in `conversion` package) may also need its `convertValue` signature updated. + +### ReflectionContextState + +Initially unchanged — continues to work via `Map<String, Object>` interface that `StrutsContext` inherits from `OgnlContext`. Promoting to typed fields is a follow-up. + +## Implementation Phases + +### Phase 1: Version bump + StrutsContext + generics (this effort) + +1. Bump `ognl.version` to `3.5.0-BETA4` in root `pom.xml` +2. Create `StrutsContext extends OgnlContext<StrutsContext>` (constructor only) +3. Update all OGNL interface implementations with `<StrutsContext>` type parameter (~20 classes in core) +4. Update method signatures: `OgnlContext` → `StrutsContext` in all accessor/handler implementations +5. Replace `Ognl.createDefaultContext()` with direct `StrutsContext` construction (3 call sites) +6. Update tiles plugin accessor classes (~6-8 files) +7. Update test files (~50+ files referencing `OgnlContext`) +8. Verify all tests pass + +### Phase 2: Typed context fields (follow-up) + +- Promote `ReflectionContextState` map entries to `StrutsContext` typed fields +- Update accessors to use typed getters instead of `context.get("string.key")` +- Deprecate `ReflectionContextState` static methods + +### Phase 3: Sandbox features (future, requires OGNL changes) + +- Instance-based `OgnlRuntime` (OGNL-side work) +- Per-sandbox accessor registrations +- Isolated evaluation engines + +## Risk Areas + +### OgnlRuntime global statics + +`OgnlRuntime.setPropertyAccessor(Class<?>, PropertyAccessor<C>)` is generic but the registration is global. Registering `PropertyAccessor<StrutsContext>` may cause unchecked warnings or issues when OGNL internally retrieves accessors with a different context type. May need raw types at registration boundary. + +### OGNL internal context preservation + +If OGNL internally creates new `OgnlContext` instances during expression evaluation (rather than preserving the passed-in `StrutsContext`), typed fields would be lost. BETA1 addressed "context root preservation during nested evaluations" but this needs runtime verification. + +### Tiles plugin type compatibility + +Tiles accessors may not naturally fit `StrutsContext` parameterization. Need to determine the right generic type during implementation. + +### OGNL BETA stability + +OGNL 3.5.0 is still in BETA. API changes may occur in subsequent releases. This is acceptable given the user is an OGNL contributor and can influence the API. + +## Expected Outcomes + +- Struts compiles and all tests pass against OGNL 3.5.0-BETA4 +- `StrutsContext` exists as the framework's OGNL context class +- All OGNL interface implementations are properly parameterized with `<StrutsContext>` +- Foundation is in place for typed context fields and eventual sandbox isolation +- Any OGNL API issues discovered are reported/fixed upstream
