Skip to content
Browse files

buffer disabling was not being honoured if the application subsequent…

…ly wrote back to the output stream. This patch checks to make sure that the stream has been disabled at some point and if so, will not post process it. It is based on the assumption that disabling comes from shouldNotProcess methods on the Selector
  • Loading branch information...
1 parent b53cf8a commit 9ebf545d86c1b276a4ab551cdfc19ffff24972b5 @rvowles rvowles committed with joewalnes Oct 28, 2009
View
2 sitemesh/src/main/java/org/sitemesh/webapp/contentfilter/ContentBufferingFilter.java
@@ -171,7 +171,7 @@ public void preCommit() {
// If content was buffered, post-process it.
boolean processed = false;
- if (buffer != null) {
+ if (buffer != null && !responseBuffer.bufferingWasDisabled()) {
processed = postProcess(responseBuffer.getContentType(), buffer, request, response, metaData);
}
View
6 sitemesh/src/main/java/org/sitemesh/webapp/contentfilter/HttpServletResponseBuffer.java
@@ -34,6 +34,7 @@
private final ResponseMetaData metaData;
private Buffer buffer;
+ private boolean bufferingWasDisabled = false;
public HttpServletResponseBuffer(final HttpServletResponse originalResponse, ResponseMetaData metaData, Selector selector) {
@@ -123,6 +124,7 @@ public ServletOutputStream create() {
*/
protected void disableBuffering() {
buffer = null;
+ bufferingWasDisabled = true;
routablePrintWriter.updateDestination(new RoutablePrintWriter.DestinationFactory() {
public PrintWriter activateDestination() throws IOException {
preCommit();
@@ -136,6 +138,10 @@ public ServletOutputStream create() throws IOException {
}
});
}
+
+ public boolean bufferingWasDisabled() {
+ return bufferingWasDisabled;
+ }
/**
* Hook that is called just before the response is committed. Last chance to modify headers.
View
55 sitemesh/src/test/java/org/sitemesh/webapp/contentfilter/ContentBufferingFilterTest.java
@@ -1,12 +1,13 @@
package org.sitemesh.webapp.contentfilter;
-import junit.framework.TestCase;
+import java.io.IOException;
+import java.nio.CharBuffer;
+import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import javax.servlet.ServletException;
-import java.nio.CharBuffer;
-import java.io.IOException;
+
+import junit.framework.TestCase;
import org.sitemesh.webapp.WebEnvironment;
@@ -23,6 +24,12 @@ protected MyContentBufferingFilter() {
super(new BasicSelector(false, "text/html"));
}
}
+
+ private static abstract class MyContentBufferingFilterDecorateErrorPages extends ContentBufferingFilter {
+ protected MyContentBufferingFilterDecorateErrorPages() {
+ super(new BasicSelector(true, "text/html"));
+ }
+ }
public void testCanRewriteContent() throws Exception {
WebEnvironment webEnvironment = new WebEnvironment.Builder()
@@ -66,18 +73,52 @@ public void testStatusCode404DoesntProcess() throws Exception {
.addFilter("/filtered", new MyContentBufferingFilter() {
@Override
protected boolean postProcess(String contentType, CharBuffer buffer, HttpServletRequest request, HttpServletResponse response, ResponseMetaData metaData) throws IOException, ServletException {
-// throw new RuntimeException("not me!");
- return false;
+ response.getOutputStream().print("1234567890");
+ return true;
}
})
.create();
webEnvironment.doGet("/filtered");
assertEquals(404, webEnvironment.getStatus());
+ assertEquals(
+ "HTTP/1.1 404 Not Found\n" +
+ "Content-Type: text/html\n" +
+ "Transfer-Encoding: chunked\n" +
+ "\n" +
+ "1\n" +
+ "1\n" +
+ "0", // <---
+ webEnvironment.getRawResponse());
// did a code coverage check and it does check the status line
}
-
+
+ public void testErrorPagesShouldBeMarkedUp() throws Exception {
+ WebEnvironment webEnvironment = new WebEnvironment.Builder()
+ .addStatusCodeFail("/filtered", 404, "text/html", "1")
+ .addFilter("/filtered", new MyContentBufferingFilterDecorateErrorPages() {
+ @Override
+ protected boolean postProcess(String contentType, CharBuffer buffer, HttpServletRequest request, HttpServletResponse response, ResponseMetaData metaData) throws IOException, ServletException {
+ response.getOutputStream().print("1234567890");
+ return true;
+ }
+ })
+ .create();
+
+ webEnvironment.doGet("/filtered");
+
+ assertEquals(404, webEnvironment.getStatus());
+ assertEquals(
+ "HTTP/1.1 404 Not Found\n" +
+ "Content-Type: text/html\n" +
+ "Content-Length: 10\n" + // <---
+ "\n" +
+ "1234567890",
+ webEnvironment.getRawResponse());
+ // with this check, we now have a fully covered status line
+ }
+
public void testUpdatesContentLengthHeader() throws Exception {
WebEnvironment webEnvironment = new WebEnvironment.Builder()

0 comments on commit 9ebf545

Please sign in to comment.
Something went wrong with that request. Please try again.