This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch fix/WW-5602-streamresult-contentcharset-bug in repository https://gitbox.apache.org/repos/asf/struts.git
commit 50a71c4382fd9cd82b4727d47a36d65d18d7f8cc Author: Lukasz Lenart <[email protected]> AuthorDate: Sun Jan 4 14:08:00 2026 +0100 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]> --- .../org/apache/struts2/result/StreamResult.java | 28 +-- .../apache/struts2/result/StreamResultTest.java | 19 ++ ...1-02-WW-5602-streamresult-contentcharset-bug.md | 210 +++++++++++++++++++++ 3 files changed, 245 insertions(+), 12 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..32b9eab33 100644 --- a/core/src/main/java/org/apache/struts2/result/StreamResult.java +++ b/core/src/main/java/org/apache/struts2/result/StreamResult.java @@ -18,12 +18,13 @@ */ 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.InputStream; import java.io.OutputStream; @@ -32,9 +33,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> @@ -225,19 +224,24 @@ public class StreamResult extends StrutsResultSupport { 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."); + "Check the <param name=\"inputName\"> tag specified for this action is correct, not excluded and accepted."); LOG.error(msg); throw new IllegalArgumentException(msg); } - HttpServletResponse oResponse = invocation.getInvocationContext().getServletResponse(); - LOG.debug("Set the content type: {};charset{}", contentType, contentCharSet); - if (contentCharSet != null && !contentCharSet.isEmpty()) { - oResponse.setContentType(conditionalParse(contentType, invocation) + ";charset=" + conditionalParse(contentCharSet, invocation)); + String parsedContentType = conditionalParse(contentType, invocation); + String parsedContentCharSet = conditionalParse(contentCharSet, invocation); + + if (StringUtils.isEmpty(parsedContentCharSet)) { + LOG.debug("Set content type to: {} and reset character encoding to null", contentType); + oResponse.setContentType(parsedContentType); + oResponse.setCharacterEncoding(null); } else { - oResponse.setContentType(conditionalParse(contentType, invocation)); + LOG.debug("Set the content type: {};charset={}", contentType, parsedContentCharSet); + oResponse.setContentType(parsedContentType); + oResponse.setCharacterEncoding(parsedContentCharSet); } LOG.debug("Set the content length: {}", contentLength); @@ -269,7 +273,7 @@ public class StreamResult extends StrutsResultSupport { oOutput = oResponse.getOutputStream(); LOG.debug("Streaming result [{}] type=[{}] length=[{}] content-disposition=[{}] charset=[{}]", - inputName, contentType, contentLength, contentDisposition, contentCharSet); + inputName, contentType, contentLength, contentDisposition, contentCharSet); LOG.debug("Streaming to output buffer +++ START +++"); byte[] oBuff = new byte[bufferSize]; 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?
