363408 GzipFilter should not attempt to compress HTTP status 204

This commit is contained in:
Jan Bartel 2011-11-14 17:29:32 +11:00
parent f7c1c414b5
commit 0e541c4c24
5 changed files with 91 additions and 26 deletions

View File

@ -691,7 +691,7 @@ public class HttpGenerator extends AbstractGenerator
{ {
// we have seen all the _content there is // we have seen all the _content there is
_contentLength = _contentWritten; _contentLength = _contentWritten;
if (content_length == null && (isResponse() || _contentLength>0 || content_type )) if (content_length == null && (isResponse() || _contentLength>0 || content_type ) && !_noContent)
{ {
// known length but not actually set. // known length but not actually set.
_header.put(HttpHeaders.CONTENT_LENGTH_BUFFER); _header.put(HttpHeaders.CONTENT_LENGTH_BUFFER);

View File

@ -120,7 +120,7 @@ public class GzipResponseWrapper extends HttpServletResponseWrapper
public void setStatus(int sc, String sm) public void setStatus(int sc, String sm)
{ {
super.setStatus(sc,sm); super.setStatus(sc,sm);
if (sc<200||sc>=300) if (sc<200 || sc==204 || sc==205 || sc>=300)
noGzip(); noGzip();
} }
@ -131,7 +131,7 @@ public class GzipResponseWrapper extends HttpServletResponseWrapper
public void setStatus(int sc) public void setStatus(int sc)
{ {
super.setStatus(sc); super.setStatus(sc);
if (sc<200||sc>=300) if (sc<200 || sc==204 || sc==205 ||sc>=300)
noGzip(); noGzip();
} }

View File

@ -5,6 +5,7 @@ import java.util.List;
import javax.servlet.Servlet; import javax.servlet.Servlet;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.gzip.GzipResponseWrapper; import org.eclipse.jetty.http.gzip.GzipResponseWrapper;
import org.eclipse.jetty.servlet.DefaultServlet; import org.eclipse.jetty.servlet.DefaultServlet;
import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.FilterHolder;
@ -104,7 +105,7 @@ public class GzipFilterContentLengthTest
try try
{ {
tester.start(); tester.start();
tester.assertIsResponseNotGzipCompressed(filename,filesize); tester.assertIsResponseNotGzipCompressed(filename,filesize,HttpStatus.OK_200);
} }
finally finally
{ {

View File

@ -1,5 +1,13 @@
package org.eclipse.jetty.servlets; package org.eclipse.jetty.servlets;
import java.io.IOException;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.gzip.GzipResponseWrapper; import org.eclipse.jetty.http.gzip.GzipResponseWrapper;
import org.eclipse.jetty.servlet.DefaultServlet; import org.eclipse.jetty.servlet.DefaultServlet;
import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.FilterHolder;
@ -13,6 +21,29 @@ import org.junit.Test;
*/ */
public class GzipFilterDefaultTest public class GzipFilterDefaultTest
{ {
public static class HttpStatusServlet extends HttpServlet
{
private int _status = 204;
public HttpStatusServlet()
{
super();
}
public void setStatus (int status)
{
_status = status;
}
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
resp.setStatus(_status);
}
}
@Rule @Rule
public TestingDir testingdir = new TestingDir(); public TestingDir testingdir = new TestingDir();
@ -77,11 +108,33 @@ public class GzipFilterDefaultTest
try try
{ {
tester.start(); tester.start();
tester.assertIsResponseNotGzipCompressed("file.mp3", filesize); tester.assertIsResponseNotGzipCompressed("file.mp3", filesize, HttpStatus.OK_200);
} }
finally finally
{ {
tester.stop(); tester.stop();
} }
} }
@Test
public void testIsNotGzipCompressedHttpStatus() throws Exception
{
GzipTester tester = new GzipTester(testingdir);
// Test error code 204
FilterHolder holder = tester.setContentServlet(HttpStatusServlet.class);
holder.setInitParameter("mimeTypes","text/plain");
try
{
tester.start();
tester.assertIsResponseNotGzipCompressed(null, -1, 204);
}
finally
{
tester.stop();
}
}
} }

View File

@ -202,7 +202,7 @@ public class GzipTester
* passing -1 will disable the Content-Length assertion) * passing -1 will disable the Content-Length assertion)
* @throws Exception * @throws Exception
*/ */
public void assertIsResponseNotGzipCompressed(String filename, int expectedFilesize) throws Exception public void assertIsResponseNotGzipCompressed(String filename, int expectedFilesize, int status) throws Exception
{ {
System.err.printf("[GzipTester] requesting /context/%s%n",filename); System.err.printf("[GzipTester] requesting /context/%s%n",filename);
HttpTester request = new HttpTester(); HttpTester request = new HttpTester();
@ -212,6 +212,9 @@ public class GzipTester
request.setVersion("HTTP/1.0"); request.setVersion("HTTP/1.0");
request.setHeader("Host","tester"); request.setHeader("Host","tester");
request.setHeader("Accept-Encoding","gzip"); request.setHeader("Accept-Encoding","gzip");
if (filename == null)
request.setURI("/context/");
else
request.setURI("/context/"+filename); request.setURI("/context/"+filename);
// Issue the request // Issue the request
@ -222,7 +225,7 @@ public class GzipTester
// Assert the response headers // Assert the response headers
Assert.assertThat("Response.method",response.getMethod(),nullValue()); Assert.assertThat("Response.method",response.getMethod(),nullValue());
Assert.assertThat("Response.status",response.getStatus(),is(HttpServletResponse.SC_OK)); Assert.assertThat("Response.status",response.getStatus(),is(status));
if (expectedFilesize != (-1)) if (expectedFilesize != (-1))
{ {
Assert.assertThat("Response.header[Content-Length]",response.getHeader("Content-Length"),notNullValue()); Assert.assertThat("Response.header[Content-Length]",response.getHeader("Content-Length"),notNullValue());
@ -232,6 +235,8 @@ public class GzipTester
Assert.assertThat("Response.header[Content-Encoding]",response.getHeader("Content-Encoding"),not(containsString("gzip"))); Assert.assertThat("Response.header[Content-Encoding]",response.getHeader("Content-Encoding"),not(containsString("gzip")));
// Assert that the contents are what we expect. // Assert that the contents are what we expect.
if (filename != null)
{
File serverFile = testdir.getFile(filename); File serverFile = testdir.getFile(filename);
String expected = IO.readToString(serverFile); String expected = IO.readToString(serverFile);
String actual = null; String actual = null;
@ -253,6 +258,10 @@ public class GzipTester
IO.close(in); IO.close(in);
} }
} }
}
/** /**
* Generate string content of arbitrary length. * Generate string content of arbitrary length.
@ -343,6 +352,8 @@ public class GzipTester
return holder; return holder;
} }
public void setEncoding(String encoding) public void setEncoding(String encoding)
{ {
this.encoding = encoding; this.encoding = encoding;