Skip to content

Commit dfc659d

Browse files
lukaszlenartclaude
lukaszlenart
and
claude
authored
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>
1 parent d59aea5 commit dfc659d

File tree

3 files changed

+360
-80
lines changed

3 files changed

+360
-80
lines changed

‎core/src/main/java/org/apache/struts2/result/StreamResult.java‎

Lines changed: 131 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,23 @@
1818
*/
1919
package org.apache.struts2.result;
2020

21-
import org.apache.struts2.ActionInvocation;
22-
import org.apache.struts2.inject.Inject;
23-
import org.apache.struts2.security.NotExcludedAcceptedPatternsChecker;
2421
import jakarta.servlet.http.HttpServletResponse;
22+
import org.apache.commons.lang3.StringUtils;
2523
import org.apache.logging.log4j.LogManager;
2624
import org.apache.logging.log4j.Logger;
25+
import org.apache.struts2.ActionInvocation;
26+
import org.apache.struts2.inject.Inject;
27+
import org.apache.struts2.security.NotExcludedAcceptedPatternsChecker;
2728

29+
import java.io.IOException;
2830
import java.io.InputStream;
2931
import java.io.OutputStream;
3032
import java.io.Serial;
3133

3234
/**
3335
* A custom Result type for sending raw data (via an InputStream) directly to the
3436
* HttpServletResponse. Very useful for allowing users to download content.
35-
*
3637
* <b>This result type takes the following parameters:</b>
37-
*
3838
* <ul>
3939
* <li><b>contentType</b> - the stream mime-type as sent to the web browser
4040
* (default = <code>text/plain</code>).</li>
@@ -80,13 +80,18 @@ public class StreamResult extends StrutsResultSupport {
8080

8181
public static final String DEFAULT_PARAM = "inputName";
8282

83-
protected String contentType = "text/plain";
83+
public static final int DEFAULT_BUFFER_SIZE = 1024;
84+
public static final String DEFAULT_CONTENT_TYPE = "text/plain";
85+
public static final String DEFAULT_CONTENT_DISPOSITION = "inline";
86+
public static final String DEFAULT_INPUT_NAME = "inputStream";
87+
88+
protected String contentType = DEFAULT_CONTENT_TYPE;
8489
protected String contentLength;
85-
protected String contentDisposition = "inline";
90+
protected String contentDisposition = DEFAULT_CONTENT_DISPOSITION;
8691
protected String contentCharSet;
87-
protected String inputName = "inputStream";
92+
protected String inputName = DEFAULT_INPUT_NAME;
8893
protected InputStream inputStream;
89-
protected int bufferSize = 1024;
94+
protected int bufferSize = DEFAULT_BUFFER_SIZE;
9095
protected boolean allowCaching = true;
9196

9297
private NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns;
@@ -126,7 +131,7 @@ public void setAllowCaching(boolean allowCaching) {
126131
* @return Returns the bufferSize.
127132
*/
128133
public int getBufferSize() {
129-
return (bufferSize);
134+
return bufferSize;
130135
}
131136

132137
/**
@@ -140,7 +145,7 @@ public void setBufferSize(int bufferSize) {
140145
* @return Returns the contentType.
141146
*/
142147
public String getContentType() {
143-
return (contentType);
148+
return contentType;
144149
}
145150

146151
/**
@@ -196,7 +201,7 @@ public void setContentCharSet(String contentCharSet) {
196201
* @return Returns the inputName.
197202
*/
198203
public String getInputName() {
199-
return (inputName);
204+
return inputName;
200205
}
201206

