From 151e96cac4b3ea0afa42a0c341a7f2ec37b5db72 Mon Sep 17 00:00:00 2001
From: Brian Clozel <bclozel@pivotal.io>
Date: Fri, 13 Mar 2015 10:30:29 +0100
Subject: [PATCH] Fix InputStream caching in ContentCachingReqWrapper

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
---
 .../util/ContentCachingRequestWrapper.java    | 43 +++++++++++++++----
 .../ContentCachingRequestWrapperTests.java    | 17 ++++++++
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/spring-web/src/main/java/org/springframework/web/util/ContentCachingRequestWrapper.java b/spring-web/src/main/java/org/springframework/web/util/ContentCachingRequestWrapper.java
index 27a1c441a9c..57fefe535fd 100644
--- a/spring-web/src/main/java/org/springframework/web/util/ContentCachingRequestWrapper.java
+++ b/spring-web/src/main/java/org/springframework/web/util/ContentCachingRequestWrapper.java
@@ -20,7 +20,6 @@ import java.io.BufferedReader;
 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;
@@ -88,14 +87,36 @@ public class ContentCachingRequestWrapper extends HttpServletRequestWrapper {
 		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() {
@@ -107,7 +128,7 @@ public class ContentCachingRequestWrapper extends HttpServletRequestWrapper {
 		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));
@@ -133,6 +154,13 @@ public class ContentCachingRequestWrapper extends HttpServletRequestWrapper {
 		}
 	}
 
+	/**
+	 * 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;
@@ -150,5 +178,4 @@ public class ContentCachingRequestWrapper extends HttpServletRequestWrapper {
 			return ch;
 		}
 	}
-
 }
diff --git a/spring-web/src/test/java/org/springframework/web/util/ContentCachingRequestWrapperTests.java b/spring-web/src/test/java/org/springframework/web/util/ContentCachingRequestWrapperTests.java
index 45e383efbdd..69661536f95 100644
--- a/spring-web/src/test/java/org/springframework/web/util/ContentCachingRequestWrapperTests.java
+++ b/spring-web/src/test/java/org/springframework/web/util/ContentCachingRequestWrapperTests.java
@@ -61,6 +61,23 @@ public class ContentCachingRequestWrapperTests {
 		// 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());
 	}
 
 }