Skip to content

Commit

Permalink
Issue #623 Add gzip suffix to etags in 304 response
Browse files Browse the repository at this point in the history
  • Loading branch information
@gregw
gregw committed Jun 8, 2016
1 parent f86d726 commit f4c13e5
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 16 deletions.
2 changes: 2 additions & 0 deletions jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java
View file
Expand Up @@ -30,6 +30,7 @@

import org.eclipse.jetty.http.GzipHttpContent;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpVersion;
Expand Down Expand Up @@ -443,6 +444,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
int i=etag.indexOf(GzipHttpContent.ETAG_GZIP_QUOTE);
if (i>0)
{
baseRequest.setAttribute("o.e.j.s.h.gzip.GzipHandler.etag",etag);
while (i>=0)
{
etag=etag.substring(0,i)+etag.substring(i+GzipHttpContent.ETAG_GZIP.length());
Expand Down
25 changes: 20 additions & 5 deletions ...server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java
View file
Expand Up @@ -157,6 +157,19 @@ protected void commit(ByteBuffer content, boolean complete, Callback callback)
{
LOG.debug("{} exclude by status {}",this,sc);
noCompression();

if (sc==304)
{
String request_etags = (String)_channel.getRequest().getAttribute("o.e.j.s.h.gzip.GzipHandler.etag");
String response_etag = response.getHttpFields().get(HttpHeader.ETAG);
if (request_etags!=null && response_etag!=null)
{
String response_etag_gzip=etagGzip(response_etag);
if (request_etags.contains(response_etag_gzip))
response.getHttpFields().put(HttpHeader.ETAG,response_etag_gzip);
}
}

_interceptor.write(content, complete, callback);
return;
}
Expand Down Expand Up @@ -216,11 +229,7 @@ protected void commit(ByteBuffer content, boolean complete, Callback callback)
response.setContentLength(-1);
String etag=fields.get(HttpHeader.ETAG);
if (etag!=null)
{
int end = etag.length()-1;
etag=(etag.charAt(end)=='"')?etag.substring(0,end)+GzipHttpContent.ETAG_GZIP+'"':etag+GzipHttpContent.ETAG_GZIP;
fields.put(HttpHeader.ETAG,etag);
}
fields.put(HttpHeader.ETAG,etagGzip(etag));

LOG.debug("{} compressing {}",this,_deflater);
_state.set(GZState.COMPRESSING);
Expand All @@ -231,6 +240,12 @@ protected void commit(ByteBuffer content, boolean complete, Callback callback)
callback.failed(new WritePendingException());
}

private String etagGzip(String etag)
{
int end = etag.length()-1;
return (etag.charAt(end)=='"')?etag.substring(0,end)+GzipHttpContent.ETAG_GZIP+'"':etag+GzipHttpContent.ETAG_GZIP;
}

public void noCompression()
{
while (true)
Expand Down
111 changes: 100 additions & 11 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java
View file
Expand Up @@ -19,10 +19,14 @@
package org.eclipse.jetty.servlet;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalToIgnoringCase;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
Expand All @@ -42,6 +46,7 @@
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.util.IO;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -62,6 +67,8 @@ public class GzipHandlerTest
"Aliquam purus mauris, consectetur nec convallis lacinia, porta sed ante. Suspendisse "+
"et cursus magna. Donec orci enim, molestie a lobortis eu, imperdiet vitae neque.";

private static String __contentETag = String.format("W/\"%x\"",__content.hashCode());
private static String __contentETagGzip = String.format("W/\"%x--gzip\"",__content.hashCode());
private static String __icontent = "BEFORE"+__content+"AFTER";

private Server _server;
Expand All @@ -75,6 +82,7 @@ public void init() throws Exception
_server.addConnector(_connector);

GzipHandler gzipHandler = new GzipHandler();
gzipHandler.setExcludedAgentPatterns();

ServletContextHandler context = new ServletContextHandler(gzipHandler,"/ctx");
ServletHandler servlets = context.getServletHandler();
Expand All @@ -93,8 +101,15 @@ public static class TestServlet extends HttpServlet
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
{
PrintWriter writer = response.getWriter();
writer.write(__content);
response.setHeader("ETag",__contentETag);
String ifnm = req.getHeader("If-None-Match");
if (ifnm!=null && ifnm.equals(__contentETag))
response.sendError(304);
else
{
PrintWriter writer = response.getWriter();
writer.write(__content);
}
}
}

Expand Down Expand Up @@ -125,6 +140,33 @@ public void destroy() throws Exception
_server.join();
}