202207
/**
@@ -209,87 +214,133 @@ public void setInputName(String inputName) {
209214
/**
210215
* @see StrutsResultSupport#doExecute(java.lang.String, ActionInvocation)
211216
*/
217+
@Override
212218
protected void doExecute(String finalLocation, ActionInvocation invocation) throws Exception {
213-
LOG.debug("Find the Response in context");
214-
215-
OutputStream oOutput = null;
216-
217-
try {
218-
String parsedInputName = conditionalParse(inputName, invocation);
219-
boolean evaluated = parsedInputName != null && !parsedInputName.equals(inputName);
220-
boolean reevaluate = !evaluated || isAcceptableExpression(parsedInputName);
221-
if (inputStream == null && reevaluate) {
222-
LOG.debug("Find the inputstream from the invocation variable stack");
223-
inputStream = (InputStream) invocation.getStack().findValue(parsedInputName);
224-
}
225-
226-
if (inputStream == null) {
227-
String msg = ("Can not find a java.io.InputStream with the name [" + parsedInputName + "] in the invocation stack. " +
228-
"Check the <param name=\"inputName\"> tag specified for this action is correct, not excluded and accepted.");
229-
LOG.error(msg);
230-
throw new IllegalArgumentException(msg);
231-
}
219+
resolveInputStream(invocation);
220+
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
221+
222+
applyResponseHeaders(response, invocation);
223+
applyContentLength(response, invocation);
232224

225+
LOG.debug("Streaming result [{}] of type [{}], length [{}], content-disposition [{}] with charset [{}]",
226+
inputName, contentType, contentLength, contentDisposition, contentCharSet);
233227

234-
HttpServletResponse oResponse = invocation.getInvocationContext().getServletResponse();
228+
try (InputStream in = inputStream; OutputStream out = response.getOutputStream()) {
229+
streamContent(in, out);
230+
}
231+
}
235232

236-
LOG.debug("Set the content type: {};charset{}", contentType, contentCharSet);
237-
if (contentCharSet != null && !contentCharSet.isEmpty()) {
238-
oResponse.setContentType(conditionalParse(contentType, invocation) + ";charset=" + conditionalParse(contentCharSet, invocation));
239-
} else {
240-
oResponse.setContentType(conditionalParse(contentType, invocation));
241-
}
233+
/**
234+
* Resolves the input stream from the action invocation.
235+
* <p>
236+
* This method can be overridden by subclasses to provide custom stream sources
237+
* (e.g., from database, cloud storage, or generated content).
238+
* </p>
239+
*
240+
* @param invocation the action invocation
241+
* @throws IllegalArgumentException if the input stream cannot be found
242+
*/
243+
protected void resolveInputStream(ActionInvocation invocation) {
244+
String parsedInputName = conditionalParse(inputName, invocation);
245+
boolean evaluated = parsedInputName != null && !parsedInputName.equals(inputName);
246+
boolean reevaluate = !evaluated || isAcceptableExpression(parsedInputName);
247+
248+
if (inputStream == null && reevaluate) {
249+
LOG.debug("Find the inputstream from the invocation variable stack");
250+
inputStream = (InputStream) invocation.getStack().findValue(parsedInputName);
251+
}
242252

243-
LOG.debug("Set the content length: {}", contentLength);
244-
if (contentLength != null) {
245-
String translatedContentLength = conditionalParse(contentLength, invocation);
246-
int contentLengthAsInt;
247-
try {
248-
contentLengthAsInt = Integer.parseInt(translatedContentLength);
249-
if (contentLengthAsInt >= 0) {
250-
oResponse.setContentLength(contentLengthAsInt);
251-
}
252-
} catch (NumberFormatException e) {
253-
LOG.warn("failed to recognize {} as a number, contentLength header will not be set",
254-
translatedContentLength, e);
255-
}
256-
}
253+
if (inputStream == null) {
254+
String msg = ("Can not find a java.io.InputStream with the name [" + parsedInputName + "] in the invocation stack. " +
255+
"Check the <param name=\"inputName\"> tag specified for this action is correct, not excluded and accepted.");
256+
LOG.error(msg);
257+
throw new IllegalArgumentException(msg);
258+
}
259+
}
257260

258-
LOG.debug("Set the content-disposition: {}", contentDisposition);
259-
if (contentDisposition != null) {
260-
oResponse.addHeader("Content-Disposition", conditionalParse(contentDisposition, invocation));
261-
}
261+
/**
262+
* Applies all response headers including content-type, charset, content-length,
263+
* content-disposition, and cache control headers.
264+
* <p>
265+
* This method can be overridden by subclasses to add custom headers
266+
* (e.g., ETag, X-Custom-Header) or modify caching behavior.
267+
* </p>
268+
*
269+
* @param response the HTTP response
270+
* @param invocation the action invocation
271+
*/
272+
protected void applyResponseHeaders(HttpServletResponse response, ActionInvocation invocation) {
273+
String parsedContentType = conditionalParse(contentType, invocation);
274+
String parsedContentCharSet = conditionalParse(contentCharSet, invocation);
275+
276+
response.setContentType(parsedContentType);
277+
if (StringUtils.isEmpty(parsedContentCharSet)) {
278+
LOG.debug("Set content type to: {} and reset character encoding to null", parsedContentType);
279+
response.setCharacterEncoding((String) null);
280+
} else {
281+
LOG.debug("Set content type: {};charset={}", parsedContentType, parsedContentCharSet);
282+
response.setCharacterEncoding(parsedContentCharSet);
283+
}
262284

263-
LOG.debug("Set the cache control headers if necessary: {}", allowCaching);
264-
if (!allowCaching) {
265-
oResponse.addHeader("Pragma", "no-cache");
266-
oResponse.addHeader("Cache-Control", "no-cache");
267-
}
285+
LOG.debug("Set the content-disposition: {}", contentDisposition);
286+
if (contentDisposition != null) {
287+
response.addHeader("Content-Disposition", conditionalParse(contentDisposition, invocation));
288+
}
268289

269-
oOutput = oResponse.getOutputStream();
290+
LOG.debug("Set the cache control headers if necessary: {}", allowCaching);
291+
if (!allowCaching) {
292+
response.addHeader("Pragma", "no-cache");
293+
response.addHeader("Cache-Control", "no-cache");
294+
}
295+
}
270296

271-
LOG.debug("Streaming result [{}] type=[{}] length=[{}] content-disposition=[{}] charset=[{}]",
272-
inputName, contentType, contentLength, contentDisposition, contentCharSet);
297+
/**
298+
* Applies the content-length header to the response.
299+
* <p>
300+
* This method can be overridden by subclasses for custom length calculation
301+
* or to skip setting the header for chunked transfer encoding.
302+
* </p>
303+
*
304+
* @param response the HTTP response
305+
* @param invocation the action invocation
306+
*/
307+
protected void applyContentLength(HttpServletResponse response, ActionInvocation invocation) {
308+
if (contentLength == null) {
309+
return;
310+
}
273311

274-
LOG.debug("Streaming to output buffer +++ START +++");
275-
byte[] oBuff = new byte[bufferSize];
276-
int iSize;
277-
while (-1 != (iSize = inputStream.read(oBuff))) {
278-
LOG.debug("Sending stream ... {}", iSize);
279-
oOutput.write(oBuff, 0, iSize);
312+
LOG.debug("Set the content length: {}", contentLength);
313+
String translatedContentLength = conditionalParse(contentLength, invocation);
314+
try {
315+
int length = Integer.parseInt(translatedContentLength);
316+
if (length >= 0) {
317+
response.setContentLength(length);
280318
}
281-
LOG.debug("Streaming to output buffer +++ END +++");
319+
} catch (NumberFormatException e) {
320+
LOG.warn("Failed to parse contentLength [{}], header will not be set", translatedContentLength, e);
321+
}
322+
}
282323

283-
// Flush
284-
oOutput.flush();
285-
} finally {
286-
if (inputStream != null) {
287-
inputStream.close();
288-
}
289-
if (oOutput != null) {
290-
oOutput.close();
291-
}
324+
/**
325+
* Streams content from the input stream to the output stream.
326+
* <p>
327+
* This method can be overridden by subclasses to implement custom streaming behavior
328+
* such as progress tracking, compression, or encryption.
329+
* </p>
330+
*
331+
* @param input the input stream to read from
332+
* @param output the output stream to write to
333+
* @throws IOException if an I/O error occurs
334+
*/
335+
protected void streamContent(InputStream input, OutputStream output) throws IOException {
336+
LOG.debug("Streaming to output buffer +++ START +++");
337+
byte[] buffer = new byte[bufferSize];
338+
int bytesRead;
339+
while ((bytesRead = input.read(buffer)) != -1) {
340+
output.write(buffer, 0, bytesRead);
292341
}
342+
LOG.debug("Streaming to output buffer +++ END +++");
343+
output.flush();
293344
}
294345

295346
/**

‎core/src/test/java/org/apache/struts2/result/StreamResultTest.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,21 @@ public void testStreamResultWithCharSet2() throws Exception {
120120
assertEquals("inline", response.getHeader("Content-disposition"));
121121
}
122122

123+
public void testStreamResultWithNullCharSetExpression() throws Exception {
124+
result.setParse(true);
125+
result.setInputName("streamForImage");
126+
result.setContentCharSet("${nullCharSetMethod}");
127+
128+
result.doExecute("helloworld", mai);
129+
130+
assertEquals(contentLength, response.getContentLength());
131+
assertEquals("text/plain", result.getContentType());
132+
// When expression evaluates to null, content-type should NOT include charset
133+
assertEquals("text/plain", response.getContentType());
134+
assertEquals("streamForImage", result.getInputName());
135+
assertEquals("inline", response.getHeader("Content-disposition"));
136+
}
137+
123138
public void testAllowCacheDefault() throws Exception {
124139
result.setInputName("streamForImage");
125140

@@ -310,6 +325,10 @@ public String getStreamForImageAsExpression() {
310325
public String getContentCharSetMethod() {
311326
return "UTF-8";
312327
}
328+
329+
public String getNullCharSetMethod() {
330+
return null;
331+
}
313332
}
314333

315334
}

0 commit comments

Comments
 (0)