mirror of
https://github.com/apache/struts.git
synced 2026-08-05 14:47:09 +00:00
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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Lukasz Lenart
@@ -18,13 +18,15 @@
|
||||
*/
|
||||
package org.apache.struts2.result;
|
||||
|
||||
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 jakarta.servlet.http.HttpServletResponse;
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
|
||||
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,89 +214,135 @@ 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");
|
||||
resolveInputStream(invocation);
|
||||
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
|
||||
|
||||
applyResponseHeaders(response, invocation);
|
||||
applyContentLength(response, invocation);
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
|
||||
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));
|
||||
} else {
|
||||
oResponse.setContentType(conditionalParse(contentType, invocation));
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
LOG.debug("Set the content-disposition: {}", contentDisposition);
|
||||
if (contentDisposition != null) {
|
||||
oResponse.addHeader("Content-Disposition", conditionalParse(contentDisposition, invocation));
|
||||
}
|
||||
|
||||
LOG.debug("Set the cache control headers if necessary: {}", allowCaching);
|
||||
if (!allowCaching) {
|
||||
oResponse.addHeader("Pragma", "no-cache");
|
||||
oResponse.addHeader("Cache-Control", "no-cache");
|
||||
}
|
||||
|
||||
oOutput = oResponse.getOutputStream();
|
||||
|
||||
LOG.debug("Streaming result [{}] type=[{}] length=[{}] content-disposition=[{}] charset=[{}]",
|
||||
LOG.debug("Streaming result [{}] of type [{}], length [{}], content-disposition [{}] with charset [{}]",
|
||||
inputName, contentType, contentLength, contentDisposition, contentCharSet);
|
||||
|
||||
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("Streaming to output buffer +++ END +++");
|
||||
|
||||
// Flush
|
||||
oOutput.flush();
|
||||
} finally {
|
||||
if (inputStream != null) {
|
||||
inputStream.close();
|
||||
}
|
||||
if (oOutput != null) {
|
||||
oOutput.close();
|
||||
}
|
||||
try (InputStream in = inputStream; OutputStream out = response.getOutputStream()) {
|
||||
streamContent(in, out);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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);
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 content-disposition: {}", contentDisposition);
|
||||
if (contentDisposition != null) {
|
||||
response.addHeader("Content-Disposition", conditionalParse(contentDisposition, invocation));
|
||||
}
|
||||
|
||||
LOG.debug("Set the cache control headers if necessary: {}", allowCaching);
|
||||
if (!allowCaching) {
|
||||
response.addHeader("Pragma", "no-cache");
|
||||
response.addHeader("Cache-Control", "no-cache");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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("Set the content length: {}", contentLength);
|
||||
String translatedContentLength = conditionalParse(contentLength, invocation);
|
||||
try {
|
||||
int length = Integer.parseInt(translatedContentLength);
|
||||
if (length >= 0) {
|
||||
response.setContentLength(length);
|
||||
}
|
||||
} catch (NumberFormatException e) {
|
||||
LOG.warn("Failed to parse contentLength [{}], header will not be set", translatedContentLength, e);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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();
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if expression doesn't contain vulnerable code
|
||||
*
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -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?
|
||||
Reference in New Issue
Block a user