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 dfc659d9b WW-5602 Fix StreamResult contentCharSet handling and
refactor for extensibility (#1510)
dfc659d9b is described below
commit dfc659d9bee51f6da5e8e70bd9e1b8708631403d
Author: Lukasz Lenart <[email protected]>
AuthorDate: Wed Jan 14 19:18:07 2026 +0100
WW-5602 Fix StreamResult contentCharSet handling and refactor for
extensibility (#1510)
* fix(core): WW-5602 fix StreamResult contentCharSet handling
- Evaluate contentCharSet expression before checking for emptiness
- Use StringUtils.isEmpty() for null/empty check on parsed value
- Call setCharacterEncoding(null) to clear Dispatcher's default encoding
- Set charset via setCharacterEncoding() instead of appending to
content-type
- Add test for expression evaluating to null
Closes WW-5602
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* refactor(core): extract methods and modernize StreamResult
- Add constants: DEFAULT_BUFFER_SIZE, DEFAULT_CONTENT_TYPE,
DEFAULT_CONTENT_DISPOSITION, DEFAULT_INPUT_NAME
- Extract resolveInputStream() for custom stream sources
- Extract applyResponseHeaders() for custom header handling
- Extract applyContentLength() for custom length calculation
- Extract streamContent() for custom streaming behavior
- Use try-with-resources for cleaner resource management
- Add JavaDoc explaining extensibility of each method
All extracted methods are protected to enable easy extension
by users creating custom streaming result types.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
* fix(core): resolve setCharacterEncoding ambiguity for Jakarta EE 11
Cast null to String to disambiguate between overloaded methods:
- setCharacterEncoding(String)
- setCharacterEncoding(Charset)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
---------
Co-authored-by: Claude <[email protected]>
---
.../org/apache/struts2/result/StreamResult.java | 211 +++++++++++++--------
.../apache/struts2/result/StreamResultTest.java | 19 ++
...1-02-WW-5602-streamresult-contentcharset-bug.md | 210 ++++++++++++++++++++
3 files changed, 360 insertions(+), 80 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/result/StreamResult.java
b/core/src/main/java/org/apache/struts2/result/StreamResult.java
index 2a6611ec7..85e81177e 100644
--- a/core/src/main/java/org/apache/struts2/result/StreamResult.java
+++ b/core/src/main/java/org/apache/struts2/result/StreamResult.java
@@ -18,13 +18,15 @@
*/
package org.apache.struts2.result;
-import org.apache.struts2.ActionInvocation;
-import org.apache.struts2.inject.Inject;
-import org.apache.struts2.security.NotExcludedAcceptedPatternsChecker;
import jakarta.servlet.http.HttpServletResponse;
+import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
+import org.apache.struts2.ActionInvocation;
+import org.apache.struts2.inject.Inject;
+import org.apache.struts2.security.NotExcludedAcceptedPatternsChecker;
+import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.Serial;
@@ -32,9 +34,7 @@ import java.io.Serial;
/**
* A custom Result type for sending raw data (via an InputStream) directly to
the
* HttpServletResponse. Very useful for allowing users to download content.
- *
* <b>This result type takes the following parameters:</b>
- *
* <ul>
* <li><b>contentType</b> - the stream mime-type as sent to the web browser
* (default = <code>text/plain</code>).</li>
@@ -80,13 +80,18 @@ public class StreamResult extends StrutsResultSupport {
public static final String DEFAULT_PARAM = "inputName";
- protected String contentType = "text/plain";
+ public static final int DEFAULT_BUFFER_SIZE = 1024;
+ public static final String DEFAULT_CONTENT_TYPE = "text/plain";
+ public static final String DEFAULT_CONTENT_DISPOSITION = "inline";
+ public static final String DEFAULT_INPUT_NAME = "inputStream";
+
+ protected String contentType = DEFAULT_CONTENT_TYPE;
protected String contentLength;
- protected String contentDisposition = "inline";
+ protected String contentDisposition = DEFAULT_CONTENT_DISPOSITION;
protected String contentCharSet;
- protected String inputName = "inputStream";
+ protected String inputName = DEFAULT_INPUT_NAME;
protected InputStream inputStream;
- protected int bufferSize = 1024;
+ protected int bufferSize = DEFAULT_BUFFER_SIZE;
protected boolean allowCaching = true;
private NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns;
@@ -126,7 +131,7 @@ public class StreamResult extends StrutsResultSupport {
* @return Returns the bufferSize.
*/
public int getBufferSize() {
- return (bufferSize);
+ return bufferSize;
}
/**
@@ -140,7 +145,7 @@ public class StreamResult extends StrutsResultSupport {
* @return Returns the contentType.
*/
public String getContentType() {
- return (contentType);
+ return contentType;
}
/**
@@ -196,7 +201,7 @@ public class StreamResult extends StrutsResultSupport {
* @return Returns the inputName.
*/
public String getInputName() {
- return (inputName);
+ return inputName;
}
/**
@@ -209,87 +214,133 @@ public class StreamResult extends StrutsResultSupport {
/**
* @see StrutsResultSupport#doExecute(java.lang.String, ActionInvocation)
*/
+ @Override
protected void doExecute(String finalLocation, ActionInvocation
invocation) throws Exception {
- LOG.debug("Find the Response in context");
-
- OutputStream oOutput = null;
-
- try {
- String parsedInputName = conditionalParse(inputName, invocation);
- boolean evaluated = parsedInputName != null &&
!parsedInputName.equals(inputName);
- boolean reevaluate = !evaluated ||
isAcceptableExpression(parsedInputName);
- if (inputStream == null && reevaluate) {
- LOG.debug("Find the inputstream from the invocation variable
stack");
- inputStream = (InputStream)
invocation.getStack().findValue(parsedInputName);
- }
-
- if (inputStream == null) {
- String msg = ("Can not find a java.io.InputStream with the
name [" + parsedInputName + "] in the invocation stack. " +
- "Check the <param name=\"inputName\"> tag specified for
this action is correct, not excluded and accepted.");
- LOG.error(msg);
- throw new IllegalArgumentException(msg);
- }
+ resolveInputStream(invocation);
+ HttpServletResponse response =
invocation.getInvocationContext().getServletResponse();
+
+ applyResponseHeaders(response, invocation);
+ applyContentLength(response, invocation);
+ LOG.debug("Streaming result [{}] of type [{}], length [{}],
content-disposition [{}] with charset [{}]",
+ inputName, contentType, contentLength, contentDisposition,
contentCharSet);
- HttpServletResponse oResponse =
invocation.getInvocationContext().getServletResponse();
+ try (InputStream in = inputStream; OutputStream out =
response.getOutputStream()) {
+ streamContent(in, out);
+ }
+ }
- LOG.debug("Set the content type: {};charset{}", contentType,
contentCharSet);
- if (contentCharSet != null && !contentCharSet.isEmpty()) {
- oResponse.setContentType(conditionalParse(contentType,
invocation) + ";charset=" + conditionalParse(contentCharSet, invocation));
- } else {
- oResponse.setContentType(conditionalParse(contentType,
invocation));
- }
+ /**
+ * Resolves the input stream from the action invocation.
+ * <p>
+ * This method can be overridden by subclasses to provide custom stream
sources
+ * (e.g., from database, cloud storage, or generated content).
+ * </p>
+ *
+ * @param invocation the action invocation
+ * @throws IllegalArgumentException if the input stream cannot be found
+ */
+ protected void resolveInputStream(ActionInvocation invocation) {
+ String parsedInputName = conditionalParse(inputName, invocation);
+ boolean evaluated = parsedInputName != null &&
!parsedInputName.equals(inputName);
+ boolean reevaluate = !evaluated ||
isAcceptableExpression(parsedInputName);
+
+ if (inputStream == null && reevaluate) {
+ LOG.debug("Find the inputstream from the invocation variable
stack");
+ inputStream = (InputStream)
invocation.getStack().findValue(parsedInputName);
+ }
- LOG.debug("Set the content length: {}", contentLength);
- if (contentLength != null) {
- String translatedContentLength =
conditionalParse(contentLength, invocation);
- int contentLengthAsInt;
- try {
- contentLengthAsInt =
Integer.parseInt(translatedContentLength);
- if (contentLengthAsInt >= 0) {
- oResponse.setContentLength(contentLengthAsInt);
- }
- } catch (NumberFormatException e) {
- LOG.warn("failed to recognize {} as a number,
contentLength header will not be set",
- translatedContentLength, e);
- }
- }
+ if (inputStream == null) {
+ String msg = ("Can not find a java.io.InputStream with the name ["
+ parsedInputName + "] in the invocation stack. " +
+ "Check the <param name=\"inputName\"> tag specified for
this action is correct, not excluded and accepted.");
+ LOG.error(msg);
+ throw new IllegalArgumentException(msg);
+ }
+ }
- LOG.debug("Set the content-disposition: {}", contentDisposition);
- if (contentDisposition != null) {
- oResponse.addHeader("Content-Disposition",
conditionalParse(contentDisposition, invocation));
- }
+ /**
+ * Applies all response headers including content-type, charset,
content-length,
+ * content-disposition, and cache control headers.
+ * <p>
+ * This method can be overridden by subclasses to add custom headers
+ * (e.g., ETag, X-Custom-Header) or modify caching behavior.
+ * </p>
+ *
+ * @param response the HTTP response
+ * @param invocation the action invocation
+ */
+ protected void applyResponseHeaders(HttpServletResponse response,
ActionInvocation invocation) {
+ String parsedContentType = conditionalParse(contentType, invocation);
+ String parsedContentCharSet = conditionalParse(contentCharSet,
invocation);
+
+ response.setContentType(parsedContentType);
+ if (StringUtils.isEmpty(parsedContentCharSet)) {
+ LOG.debug("Set content type to: {} and reset character encoding to
null", parsedContentType);
+ response.setCharacterEncoding((String) null);
+ } else {
+ LOG.debug("Set content type: {};charset={}", parsedContentType,
parsedContentCharSet);
+ response.setCharacterEncoding(parsedContentCharSet);
+ }
- LOG.debug("Set the cache control headers if necessary: {}",
allowCaching);
- if (!allowCaching) {
- oResponse.addHeader("Pragma", "no-cache");
- oResponse.addHeader("Cache-Control", "no-cache");
- }
+ LOG.debug("Set the content-disposition: {}", contentDisposition);
+ if (contentDisposition != null) {
+ response.addHeader("Content-Disposition",
conditionalParse(contentDisposition, invocation));
+ }
- oOutput = oResponse.getOutputStream();
+ LOG.debug("Set the cache control headers if necessary: {}",
allowCaching);
+ if (!allowCaching) {
+ response.addHeader("Pragma", "no-cache");
+ response.addHeader("Cache-Control", "no-cache");
+ }
+ }
- LOG.debug("Streaming result [{}] type=[{}] length=[{}]
content-disposition=[{}] charset=[{}]",
- inputName, contentType, contentLength, contentDisposition,
contentCharSet);
+ /**
+ * Applies the content-length header to the response.
+ * <p>
+ * This method can be overridden by subclasses for custom length
calculation
+ * or to skip setting the header for chunked transfer encoding.
+ * </p>
+ *
+ * @param response the HTTP response
+ * @param invocation the action invocation
+ */
+ protected void applyContentLength(HttpServletResponse response,
ActionInvocation invocation) {
+ if (contentLength == null) {
+ return;
+ }
- LOG.debug("Streaming to output buffer +++ START +++");
- byte[] oBuff = new byte[bufferSize];
- int iSize;
- while (-1 != (iSize = inputStream.read(oBuff))) {
- LOG.debug("Sending stream ... {}", iSize);
- oOutput.write(oBuff, 0, iSize);
+ LOG.debug("Set the content length: {}", contentLength);
+ String translatedContentLength = conditionalParse(contentLength,
invocation);
+ try {
+ int length = Integer.parseInt(translatedContentLength);
+ if (length >= 0) {
+ response.setContentLength(length);
}
- LOG.debug("Streaming to output buffer +++ END +++");
+ } catch (NumberFormatException e) {
+ LOG.warn("Failed to parse contentLength [{}], header will not be
set", translatedContentLength, e);
+ }
+ }
- // Flush
- oOutput.flush();
- } finally {
- if (inputStream != null) {
- inputStream.close();
- }
- if (oOutput != null) {
- oOutput.close();
- }
+ /**
+ * Streams content from the input stream to the output stream.
+ * <p>
+ * This method can be overridden by subclasses to implement custom
streaming behavior
+ * such as progress tracking, compression, or encryption.
+ * </p>
+ *
+ * @param input the input stream to read from
+ * @param output the output stream to write to
+ * @throws IOException if an I/O error occurs
+ */
+ protected void streamContent(InputStream input, OutputStream output)
throws IOException {
+ LOG.debug("Streaming to output buffer +++ START +++");
+ byte[] buffer = new byte[bufferSize];
+ int bytesRead;
+ while ((bytesRead = input.read(buffer)) != -1) {
+ output.write(buffer, 0, bytesRead);
}
+ LOG.debug("Streaming to output buffer +++ END +++");
+ output.flush();
}
/**
diff --git a/core/src/test/java/org/apache/struts2/result/StreamResultTest.java
b/core/src/test/java/org/apache/struts2/result/StreamResultTest.java
index b1e756cbc..be0002643 100644
--- a/core/src/test/java/org/apache/struts2/result/StreamResultTest.java
+++ b/core/src/test/java/org/apache/struts2/result/StreamResultTest.java
@@ -120,6 +120,21 @@ public class StreamResultTest extends
StrutsInternalTestCase {
assertEquals("inline", response.getHeader("Content-disposition"));
}
+ public void testStreamResultWithNullCharSetExpression() throws Exception {
+ result.setParse(true);
+ result.setInputName("streamForImage");
+ result.setContentCharSet("${nullCharSetMethod}");
+
+ result.doExecute("helloworld", mai);
+
+ assertEquals(contentLength, response.getContentLength());
+ assertEquals("text/plain", result.getContentType());
+ // When expression evaluates to null, content-type should NOT include
charset
+ assertEquals("text/plain", response.getContentType());
+ assertEquals("streamForImage", result.getInputName());
+ assertEquals("inline", response.getHeader("Content-disposition"));
+ }
+
public void testAllowCacheDefault() throws Exception {
result.setInputName("streamForImage");
@@ -310,6 +325,10 @@ public class StreamResultTest extends
StrutsInternalTestCase {
public String getContentCharSetMethod() {
return "UTF-8";
}
+
+ public String getNullCharSetMethod() {
+ return null;
+ }
}
}
diff --git
a/thoughts/shared/research/2026-01-02-WW-5602-streamresult-contentcharset-bug.md
b/thoughts/shared/research/2026-01-02-WW-5602-streamresult-contentcharset-bug.md
new file mode 100644
index 000000000..fe817b9ff
--- /dev/null
+++
b/thoughts/shared/research/2026-01-02-WW-5602-streamresult-contentcharset-bug.md
@@ -0,0 +1,210 @@
+---
+date: 2026-01-02T15:55:38+01:00
+last_updated: 2026-01-02T16:15:00+01:00
+topic: "WW-5602: StreamResult contentCharSet Handling Issues"
+tags: [research, codebase, streamresult, charset, bug, WW-5602]
+status: complete
+jira_ticket: WW-5602
+---
+
+# Research: WW-5602 - StreamResult contentCharSet Handling Issues
+
+**Date**: 2026-01-02 15:55:38 +0100
+**JIRA**: [WW-5602](https://issues.apache.org/jira/browse/WW-5602)
+
+## Research Question
+
+Two issues related to StreamResult's contentCharSet handling:
+
+1. Why does UTF-8 appear in content-type when contentCharSet is not specified?
+2. When contentCharSet is an expression that evaluates to null, shouldn't the
charset be omitted instead of appending
+ `;charset=`?
+
+## Summary
+
+**Question 1 (UTF-8 appearing automatically):**
+The UTF-8 encoding is set by `Dispatcher.prepare()` which calls
`response.setCharacterEncoding(defaultEncoding)` before
+the action executes. The `defaultEncoding` comes from `struts.i18n.encoding`
property (defaults to UTF-8). When
+StreamResult later sets the content type, the servlet container may include
this pre-set charset.
+
+**Question 2 (Expression evaluating to null):**
+This IS a bug. The code checks the raw `contentCharSet` string before parsing,
not the evaluated result. When an
+expression like `${myMethod}` evaluates to null, `conditionalParse()` returns
an empty string, resulting in `;charset=`
+being appended with no value.
+
+## Detailed Findings
+
+### Source of Default UTF-8
+
+The UTF-8 encoding comes from `Dispatcher.prepare()`:
+
+```java
+// Dispatcher.java:937-952
+public void prepare(HttpServletRequest request, HttpServletResponse response) {
+ String encoding = null;
+ if (defaultEncoding != null) {
+ encoding = defaultEncoding; // From struts.i18n.encoding, defaults to
UTF-8
+ }
+ // ...
+ if (encoding != null) {
+ applyEncoding(request, encoding);
+ applyEncoding(response, encoding); // <-- Sets UTF-8 on response
+ }
+}
+```
+
+The `applyEncoding()` method calls `response.setCharacterEncoding(encoding)`:
+
+```java
+// Dispatcher.java:976-984
+private void applyEncoding(HttpServletResponse response, String encoding) {
+ try {
+ if (!encoding.equals(response.getCharacterEncoding())) {
+ response.setCharacterEncoding(encoding);
+ }
+ } catch (Exception e) {
+ // ...
+ }
+}
+```
+
+The default value is set in `default.properties`:
+
+```properties
+struts.i18n.encoding=UTF-8
+```
+
+### The contentCharSet Bug
+
+The bug is in `StreamResult.doExecute()`:
+
+```java
+// StreamResult.java:237-241
+if (contentCharSet != null && !contentCharSet.isEmpty()) {
+ oResponse.setContentType(conditionalParse(contentType, invocation) +
";charset=" + conditionalParse(contentCharSet, invocation));
+} else {
+ oResponse.setContentType(conditionalParse(contentType, invocation));
+}
+```
+
+The problem: The check evaluates the **raw string** (`${myMethod}`) rather
than the **parsed result**.
+
+When `contentCharSet = "${myMethod}"` and `myMethod` returns `null`:
+
+1. Check passes: `"${myMethod}"` is not null or empty
+2. `conditionalParse()` evaluates the expression
+3. `OgnlTextParser.evaluate()` handles null by returning empty string (lines
85-88)
+4. Result: `;charset=` appended with empty value
+
+### OgnlTextParser Null Handling
+
+```java
+// OgnlTextParser.java:85-89
+} else {
+ // the variable doesn't exist, so don't display anything
+ expression = left.concat(right);
+ result = expression;
+}
+```
+
+When an expression `${foo}` evaluates to null, it returns the concatenation of
left+right (empty strings in this case).
+
+## Code References
+
+- `core/src/main/java/org/apache/struts2/result/StreamResult.java:237-241` -
The buggy charset check
+- `core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java:937-984` -
Source of default UTF-8
+- `core/src/main/java/org/apache/struts2/util/OgnlTextParser.java:85-89` -
Null expression handling
+- `core/src/main/resources/org/apache/struts2/default.properties:27` - Default
UTF-8 setting
+-
`core/src/main/java/org/apache/struts2/result/StrutsResultSupport.java:216-225`
- conditionalParse implementation
+
+## Proposed Fix
+
+The fix should address both issues:
+
+1. Evaluate the expression BEFORE checking for emptiness
+2. Call `response.setCharacterEncoding(null)` to clear Dispatcher's default
UTF-8
+
+```java
+// Proposed fix for StreamResult.java
+String parsedContentCharSet = contentCharSet != null ?
conditionalParse(contentCharSet, invocation) : null;
+if (parsedContentCharSet != null && !parsedContentCharSet.isEmpty()) {
+ oResponse.setContentType(conditionalParse(contentType, invocation) +
";charset=" + parsedContentCharSet);
+} else {
+ oResponse.setContentType(conditionalParse(contentType, invocation));
+ oResponse.setCharacterEncoding(null); // Clear Dispatcher's default
encoding
+}
+```
+
+This ensures:
+- If the expression evaluates to null or empty, the charset is omitted entirely
+- The `setCharacterEncoding(null)` call resets the response encoding that was
set by `Dispatcher.prepare()`
+
+## How Other Result Types Handle Encoding
+
+Research into other Struts Result types revealed two patterns:
+
+### Pattern 1: Content-Type Header Only (PlainTextResult, StreamResult)
+- Add charset via Content-Type header: `text/plain; charset=UTF-8`
+- Does NOT override response encoding set by Dispatcher
+- Example: `response.setContentType("text/plain; charset=" + charSet)`
+
+### Pattern 2: Direct Response Encoding (XSLTResult, JSONValidationInterceptor)
+- Calls `response.setCharacterEncoding(encoding)` directly
+- Explicitly overrides what Dispatcher set
+- More forceful approach
+
+StreamResult currently uses Pattern 1, but the proposed fix adds Pattern 2's
approach to clear the encoding when no charset is specified.
+
+## Servlet API: setCharacterEncoding(null)
+
+According to Servlet API specification:
+- Calling `response.setCharacterEncoding(null)` resets encoding to the servlet
container default
+- This is a valid way to "clear" the Dispatcher's default encoding
+- Not commonly used in Struts codebase, but follows the API specification
+
+## Potential Breaking Changes
+
+This fix could affect applications that:
+- Rely on Dispatcher's default UTF-8 encoding for StreamResult responses
+- Serve text-based streams without explicitly setting contentCharSet
+
+**Mitigation**: Users who want UTF-8 can explicitly set
`contentCharSet="UTF-8"` in their result configuration:
+```xml
+<result name="success" type="stream">
+ <param name="contentType">text/plain</param>
+ <param name="contentCharSet">UTF-8</param>
+ <param name="inputName">myStream</param>
+</result>
+```
+
+## Alternative Approaches Considered
+
+### Option A: Minimal Fix (Bug Only)
+Only fix the expression evaluation bug, leave UTF-8 behavior unchanged.
+- Pros: Minimal change
+- Cons: Doesn't address the unexpected UTF-8 appearing
+
+### Option B: Auto-reset Encoding (Recommended)
+Fix bug + call `setCharacterEncoding(null)` when no charset specified.
+- Pros: Addresses both issues, clean content-type for binary streams
+- Cons: Could break apps relying on default UTF-8
+
+### Option C: Add Parameter
+Add a new `resetEncoding` parameter for explicit control.
+- Pros: Fully backward compatible, flexible
+- Cons: More complex, adds configuration option
+
+## Test Coverage
+
+Existing tests in `StreamResultTest.java`:
+
+- `testStreamResultDefault()` - Tests default behavior (no charset)
+- `testStreamResultWithCharSet()` - Tests explicit charset "ISO-8859-1"
+- `testStreamResultWithCharSet2()` - Tests expression-based charset returning
UTF-8
+
+**Missing test case:** Expression that evaluates to null (the bug scenario)
+
+## Open Questions
+
+1. Should there be a way to explicitly prevent charset from being added even
when `struts.i18n.encoding` is set?
+2. Should the framework provide a more explicit "no charset" option for binary
streams?
