Skip to content

Commit

Permalink
Fix InputStream caching in ContentCachingReqWrapper
Browse files Browse the repository at this point in the history
Prior to this commit, the ContentCachingRequestWrapper would immediately
consume the wrapped request's InputStream when asked for the cached
content; that caused several issues:

* the request body was read in memory even if it wasn't yet consumed by
the application, leading to inefficiencies.
* when requesting the InputStream, an empty InputStream was returned
since the original was already read.

This case only happened for form POSTs requests.

This commit makes sure that the wrapper does not alter the request
expected behavior:

* when getting the inputstream, it is wrapped in order to cache its
content
* when getting request parameters, the request body is cached and its
inputstream is consumed, as expected

Issue: SPR-12810
  • Loading branch information
bclozel committed Mar 13, 2015
1 parent 88a1448 commit c6250f5
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 8 deletions.
Expand Up @@ -20,7 +20,6 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.util.Arrays;
import java.util.Enumeration;
Expand Down Expand Up @@ -88,14 +87,36 @@ public BufferedReader getReader() throws IOException {
return this.reader;
}

/**
* Return the cached request content as a byte array.
*/
public byte[] getContentAsByteArray() {
@Override
public String getParameter(String name) {
if(this.cachedContent.size() == 0 && isFormPost()) {
writeRequestParamsToContent();
}
return this.cachedContent.toByteArray();
return super.getParameter(name);
}

@Override
public Map<String, String[]> getParameterMap() {
if(this.cachedContent.size() == 0 && isFormPost()) {
writeRequestParamsToContent();
}
return super.getParameterMap();
}

@Override
public Enumeration<String> getParameterNames() {
if(this.cachedContent.size() == 0 && isFormPost()) {
writeRequestParamsToContent();
}
return super.getParameterNames();
}

@Override
public String[] getParameterValues(String name) {
if(this.cachedContent.size() == 0 && isFormPost()) {
writeRequestParamsToContent();
}
return super.getParameterValues(name);
}

private boolean isFormPost() {
Expand All @@ -107,7 +128,7 @@ private void writeRequestParamsToContent() {
try {
if (this.cachedContent.size() == 0) {
String requestEncoding = getCharacterEncoding();
Map<String, String[]> form = getParameterMap();
Map<String, String[]> form = super.getParameterMap();
for (Iterator<String> nameIterator = form.keySet().iterator(); nameIterator.hasNext(); ) {
String name = nameIterator.next();
List<String> values = Arrays.asList(form.get(name));
Expand All @@ -133,6 +154,13 @@ private void writeRequestParamsToContent() {
}
}

/**
* Return the cached request content as a byte array.
*/
public byte[] getContentAsByteArray() {
return this.cachedContent.toByteArray();
}

private class ContentCachingInputStream extends ServletInputStream {

private final ServletInputStream is;
Expand All @@ -150,5 +178,4 @@ public int read() throws IOException {
return ch;
}
}

}
Expand Up @@ -61,6 +61,23 @@ public void requestParams() throws Exception {
// getting request parameters will consume the request body
Assert.assertFalse(wrapper.getParameterMap().isEmpty());
Assert.assertEquals("first=value&second=foo&second=bar", new String(wrapper.getContentAsByteArray()));
// SPR-12810 : inputstream body should be consumed
Assert.assertEquals("", new String(FileCopyUtils.copyToByteArray(wrapper.getInputStream())));
}

// SPR-12810
@Test
public void inputStreamFormPostRequest() throws Exception {
this.request.setMethod("POST");
this.request.setContentType(FORM_CONTENT_TYPE);
this.request.setCharacterEncoding(CHARSET);
this.request.setParameter("first", "value");
this.request.setParameter("second", new String[] {"foo", "bar"});

ContentCachingRequestWrapper wrapper = new ContentCachingRequestWrapper(this.request);

byte[] response = FileCopyUtils.copyToByteArray(wrapper.getInputStream());
Assert.assertArrayEquals(response, wrapper.getContentAsByteArray());
}

}

0 comments on commit c6250f5

Please sign in to comment.