Commit d8cf616c authored by Andrea Aime's avatar Andrea Aime
Browse files

[GEOS-7016] Enabling request body logging ruins binary data POST/PUT requests

parent 5f9014fb
......@@ -18,8 +18,8 @@ import javax.servlet.ServletInputStream;
public class BufferedRequestStream extends ServletInputStream{
InputStream myInputStream;
public BufferedRequestStream(String buff) throws IOException {
myInputStream = new ByteArrayInputStream(buff.getBytes());
public BufferedRequestStream(byte[] buff) throws IOException {
myInputStream = new ByteArrayInputStream(buff);
myInputStream.mark(16);
myInputStream.read();
myInputStream.reset();
......
......@@ -27,24 +27,27 @@ import javax.servlet.http.HttpServletRequestWrapper;
import org.geotools.util.Converters;
public class BufferedRequestWrapper extends HttpServletRequestWrapper{
public class BufferedRequestWrapper extends HttpServletRequestWrapper {
protected HttpServletRequest myWrappedRequest;
protected String myBuffer;
protected byte[] myBuffer;
protected String charset;
protected ServletInputStream myStream = null;
protected BufferedReader myReader = null;
protected Map myParameterMap;
protected Logger logger =
org.geotools.util.logging.Logging.getLogger("org.geoserver.filters");
public BufferedRequestWrapper(HttpServletRequest req, String buff){
public BufferedRequestWrapper(HttpServletRequest req, String charset, byte[] buff) {
super(req);
myWrappedRequest = req;
myBuffer = buff;
logger.fine("Created BufferedRequestWrapper with String: \"" + buff + "\" as buffer");
this.myWrappedRequest = req;
this.myBuffer = buff;
this.charset = charset;
}
public ServletInputStream getInputStream() throws IOException{
if (myStream == null){
if (myStream == null) {
if (myReader == null){
myStream = new BufferedRequestStream(myBuffer);
} else {
......@@ -58,12 +61,8 @@ public class BufferedRequestWrapper extends HttpServletRequestWrapper{
public BufferedReader getReader() throws IOException{
if (myReader == null){
if (myStream == null){
myReader = new BufferedReader(
new InputStreamReader(
new BufferedRequestStream(myBuffer)
)
);
myReader = new BufferedReader(new InputStreamReader(new BufferedRequestStream(
myBuffer), charset));
} else {
throw new IOException("Requesting a reader after a stream is already in use!!");
}
......@@ -88,7 +87,7 @@ public class BufferedRequestWrapper extends HttpServletRequestWrapper{
while (it.hasNext()){
Map.Entry entry = (Map.Entry)it.next();
toArrays.put(entry.getKey(),
(String[])((List)entry.getValue()).toArray(new String[0]));
((List)entry.getValue()).toArray(new String[0]));
}
return Collections.unmodifiableMap(toArrays);
......@@ -132,7 +131,13 @@ public class BufferedRequestWrapper extends HttpServletRequestWrapper{
myParameterMap = new TreeMap();
// parse the body
String[] pairs = myBuffer.split("\\&");
String[] pairs;
try {
pairs = new String(myBuffer, charset).split("\\&");
} catch (UnsupportedEncodingException e) {
// should not happen
throw new RuntimeException(e);
}
for (int i = 0; i < pairs.length; i++){
parsePair(pairs[i]);
......
......@@ -5,8 +5,9 @@
*/
package org.geoserver.filters;
import java.io.BufferedReader;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.logging.Logger;
import javax.servlet.Filter;
......@@ -17,6 +18,8 @@ import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import org.apache.commons.io.IOUtils;
/**
* Filter to log requests for debugging or statistics-gathering purposes.
*
......@@ -53,15 +56,22 @@ public class LoggingFilter implements Filter {
if (logBodies && (hreq.getMethod().equals("PUT") || hreq.getMethod().equals("POST"))){
message += " request-size: " + hreq.getContentLength();
message += " body: ";
StringBuffer buff = new StringBuffer();
BufferedReader reader = hreq.getReader();
char[] readIn = new char[256];
int amountRead = 0;
while ((amountRead = reader.read(readIn, 0 , 256)) != -1){
buff.append(readIn, 0, amountRead);
String encoding = hreq.getCharacterEncoding();
if (encoding == null) {
// the default encoding for HTTP 1.1
encoding = "ISO-8859-1";
}
ByteArrayOutputStream bos = new ByteArrayOutputStream();
byte[] bytes;
try (InputStream is = hreq.getInputStream()) {
IOUtils.copy(is, bos);
bytes = bos.toByteArray();
body = new String(bytes, encoding);
}
body = buff.toString();
req = new BufferedRequestWrapper(hreq, buff.toString());
req = new BufferedRequestWrapper(hreq, encoding, bytes);
}
} else {
message = "" + req.getRemoteHost() + " made a non-HTTP request";
......@@ -99,4 +109,32 @@ public class LoggingFilter implements Filter {
public void destroy() {
}
/**
* @return the enabled
*/
public boolean isEnabled() {
return enabled;
}
/**
* @param enabled the enabled to set
*/
public void setEnabled(boolean enabled) {
this.enabled = enabled;
}
/**
* @return the logBodies
*/
public boolean isLogBodies() {
return logBodies;
}
/**
* @param logBodies the logBodies to set
*/
public void setLogBodies(boolean logBodies) {
this.logBodies = logBodies;
}
}
......@@ -5,7 +5,7 @@
*/
package org.geoserver.filters;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import org.junit.Before;
import org.junit.Test;
......@@ -21,7 +21,7 @@ public class BufferedRequestStreamTest {
@Before
public void setUpInternal() throws Exception{
myTestString = "Hello, this is a test";
myBRS = new BufferedRequestStream(myTestString);
myBRS = new BufferedRequestStream(myTestString.getBytes());
}
@Test
......
......@@ -14,61 +14,66 @@ import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;
import org.junit.Test;
import org.springframework.web.util.WebUtils;
public class BufferedRequestWrapperTest extends RequestWrapperTestSupport {
@Test
public void testGetInputStream() throws Exception{
for (int i = 0; i < testStrings.length; i++){
doInputStreamTest(testStrings[i]);
}
}
public void testGetInputStream() throws Exception {
for (int i = 0; i < testStrings.length; i++) {
doInputStreamTest(testStrings[i]);
}
}
@Test
public void testGetReader() throws Exception{
for (int i = 0; i < testStrings.length; i++){
doGetReaderTest(testStrings[i]);
}
}
@Test
public void testGetReader() throws Exception {
for (int i = 0; i < testStrings.length; i++) {
doGetReaderTest(testStrings[i]);
}
}
public void doInputStreamTest(String testString) throws Exception{
HttpServletRequest req = makeRequest(testString, null);
public void doInputStreamTest(String testString) throws Exception {
HttpServletRequest req = makeRequest(testString, null);
BufferedRequestWrapper wrapper = new BufferedRequestWrapper(req, testString);
ServletInputStream sis = req.getInputStream();
byte b[] = new byte[32];
int amountRead;
BufferedRequestWrapper wrapper = new BufferedRequestWrapper(req,
WebUtils.DEFAULT_CHARACTER_ENCODING, testString.getBytes());
ServletInputStream sis = req.getInputStream();
byte b[] = new byte[32];
int amountRead;
while (( sis.readLine(b, 0, 32)) > 0){ /*clear out the request body*/ }
while ((sis.readLine(b, 0, 32)) > 0) { /* clear out the request body */
}
sis = wrapper.getInputStream();
StringBuffer buff = new StringBuffer();
sis = wrapper.getInputStream();
StringBuffer buff = new StringBuffer();
while ((amountRead = sis.readLine(b, 0, 32)) != 0){
buff.append(new String(b, 0, amountRead));
}
while ((amountRead = sis.readLine(b, 0, 32)) != 0) {
buff.append(new String(b, 0, amountRead));
}
assertEquals(buff.toString(), testString);
}
assertEquals(buff.toString(), testString);
}
public void doGetReaderTest(String testString) throws Exception{
HttpServletRequest req = makeRequest(testString, null);
public void doGetReaderTest(String testString) throws Exception {
HttpServletRequest req = makeRequest(testString, null);
BufferedReader br = req.getReader();
while ((br.readLine()) != null){ /* clear out the body */ }
BufferedReader br = req.getReader();
while ((br.readLine()) != null) { /* clear out the body */
}
BufferedRequestWrapper wrapper = new BufferedRequestWrapper(req, testString);
StringBuffer buff = new StringBuffer();
BufferedRequestWrapper wrapper = new BufferedRequestWrapper(req,
WebUtils.DEFAULT_CHARACTER_ENCODING, testString.getBytes());
StringBuffer buff = new StringBuffer();
int c;
br = wrapper.getReader();
while ((c = br.read()) != -1){
buff.append((char)c);
}
assertEquals(buff.toString(), testString);
}
br = wrapper.getReader();
while ((c = br.read()) != -1) {
buff.append((char) c);
}
assertEquals(buff.toString(), testString);
}
@Test
public void testMixedRequest() throws Exception {
String body = "a=1&b=2";
......@@ -76,9 +81,10 @@ public class BufferedRequestWrapperTest extends RequestWrapperTestSupport {
HttpServletRequest req = makeRequest(body, queryString);
BufferedReader br = req.getReader();
while ((br.readLine()) != null){ /* clear out the body */ }
while ((br.readLine()) != null) { /* clear out the body */
}
BufferedRequestWrapper wrapper = new BufferedRequestWrapper(req, body);
BufferedRequestWrapper wrapper = new BufferedRequestWrapper(req, "UTF-8", body.getBytes());
Map params = wrapper.getParameterMap();
assertEquals(4, params.size());
assertEquals("1", ((String[]) params.get("a"))[0]);
......
......@@ -16,15 +16,20 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStreamWriter;
import java.net.URL;
import java.util.Collections;
import java.util.List;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import java.util.zip.ZipInputStream;
import java.util.zip.ZipOutputStream;
import javax.servlet.Filter;
import org.apache.commons.io.IOUtils;
import org.geoserver.catalog.Catalog;
import org.geoserver.catalog.DataStoreInfo;
import org.geoserver.data.test.MockData;
import org.geoserver.filters.LoggingFilter;
import org.geotools.data.DataUtilities;
import org.h2.tools.DeleteDbFiles;
import org.junit.After;
......@@ -37,6 +42,14 @@ import com.mockrunner.mock.web.MockHttpServletResponse;
public class DataStoreFileUploadTest extends CatalogRESTTestSupport {
@Override
protected List<Filter> getFilters() {
LoggingFilter filter = new LoggingFilter();
filter.setEnabled(true);
filter.setLogBodies(true);
return Collections.singletonList((Filter) filter);
}
@Before
public void removePdsDataStore() {
removeStore("gs", "pds");
......
......@@ -114,7 +114,7 @@
parameter defaults to false.
-->
<param-name>enabled</param-name>
<param-value>false</param-value>
<param-value>true</param-value>
</init-param>
<init-param>
<!-- The 'log-request-bodies' parameter is a boolean value, "true" (case-insensitive) for
......@@ -125,7 +125,7 @@
server.
-->
<param-name>log-request-bodies</param-name>
<param-value>false</param-value>
<param-value>true</param-value>
</init-param>
</filter>
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment