408909 GzipFilter setting of headers when reset and/or not compressed

The gzip filter now sets deferred headers (content-length and etag) when it decides not to commit.
Also does not allow a reset after a decision to commit
This commit is contained in:
Greg Wilkins 2013-05-27 16:09:12 +10:00
parent a1a6f0e5f2
commit 16994cbd6c
12 changed files with 159 additions and 34 deletions

View File

@ -65,22 +65,31 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
doCompress();
}
/* ------------------------------------------------------------ */
/**
* Reset buffer.
*/
public void resetBuffer()
{
if (_response.isCommitted())
if (_response.isCommitted() || _compressedOutputStream!=null )
throw new IllegalStateException("Committed");
_closed = false;
_out = null;
_bOut = null;
if (_compressedOutputStream != null)
_response.setHeader("Content-Encoding",null);
_compressedOutputStream = null;
_doNotCompress = false;
}
/* ------------------------------------------------------------ */
public void setBufferSize(int bufferSize)
{
if (_bOut!=null && _bOut.getBuf().length<bufferSize)
{
ByteArrayOutputStream2 b = new ByteArrayOutputStream2(bufferSize);
b.write(_bOut.getBuf(),0,_bOut.size());
_bOut=b;
}
}
/* ------------------------------------------------------------ */
public void setContentLength()
{
@ -170,7 +179,7 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
if (_out == null || _bOut != null)
{
long length=_wrapper.getContentLength();
if (length > 0 && length < _wrapper.getMinCompressSize())
if (length >= 0 && length < _wrapper.getMinCompressSize())
doNotCompress(false);
else
doCompress();
@ -359,4 +368,5 @@ public abstract class AbstractCompressedStream extends ServletOutputStream
*/
protected abstract DeflaterOutputStream createStream() throws IOException;
}

View File

@ -107,6 +107,8 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
public void setBufferSize(int bufferSize)
{
_bufferSize = bufferSize;
if (_compressedStream!=null)
_compressedStream.setBufferSize(bufferSize);
}
/* ------------------------------------------------------------ */
@ -127,18 +129,21 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
{
super.setContentType(ct);
if (ct!=null)
if (!_noCompression)
{
int colon=ct.indexOf(";");
if (colon>0)
ct=ct.substring(0,colon);
}
if ((_compressedStream==null || _compressedStream.getOutputStream()==null) &&
(_mimeTypes==null && ct!=null && ct.contains("gzip") ||
_mimeTypes!=null && (ct==null||!_mimeTypes.contains(StringUtil.asciiToLowerCase(ct)))))
{
noCompression();
if (ct!=null)
{
int colon=ct.indexOf(";");
if (colon>0)
ct=ct.substring(0,colon);
}
if ((_compressedStream==null || _compressedStream.getOutputStream()==null) &&
(_mimeTypes==null && ct!=null && ct.contains("gzip") ||
_mimeTypes!=null && (ct==null||!_mimeTypes.contains(StringUtil.asciiToLowerCase(ct)))))
{
noCompression();
}
}
}
@ -173,7 +178,10 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
@Override
public void setContentLength(int length)
{
setContentLength((long)length);
if (_noCompression)
super.setContentLength(length);
else
setContentLength((long)length);
}
/* ------------------------------------------------------------ */
@ -311,6 +319,8 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
*/
public void noCompression()
{
if (!_noCompression)
setDeferredHeaders();
_noCompression=true;
if (_compressedStream!=null)
{
@ -335,6 +345,25 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
_writer.flush();
if (_compressedStream!=null)
_compressedStream.finish();
else
setDeferredHeaders();
}
/* ------------------------------------------------------------ */
private void setDeferredHeaders()
{
if (!isCommitted())
{
if (_contentLength>=0)
{
if (_contentLength < Integer.MAX_VALUE)
super.setContentLength((int)_contentLength);
else
super.setHeader("Content-Length",Long.toString(_contentLength));
}
if(_etag!=null)
super.setHeader("ETag",_etag);
}
}
/* ------------------------------------------------------------ */
@ -344,7 +373,9 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
@Override
public void setHeader(String name, String value)
{
if ("content-length".equalsIgnoreCase(name))
if (_noCompression)
super.setHeader(name,value);
else if ("content-length".equalsIgnoreCase(name))
{
setContentLength(Long.parseLong(value));
}
@ -370,7 +401,7 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
@Override
public boolean containsHeader(String name)
{
if ("etag".equalsIgnoreCase(name) && _etag!=null)
if (!_noCompression && "etag".equalsIgnoreCase(name) && _etag!=null)
return true;
return super.containsHeader(name);
}
@ -385,10 +416,7 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
if (_compressedStream==null)
{
if (getResponse().isCommitted() || _noCompression)
{
setContentLength(_contentLength);
return getResponse().getOutputStream();
}
_compressedStream=newCompressedStream(_request,(HttpServletResponse)getResponse());
}
@ -411,10 +439,7 @@ public abstract class CompressedResponseWrapper extends HttpServletResponseWrapp
throw new IllegalStateException("getOutputStream() called");
if (getResponse().isCommitted() || _noCompression)
{
setContentLength(_contentLength);
return getResponse().getWriter();
}
_compressedStream=newCompressedStream(_request,(HttpServletResponse)getResponse());
_writer=newWriter(_compressedStream,getCharacterEncoding());

View File

@ -33,7 +33,10 @@ import org.eclipse.jetty.servlets.gzip.TestServletStreamLengthTypeWrite;
import org.eclipse.jetty.servlets.gzip.TestServletStreamTypeLengthWrite;
import org.eclipse.jetty.servlets.gzip.TestServletTypeLengthStreamWrite;
import org.eclipse.jetty.servlets.gzip.TestServletTypeStreamLengthWrite;
import org.eclipse.jetty.testing.HttpTester;
import org.eclipse.jetty.toolchain.test.TestingDir;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
@ -131,7 +134,8 @@ public class GzipFilterContentLengthTest
try
{
tester.start();
tester.assertIsResponseNotGzipCompressed("GET",testfile.getName(),filesize,HttpStatus.OK_200);
HttpTester response = tester.assertIsResponseNotGzipCompressed("GET",testfile.getName(),filesize,HttpStatus.OK_200);
Assert.assertThat(response.getHeader("ETAG"),Matchers.startsWith("w/etag-"));
}
finally
{
@ -139,6 +143,15 @@ public class GzipFilterContentLengthTest
}
}
/**
* Tests gzip compression of a small size file
*/
@Test
public void testEmpty() throws Exception
{
assertIsNotGzipCompressed("empty.txt",0);
}
/**
* Tests gzip compression of a small size file
*/

View File

@ -79,6 +79,7 @@ public class GzipFilterDefaultTest
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
resp.setStatus(_status);
resp.setHeader("ETag","W/\"204\"");
}
}
@ -141,11 +142,40 @@ public class GzipFilterDefaultTest
@Override
public void service(HttpServletRequest req, HttpServletResponse resp) throws IOException,ServletException
{
String uri=req.getRequestURI();
if (uri.endsWith(".deferred"))
{
System.err.println("type for "+uri.substring(0,uri.length()-9)+" is "+getServletContext().getMimeType(uri.substring(0,uri.length()-9)));
resp.setContentType(getServletContext().getMimeType(uri.substring(0,uri.length()-9)));
}
doGet(req,resp);
}
}
@Test
public void testIsGzipCompressedEmpty() throws Exception
{
GzipTester tester = new GzipTester(testingdir, compressionType);
// Test content that is smaller than the buffer.
tester.prepareServerFile("empty.txt",0);
FilterHolder holder = tester.setContentServlet(org.eclipse.jetty.servlet.DefaultServlet.class);
holder.setInitParameter("mimeTypes","text/plain");
try
{
tester.start();
HttpTester http = tester.assertIsResponseNotGzipCompressed("GET","empty.txt",0,200);
}
finally
{
tester.stop();
}
}
@Test
public void testIsGzipCompressedTiny() throws Exception
@ -242,7 +272,7 @@ public class GzipFilterDefaultTest
tester.stop();
}
}
@Test
public void testIsNotGzipCompressedWithQ() throws Exception
{
@ -267,7 +297,7 @@ public class GzipFilterDefaultTest
}
@Test
public void testIsNotGzipCompressed() throws Exception
public void testIsNotGzipCompressedByContentType() throws Exception
{
GzipTester tester = new GzipTester(testingdir, compressionType);
@ -289,6 +319,29 @@ public class GzipFilterDefaultTest
}
}
@Test
public void testIsNotGzipCompressedByDeferredContentType() throws Exception
{
GzipTester tester = new GzipTester(testingdir, compressionType);
int filesize = CompressedResponseWrapper.DEFAULT_BUFFER_SIZE * 4;
tester.prepareServerFile("file.mp3.deferred",filesize);
FilterHolder holder = tester.setContentServlet(GetServlet.class);
holder.setInitParameter("mimeTypes","text/plain");
try
{
tester.start();
HttpTester http = tester.assertIsResponseNotGzipCompressed("GET","file.mp3.deferred", filesize, HttpStatus.OK_200);
Assert.assertNull(http.getHeader("Vary"));
}
finally
{
tester.stop();
}
}
@Test
public void testIsNotGzipCompressedHttpStatus() throws Exception
{

View File

@ -54,6 +54,7 @@ import org.eclipse.jetty.testing.ServletTester;
import org.eclipse.jetty.toolchain.test.IO;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.TestingDir;
import org.hamcrest.Matchers;
import org.junit.Assert;
public class GzipTester
@ -108,6 +109,8 @@ public class GzipTester
else
Assert.assertThat("Response.header[Content-Encoding]", response.getHeader("Content-Encoding"),containsString(compressionType.substring(0,qindex)));
Assert.assertThat(response.getHeader("ETag"),Matchers.startsWith("W/"));
// Assert that the decompressed contents are what we expect.
File serverFile = testdir.getFile(serverFilename);
String expected = IO.readToString(serverFile);
@ -189,6 +192,8 @@ public class GzipTester
Assert.assertThat(prefix + ".header[Content-Type] (should have a Content-Type associated with it)",response.getHeader("Content-Type"),notNullValue());
Assert.assertThat(prefix + ".header[Content-Type]",response.getHeader("Content-Type"),is(expectedContentType));
Assert.assertThat(response.getHeader("ETAG"),Matchers.startsWith("w/etag-"));
ByteArrayInputStream bais = null;
DigestOutputStream digester = null;
try
@ -313,6 +318,10 @@ public class GzipTester
int serverLength = Integer.parseInt(response.getHeader("Content-Length"));
Assert.assertThat("Response.header[Content-Length]",serverLength,is(expectedFilesize));
}
if (status>=200 && status<300)
Assert.assertThat(response.getHeader("ETAG"),Matchers.startsWith("W/"));
}
private HttpTester executeRequest(String method,String uri) throws IOException, Exception
@ -345,11 +354,11 @@ public class GzipTester
ByteArrayOutputStream out = null;
try
{
in = new ByteArrayInputStream(response.getContentBytes());
out = new ByteArrayOutputStream();
IO.copy(in,out);
actual = out.toString(encoding);
byte[] content=response.getContentBytes();
if (content!=null)
actual=new String(response.getContentBytes(),encoding);
else
actual="";
}
finally
{
@ -432,7 +441,7 @@ public class GzipTester
finally
{
IO.close(in);
IO.close(fos);
IO.close(fos);
}
}
@ -464,6 +473,7 @@ public class GzipTester
servletTester.setResourceBase(testdir.getDir().getCanonicalPath());
ServletHolder servletHolder = servletTester.addServlet(servletClass,"/");
servletHolder.setInitParameter("baseDir",testdir.getDir().getAbsolutePath());
servletHolder.setInitParameter("etags","true");
FilterHolder holder = servletTester.addFilter(gzipFilterClass,"/*",0);
holder.setInitParameter("vary","Accept-Encoding");
return holder;

View File

@ -59,6 +59,7 @@ public class TestServletLengthStreamTypeWrite extends TestDirContentServlet
response.setContentType("text/plain");
else if (fileName.endsWith("mp3"))
response.setContentType("audio/mpeg");
response.setHeader("ETag","w/etag-"+fileName);
out.write(dataBytes);
}

View File

@ -57,6 +57,7 @@ public class TestServletLengthTypeStreamWrite extends TestDirContentServlet
response.setContentType("text/plain");
else if (fileName.endsWith("mp3"))
response.setContentType("audio/mpeg");
response.setHeader("ETag","w/etag-"+fileName);
ServletOutputStream out = response.getOutputStream();
out.write(dataBytes);

View File

@ -59,6 +59,7 @@ public class TestServletStreamLengthTypeWrite extends TestDirContentServlet
response.setContentType("text/plain");
else if (fileName.endsWith("mp3"))
response.setContentType("audio/mpeg");
response.setHeader("ETag","w/etag-"+fileName);
out.write(dataBytes);
}

View File

@ -57,6 +57,7 @@ public class TestServletStreamTypeLengthWrite extends TestDirContentServlet
response.setContentType("text/plain");
else if (fileName.endsWith("mp3"))
response.setContentType("audio/mpeg");
response.setHeader("ETag","w/etag-"+fileName);
response.setContentLength(dataBytes.length);

View File

@ -55,6 +55,7 @@ public class TestServletTypeLengthStreamWrite extends TestDirContentServlet
response.setContentType("text/plain");
else if (fileName.endsWith("mp3"))
response.setContentType("audio/mpeg");
response.setHeader("ETag","w/etag-"+fileName);
response.setContentLength(dataBytes.length);

View File

@ -55,6 +55,7 @@ public class TestServletTypeStreamLengthWrite extends TestDirContentServlet
response.setContentType("text/plain");
else if (fileName.endsWith("mp3"))
response.setContentType("audio/mpeg");
response.setHeader("ETag","w/etag-"+fileName);
ServletOutputStream out = response.getOutputStream();

View File

@ -599,4 +599,12 @@ public class HttpTester
}
@Override
public String toString()
{
if (_method!=null)
return super.toString()+" "+_method+" "+_uri+" "+_version+"\n"+_fields.toString();
return super.toString()+" HTTP/1.1 "+_status+" "+_reason+"\n"+_fields.toString();
}
}