Merge pull request #55 from SAP/fix/InputReader

Fix Request Content-Length Calculation when using Reader

Author: KarstenSchnitter

cf-java-logging-support-servlet/src/main/java/com/sap/hcp/cf/logging/servlet/filter/ContentLengthTrackingRequestWrapper.java

@@ -35,7 +35,7 @@ public ServletInputStream getInputStream() throws IOException {
     @Override
     public BufferedReader getReader() throws IOException {
         wrappedReader = new WrappedInputReader(super.getReader());
-        return wrappedReader;
+		return new BufferedReader(wrappedReader);
     }
 
     @Override

cf-java-logging-support-servlet/src/main/java/com/sap/hcp/cf/logging/servlet/filter/WrappedInputReader.java

@@ -1,16 +1,16 @@
 package com.sap.hcp.cf.logging.servlet.filter;
 
-import java.io.BufferedReader;
+import java.io.FilterReader;
 import java.io.IOException;
+import java.io.Reader;
 
-public class WrappedInputReader extends BufferedReader {
+public class WrappedInputReader extends FilterReader {
 
-    private final BufferedReader wrappedReader;
     private int contentLength = -1;
+	private int markContentLength = -1;
 
-    protected WrappedInputReader(BufferedReader in) {
-        super(in, 1); // unused
-        wrappedReader = in;
+	protected WrappedInputReader(Reader in) {
+		super(in);
     }
 
     public int getContentLength() {
@@ -19,7 +19,7 @@ public int getContentLength() {
 
     @Override
     public int read() throws IOException {
-        int c = wrappedReader.read();
+		int c = in.read();
         if (c != -1) {
             incrContentLength(1);
         }
@@ -28,21 +28,42 @@ public int read() throws IOException {
 
     @Override
     public int read(char[] cbuf, int off, int len) throws IOException {
-        int c = wrappedReader.read(cbuf, off, len);
+		int c = in.read(cbuf, off, len);
         if (c != -1) {
             incrContentLength(c);
         }
         return c;
     }
 
-    @Override
-    public String readLine() throws IOException {
-        String s = wrappedReader.readLine();
-        if (s != null) {
-            incrContentLength(s.length());
-        }
-        return s;
-    }
+	@Override
+	public long skip(long n) throws IOException {
+		long skipped = in.skip(n);
+		incrContentLength(toIntExact(skipped));
+		return skipped;
+	}
+
+	private static int toIntExact(long value) {
+		if ((int) value != value) {
+			throw new ArithmeticException("integer overflow");
+		}
+		return (int) value;
+	}
+
+	@Override
+	public void mark(int readAheadLimit) throws IOException {
+		super.mark(readAheadLimit);
+		synchronized (lock) {
+			this.markContentLength = contentLength;
+		}
+	}
+
+	@Override
+	public void reset() throws IOException {
+		super.reset();
+		synchronized (lock) {
+			this.contentLength = markContentLength;
+		}
+	}
 
     private void incrContentLength(int i) {
         if (contentLength == -1) {

cf-java-logging-support-servlet/src/test/java/com/sap/hcp/cf/logging/servlet/filter/RequestLoggingFilterTest.java

@@ -12,6 +12,7 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.io.StringReader;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -118,10 +119,9 @@ public void doFilter(ServletRequest request, ServletResponse response)
 
 	@Test
 	public void testReader() throws IOException, ServletException {
-		BufferedReader mockReader = mock(BufferedReader.class);
+		BufferedReader reader = new BufferedReader(new StringReader("TEST"));
 
-		when(mockReq.getReader()).thenReturn(mockReader);
-		when(mockReader.read()).thenReturn(1);
+		when(mockReq.getReader()).thenReturn(reader);
 		FilterChain mockFilterChain = new FilterChain() {
 			@Override
 			public void doFilter(ServletRequest request, ServletResponse response)
@@ -136,7 +136,7 @@ public void doFilter(ServletRequest request, ServletResponse response)
 		assertThat(getField(Fields.REMOTE_HOST), is(Defaults.UNKNOWN));
 		assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN));
 		assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN));
-		assertThat(getField(Fields.REQUEST_SIZE_B), is("1"));
+		assertThat(getField(Fields.REQUEST_SIZE_B), is("4"));
 		assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN));
 	}
 

cf-java-logging-support-servlet/src/test/java/com/sap/hcp/cf/logging/servlet/filter/WrappedInputReaderTest.java

@@ -0,0 +1,93 @@
+package com.sap.hcp.cf.logging.servlet.filter;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.StringReader;
+
+import org.junit.Test;
+
+public class WrappedInputReaderTest {
+
+	private static final String MESSAGE = "ABCDEFGH";
+
+	private static WrappedInputReader wrap(String text) {
+		return new WrappedInputReader(new BufferedReader(new StringReader(text)));
+	}
+
+	@Test
+	public void unreadInputGivesMinusOne() throws Exception {
+		WrappedInputReader reader = wrap(MESSAGE);
+
+		assertThat(reader.getContentLength(), is(equalTo(-1)));
+	}
+
+	@Test
+	public void readSingleCharacter() throws Exception {
+		WrappedInputReader reader = wrap(MESSAGE);
+
+		int read = reader.read();
+		
+		assertThat((char) read, is(equalTo('A')));
+		assertThat(reader.getContentLength(), is(equalTo(1)));
+	}
+
+	@Test
+	public void readCharacterArray() throws Exception {
+		WrappedInputReader reader = wrap(MESSAGE);
+		char[] cbuf = new char[3];
+
+		reader.read(cbuf);
+
+		assertThat(new String(cbuf), is(equalTo("ABC")));
+		assertThat(reader.getContentLength(), is(equalTo(3)));
+	}
+
+	@Test
+	public void readCharacterArrayWithOffset() throws Exception {
+		WrappedInputReader reader = wrap(MESSAGE);
+
+		char[] cbuf = new char[5];
+		reader.read(cbuf, 1, 4);
+
+		assertThat(cbuf, is(equalTo(new char[] { 0, 'A', 'B', 'C', 'D' })));
+		assertThat(reader.getContentLength(), is(equalTo(4)));
+	}
+
+
+	@Test
+	public void skipCharacters() throws Exception {
+		WrappedInputReader reader = wrap(MESSAGE);
+
+		long skipped = reader.skip(3);
+
+		assertThat(reader.getContentLength(), is(equalTo((int) skipped)));
+	}
+
+	@Test
+	public void markAndReset() throws Exception {
+		WrappedInputReader reader = wrap(MESSAGE);
+
+		reader.mark(1);
+		int blind = reader.read();
+		reader.reset();
+		String text = consume(reader);
+
+		assertThat((char) blind, is(equalTo('A')));
+		assertThat(text, is(equalTo(MESSAGE)));
+		assertThat(reader.getContentLength(), is(equalTo(MESSAGE.length())));
+	}
+
+	private String consume(WrappedInputReader reader) throws IOException {
+		StringBuffer buffer = new StringBuffer();
+		int current;
+		while ((current = reader.read()) != -1) {
+			buffer.append((char) current);
+		}
+		return buffer.toString();
+	}
+
+}