@Test
public void testNotGzipHandler() throws Exception
{
// generated and parsed test
HttpTester.Request request = HttpTester.newRequest();
HttpTester.Response response;

request.setMethod("GET");
request.setURI("/ctx/content");
request.setVersion("HTTP/1.0");
request.setHeader("Host","tester");

response = HttpTester.parseResponse(_connector.getResponses(request.generate()));

assertThat(response.getStatus(),is(200));
assertThat(response.get("Content-Encoding"),not(equalToIgnoringCase("gzip")));
assertThat(response.get("ETag"),is(__contentETag));
assertThat(response.get("Vary"),is("Accept-Encoding"));

InputStream testIn = new ByteArrayInputStream(response.getContentBytes());
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn,testOut);

assertEquals(__content, testOut.toString("UTF8"));

}

@Test
public void testGzipHandler() throws Exception
{
Expand All @@ -133,22 +175,65 @@ public void testGzipHandler() throws Exception
HttpTester.Response response;

request.setMethod("GET");
request.setURI("/ctx/content");
request.setVersion("HTTP/1.0");
request.setHeader("Host","tester");
request.setHeader("accept-encoding","gzip");
request.setURI("/ctx/content");

response = HttpTester.parseResponse(_connector.getResponses(request.generate()));

assertTrue(response.get("Content-Encoding").equalsIgnoreCase("gzip"));
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
assertThat(response.getStatus(),is(200));
assertThat(response.get("Content-Encoding"),Matchers.equalToIgnoringCase("gzip"));
assertThat(response.get("ETag"),is(__contentETagGzip));
assertThat(response.get("Vary"),is("Accept-Encoding"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
IO.copy(testIn,testOut);

assertEquals(__content, testOut.toString("UTF8"));
}

@Test
public void testETagNotGzipHandler() throws Exception
{
// generated and parsed test
HttpTester.Request request = HttpTester.newRequest();
HttpTester.Response response;

request.setMethod("GET");
request.setURI("/ctx/content");
request.setVersion("HTTP/1.0");
request.setHeader("Host","tester");
request.setHeader("If-None-Match",__contentETag);
request.setHeader("accept-encoding","gzip");

response = HttpTester.parseResponse(_connector.getResponses(request.generate()));

assertThat(response.getStatus(),is(304));
assertThat(response.get("Content-Encoding"),not(Matchers.equalToIgnoringCase("gzip")));
assertThat(response.get("ETag"),is(__contentETag));
}

@Test
public void testETagGzipHandler() throws Exception
{
// generated and parsed test
HttpTester.Request request = HttpTester.newRequest();
HttpTester.Response response;

request.setMethod("GET");
request.setURI("/ctx/content");
request.setVersion("HTTP/1.0");
request.setHeader("Host","tester");
request.setHeader("If-None-Match",__contentETagGzip);
request.setHeader("accept-encoding","gzip");

response = HttpTester.parseResponse(_connector.getResponses(request.generate()));

assertThat(response.getStatus(),is(304));
assertThat(response.get("Content-Encoding"),not(Matchers.equalToIgnoringCase("gzip")));
assertThat(response.get("ETag"),is(__contentETagGzip));
}

@Test
Expand All @@ -166,8 +251,10 @@ public void testForwardGzipHandler() throws Exception

response = HttpTester.parseResponse(_connector.getResponses(request.generate()));

assertTrue(response.get("Content-Encoding").equalsIgnoreCase("gzip"));
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
assertThat(response.getStatus(),is(200));
assertThat(response.get("Content-Encoding"),Matchers.equalToIgnoringCase("gzip"));
assertThat(response.get("ETag"),is(__contentETagGzip));
assertThat(response.get("Vary"),is("Accept-Encoding"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
Expand All @@ -190,9 +277,11 @@ public void testIncludeGzipHandler() throws Exception
request.setURI("/ctx/include");

response = HttpTester.parseResponse(_connector.getResponses(request.generate()));

assertTrue(response.get("Content-Encoding").equalsIgnoreCase("gzip"));
assertEquals(HttpServletResponse.SC_OK,response.getStatus());

assertThat(response.getStatus(),is(200));
assertThat(response.get("Content-Encoding"),Matchers.equalToIgnoringCase("gzip"));
assertThat(response.get("ETag"),nullValue());
assertThat(response.get("Vary"),is("Accept-Encoding"));

InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes()));
ByteArrayOutputStream testOut = new ByteArrayOutputStream();
Expand Down

0 comments on commit f4c13e5

Please sign in to comment.