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 e14c4f7dac5d7c8883c3a16a8a4d5e5bd5a1725f
Author: Lukasz Lenart <[email protected]>
AuthorDate: Sun Jan 4 17:02:14 2026 +0100

    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]>
---
 .../org/apache/struts2/result/StreamResult.java    | 207 +++++++++++++--------
 1 file changed, 127 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 32b9eab33..cd762e3ed 100644
--- a/core/src/main/java/org/apache/struts2/result/StreamResult.java
+++ b/core/src/main/java/org/apache/struts2/result/StreamResult.java
@@ -26,6 +26,7 @@ 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;
@@ -79,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;
@@ -125,7 +131,7 @@ public class StreamResult extends StrutsResultSupport {
      * @return Returns the bufferSize.
      */
     public int getBufferSize() {
-        return (bufferSize);
+        return bufferSize;
     }
 
     /**
@@ -139,7 +145,7 @@ public class StreamResult extends StrutsResultSupport {
      * @return Returns the contentType.
      */
     public String getContentType() {
-        return (contentType);
+        return contentType;
     }
 
     /**
@@ -195,7 +201,7 @@ public class StreamResult extends StrutsResultSupport {
      * @return Returns the inputName.
      */
     public String getInputName() {
-        return (inputName);
+        return inputName;
     }
 
     /**
@@ -208,92 +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);
-            }
+        resolveInputStream(invocation);
+        HttpServletResponse response = 
invocation.getInvocationContext().getServletResponse();
+        
+        applyResponseHeaders(response, invocation);
+        applyContentLength(response, invocation);
 
-            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("Streaming result [{}] of type [{}], length [{}], 
content-disposition [{}] with charset [{}]",
+                inputName, contentType, contentLength, contentDisposition, 
contentCharSet);
 
-            String parsedContentType = conditionalParse(contentType, 
invocation);
-            String parsedContentCharSet = conditionalParse(contentCharSet, 
invocation);
+        try (InputStream in = inputStream; OutputStream out = 
response.getOutputStream()) {
+            streamContent(in, out);
+        }
+    }
 
-            if (StringUtils.isEmpty(parsedContentCharSet)) {
-                LOG.debug("Set content type to: {} and reset character 
encoding to null", contentType);
-                oResponse.setContentType(parsedContentType);
-                oResponse.setCharacterEncoding(null);
-            } else {
-                LOG.debug("Set the content type: {};charset={}", contentType, 
parsedContentCharSet);
-                oResponse.setContentType(parsedContentType);
-                oResponse.setCharacterEncoding(parsedContentCharSet);
-            }
+    /**
+     * 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(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();
     }
 
     /**

Reply via email to