You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2026/02/07 18:38:23 UTC
(struts) branch fix/WW-3647-spring-constructor-autowiring created (now f6f0a3fad)
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a change to branch fix/WW-3647-spring-constructor-autowiring
in repository https://gitbox.apache.org/repos/asf/struts.git
at f6f0a3fad fix(spring): WW-3647 change autowire alwaysRespect default to true
This branch includes the following new commits:
new f6f0a3fad fix(spring): WW-3647 change autowire alwaysRespect default to true
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(struts) 01/01: fix(spring): WW-3647 change autowire alwaysRespect default to true
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/WW-3647-spring-constructor-autowiring
in repository https://gitbox.apache.org/repos/asf/struts.git
commit f6f0a3fadfd6acf4b82949eda7ff69e7dba2b2e9
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Sat Feb 7 20:38:05 2026 +0200
fix(spring): WW-3647 change autowire alwaysRespect default to true
Change the default value of struts.objectFactory.spring.autoWire.alwaysRespect
from false to true to fix the Spring constructor autowiring issue.
When a Spring String bean exists (e.g., JNDI lookup with default-value),
Spring's AUTOWIRE_CONSTRUCTOR strategy incorrectly injects that value into
ALL String parameters of ServletActionRedirectResult constructors, causing
malformed redirect URLs.
Setting alwaysRespect to true by default ensures the configured autowire
strategy (AUTOWIRE_BY_NAME) is consistently used, preventing unintended
bean injection.
Users who rely on the legacy constructor autowiring behavior can restore
it by setting:
<constant name="struts.objectFactory.spring.autoWire.alwaysRespect" value="false" />
Fixes https://issues.apache.org/jira/browse/WW-3647
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <no...@anthropic.com>
---
.../java/org/apache/struts2/StrutsConstants.java | 10 +-
.../org/apache/struts2/default.properties | 4 +-
.../apache/struts2/spring/SpringObjectFactory.java | 24 ++-
...-02-07-WW-3647-spring-constructor-autowiring.md | 200 +++++++++++++++++++++
4 files changed, 221 insertions(+), 17 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index a66478a97..c7e1dd916 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -256,9 +256,15 @@ public final class StrutsConstants {
public static final String STRUTS_OBJECTFACTORY_SPRING_AUTOWIRE = "struts.objectFactory.spring.autoWire";
/**
- * Whether the autowire strategy chosen by STRUTS_OBJECTFACTORY_SPRING_AUTOWIRE is always respected. Defaults
- * to false, which is the legacy behavior that tries to determine the best strategy for the situation.
+ * Whether the autowire strategy chosen by STRUTS_OBJECTFACTORY_SPRING_AUTOWIRE is always respected.
+ * Defaults to true, which ensures the configured autowire strategy (AUTOWIRE_BY_NAME by default) is
+ * consistently used. This prevents issues where Spring's AUTOWIRE_CONSTRUCTOR strategy could inject
+ * unintended beans (e.g., String beans) into constructors.
+ * <p>
+ * Set to false to restore legacy behavior that mixes injection strategies, but be aware this can
+ * cause issues like WW-3647 where String beans are incorrectly injected into result class constructors.
*
+ * @see <a href="https://issues.apache.org/jira/browse/WW-3647">WW-3647</a>
* @since 2.1.3
*/
public static final String STRUTS_OBJECTFACTORY_SPRING_AUTOWIRE_ALWAYS_RESPECT = "struts.objectFactory.spring.autoWire.alwaysRespect";
diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties
index a980196a6..59165eca3 100644
--- a/core/src/main/resources/org/apache/struts2/default.properties
+++ b/core/src/main/resources/org/apache/struts2/default.properties
@@ -42,8 +42,8 @@ struts.objectFactory.spring.autoWire = name
struts.objectFactory.spring.useClassCache = true
### ensures the autowire strategy is always respected.
-### valid values are: true, false (false is the default)
-struts.objectFactory.spring.autoWire.alwaysRespect = false
+### valid values are: true, false (true is the default)
+struts.objectFactory.spring.autoWire.alwaysRespect=true
### By default SpringObjectFactory doesn't support AOP
### This flag was added just temporally to check if nothing is broken
diff --git a/plugins/spring/src/main/java/org/apache/struts2/spring/SpringObjectFactory.java b/plugins/spring/src/main/java/org/apache/struts2/spring/SpringObjectFactory.java
index 2cfd97b66..e6cd2195b 100644
--- a/plugins/spring/src/main/java/org/apache/struts2/spring/SpringObjectFactory.java
+++ b/plugins/spring/src/main/java/org/apache/struts2/spring/SpringObjectFactory.java
@@ -18,11 +18,11 @@
*/
package org.apache.struts2.spring;
-import org.apache.struts2.ObjectFactory;
-import org.apache.struts2.inject.Inject;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
+import org.apache.struts2.ObjectFactory;
+import org.apache.struts2.inject.Inject;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.UnsatisfiedDependencyException;
import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
@@ -55,9 +55,10 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
protected int autowireStrategy = AutowireCapableBeanFactory.AUTOWIRE_BY_NAME;
private final Map<String, Object> classes = new HashMap<>();
private boolean useClassCache = true;
- private boolean alwaysRespectAutowireStrategy = false;
+ private boolean alwaysRespectAutowireStrategy = true;
/**
* This is temporary solution, after validating can be removed
+ *
* @since 2.3.18
*/
@Deprecated
@@ -66,7 +67,7 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
public SpringObjectFactory() {
}
- @Inject(value="applicationContextPath",required=false)
+ @Inject(value = "applicationContextPath", required = false)
public void setApplicationContextPath(String ctx) {
if (ctx != null) {
setApplicationContext(new ClassPathXmlApplicationContext(ctx));
@@ -130,7 +131,6 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
* set the autoWiringFactory appropriately.
*
* @param context the application context
- *
* @return the bean factory
*/
protected AutowireCapableBeanFactory findAutoWiringBeanFactory(ApplicationContext context) {
@@ -153,8 +153,7 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
*
* @param beanName The name of the bean to look up in the application context
* @param extraContext additional context parameters
- * @return A bean from Spring or the result of calling the overridden
- * method.
+ * @return A bean from Spring or the result of calling the overridden method.
* @throws Exception in case of any errors
*/
@Override
@@ -174,7 +173,7 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
}
/**
- * @param clazz class of bean
+ * @param clazz class of bean
* @param extraContext additional context parameters
* @return bean
* @throws Exception in case of any errors
@@ -212,9 +211,8 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
}
/**
- * @param bean the bean to be autowired
+ * @param bean the bean to be autowired
* @param autoWiringFactory the autowiring factory
- *
* @return bean
*/
public Object autoWireBean(Object bean, AutowireCapableBeanFactory autoWiringFactory) {
@@ -235,7 +233,7 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
public Class getClassInstance(String className) throws ClassNotFoundException {
Class clazz = null;
if (useClassCache) {
- synchronized(classes) {
+ synchronized (classes) {
// this cache of classes is needed because Spring sucks at dealing with situations where the
// class instance changes
clazz = (Class) classes.get(className);
@@ -250,7 +248,7 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
}
if (useClassCache) {
- synchronized(classes) {
+ synchronized (classes) {
classes.put(className, clazz);
}
}
@@ -271,7 +269,7 @@ public class SpringObjectFactory extends ObjectFactory implements ApplicationCon
}
/**
- * Enable / disable caching of classes loaded by Spring.
+ * Enable / disable caching of classes loaded by Spring.
*
* @param useClassCache enable / disable class cache
*/
diff --git a/thoughts/shared/research/2026-02-07-WW-3647-spring-constructor-autowiring.md b/thoughts/shared/research/2026-02-07-WW-3647-spring-constructor-autowiring.md
new file mode 100644
index 000000000..75c884731
--- /dev/null
+++ b/thoughts/shared/research/2026-02-07-WW-3647-spring-constructor-autowiring.md
@@ -0,0 +1,200 @@
+---
+date: 2026-02-07T12:00:00+01:00
+topic: "WW-3647: ServletActionRedirectResult Spring Constructor Autowiring Issue"
+tags: [ research, codebase, spring-plugin, autowiring, constructor-injection ]
+status: complete
+---
+
+# Research: WW-3647 ServletActionRedirectResult Spring Constructor Autowiring Issue
+
+**Date**: 2026-02-07
+
+## Research Question
+
+Is WW-3647 (ServletActionRedirectResult injection problem with Spring JNDI lookup beans) still an issue, and what is the
+root cause?
+
+## Summary
+
+**YES, this issue is still present in the current codebase.** The root cause is the default "mixed injection strategy"
+in `SpringObjectFactory` that uses `AUTOWIRE_CONSTRUCTOR`, which causes Spring to inject any available String bean into
+all String parameters of result class constructors.
+
+## Detailed Findings
+
+### The Problem
+
+When a Spring JNDI lookup bean with a String default value exists:
+
+```xml
+
+<jee:jndi-lookup jndi-name="someName" id="currentEnvironment" default-value="XXXX"/>
+```
+
+`ServletActionRedirectResult` redirect URLs become malformed:
+
+```
+http://localhost:8080/XXXX/index!XXXX.action#XXXX
+```
+
+### Root Cause
+
+The issue is in `SpringObjectFactory.buildBean(Class, Map)` at line 199:
+
+```java
+// When alwaysRespectAutowireStrategy is false (DEFAULT)
+bean =autoWiringFactory.
+
+autowire(clazz, AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false);
+```
+
+Spring's `AUTOWIRE_CONSTRUCTOR` strategy:
+
+1. Examines all constructors of `ServletActionRedirectResult`
+2. Finds constructors with String parameters
+3. Looks for Spring beans of type `String` to inject
+4. Injects the JNDI bean's String value into **ALL** String parameters
+
+### ServletActionRedirectResult Constructors
+
+The class has 5 constructors, 4 of which take String parameters (
+`core/src/main/java/org/apache/struts2/result/ServletActionRedirectResult.java:135-156`):
+
+```java
+public ServletActionRedirectResult() // no-arg
+
+public ServletActionRedirectResult(String actionName) // 1 String
+
+public ServletActionRedirectResult(String actionName, String method) // 2 Strings
+
+public ServletActionRedirectResult(String namespace, String actionName, String method) // 3 Strings
+
+public ServletActionRedirectResult(String namespace, String actionName, String method, String anchor) // 4 Strings
+```
+
+When Spring's constructor autowiring finds a String bean, it matches that bean to **every** String parameter, resulting
+in:
+
+- `namespace = "XXXX"`
+- `actionName = "XXXX"`
+- `method = "XXXX"`
+- `anchor = "XXXX"`
+
+### Default Configuration
+
+In `core/src/main/resources/org/apache/struts2/default.properties`:
+
+```properties
+struts.objectFactory.spring.autoWire.alwaysRespect=false
+```
+
+This default value enables the legacy mixed injection strategy that causes the issue.
+
+### Code Flow
+
+1. Struts needs to create a `ServletActionRedirectResult` instance
+2. `SpringObjectFactory.buildBean()` is called
+3. Since `alwaysRespectAutowireStrategy = false`, it uses constructor autowiring:
+ ```java
+ bean = autoWiringFactory.autowire(clazz, AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false);
+ ```
+4. Spring finds a String bean (the JNDI lookup) and injects it into constructor String parameters
+5. The result is instantiated with incorrect values
+
+### Why the Workaround Works
+
+Setting `struts.objectFactory.spring.autoWire.alwaysRespect = true` causes the code to take a different path (
+`SpringObjectFactory.java:188-192`):
+
+```java
+if(alwaysRespectAutowireStrategy){
+// Leave the creation up to Spring
+bean =autoWiringFactory.
+
+createBean(clazz, autowireStrategy, false);
+
+injectApplicationContext(bean);
+ return
+
+injectInternalBeans(bean);
+}
+```
+
+The default `autowireStrategy` is `AUTOWIRE_BY_NAME`, which only injects beans when property names match bean names.
+Since `ServletActionRedirectResult` doesn't have properties named after JNDI beans, no incorrect injection occurs.
+
+## Code References
+
+- `plugins/spring/src/main/java/org/apache/struts2/spring/SpringObjectFactory.java:183-208` - buildBean method with
+ mixed injection strategy
+- `plugins/spring/src/main/java/org/apache/struts2/spring/SpringObjectFactory.java:58` - alwaysRespectAutowireStrategy
+ field (default: false)
+- `core/src/main/java/org/apache/struts2/result/ServletActionRedirectResult.java:135-156` - Constructors with String
+ parameters
+- `core/src/main/java/org/apache/struts2/StrutsConstants.java:138` - STRUTS_OBJECTFACTORY_SPRING_AUTOWIRE_ALWAYS_RESPECT
+ constant
+- `core/src/main/resources/org/apache/struts2/default.properties` - Default configuration
+
+## Test Coverage Gap
+
+There are **no tests** in the Spring plugin specifically testing `ServletActionRedirectResult` autowiring behavior.
+Existing tests cover:
+
+- Constructor injection with custom beans (`testShouldUseConstructorBasedInjectionWhenCreatingABeanFromAClassName`)
+- alwaysRespect configuration switching
+- Fallback behavior for ambiguous constructors
+
+But none test the scenario of a String bean being incorrectly injected into result class constructors.
+
+## Potential Fixes
+
+1. **Change the default**: Set `struts.objectFactory.spring.autoWire.alwaysRespect = true` as the default
+ - Pros: Fixes the issue for everyone
+ - Cons: Breaking change for existing applications relying on constructor injection
+
+2. **Remove String-parameter constructors**: Use only the no-arg constructor with setters
+ - Pros: Avoids the problem entirely
+ - Cons: Less convenient API, breaks existing code using these constructors
+
+3. **Add @Autowired(required=false) annotations**: Explicitly mark constructors
+ - Pros: Gives control over which constructors Spring considers
+ - Cons: Adds Spring-specific annotations to core classes
+
+4. **Use primary constructor designation**: Mark the no-arg constructor as preferred
+ - Pros: Spring would prefer the no-arg constructor
+ - Cons: Requires additional Spring configuration
+
+## Workaround (Still Valid)
+
+Configure Struts to respect the autowire strategy:
+
+**struts.xml:**
+
+```xml
+
+<constant name="struts.objectFactory.spring.autoWire.alwaysRespect" value="true"/>
+```
+
+**struts.properties:**
+
+```properties
+struts.objectFactory.spring.autoWire.alwaysRespect=true
+```
+
+## Architecture Insights
+
+The "mixed injection strategy" was designed to provide flexibility by:
+
+1. First using constructor autowiring to create beans
+2. Then applying property-based autowiring for additional dependencies
+
+However, this approach has an inherent flaw: constructor autowiring is too aggressive with simple types like String,
+matching any available String bean to any String constructor parameter. This design predates the modern understanding
+that constructor injection should primarily be used for required dependencies with specific types.
+
+## Open Questions
+
+1. Should the default for `alwaysRespectAutowireStrategy` be changed to `true`?
+2. Are there other result classes or Struts components with similar String-parameter constructors that might be
+ affected?
+3. Is the "mixed injection strategy" still needed, or should it be deprecated?