text/plain).+ * This method can be overridden by subclasses to provide custom stream sources + * (e.g., from database, cloud storage, or generated content). + *
+ * + * @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 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. + *+ * This method can be overridden by subclasses to add custom headers + * (e.g., ETag, X-Custom-Header) or modify caching behavior. + *
+ * + * @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. + *+ * This method can be overridden by subclasses for custom length calculation + * or to skip setting the header for chunked transfer encoding. + *
+ * + * @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. + *+ * This method can be overridden by subclasses to implement custom streaming behavior + * such as progress tracking, compression, or encryption. + *
+ * + * @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 b1e756cbce..be00026432 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 void testStreamResultWithCharSet2() throws Exception { 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 String getStreamForImageAsExpression() { 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 0000000000..fe817b9ff4 --- /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 +