From 99f4ed7352504072753e14615fe0e3471a00824b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 18 Sep 2015 12:30:02 +1000 Subject: [PATCH] 477737 Improve handling of etags with dynamic and static gzip --- .../jetty/embedded/GzipHandlerTest.java | 129 ++++++++++++++++-- .../eclipse/jetty/http/GzipHttpContent.java | 11 ++ .../server/handler/gzip/GzipHandler.java | 13 +- .../eclipse/jetty/servlet/DefaultServlet.java | 28 ++-- .../jetty/servlet/DefaultServletTest.java | 42 ++++++ 5 files changed, 192 insertions(+), 31 deletions(-) diff --git a/examples/embedded/src/test/java/org/eclipse/jetty/embedded/GzipHandlerTest.java b/examples/embedded/src/test/java/org/eclipse/jetty/embedded/GzipHandlerTest.java index 0edc528b176..c75586eb6e6 100644 --- a/examples/embedded/src/test/java/org/eclipse/jetty/embedded/GzipHandlerTest.java +++ b/examples/embedded/src/test/java/org/eclipse/jetty/embedded/GzipHandlerTest.java @@ -32,6 +32,9 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.GzipHttpContent; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.LocalConnector; @@ -42,6 +45,7 @@ import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.util.IO; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -75,13 +79,25 @@ public class GzipHandlerTest Handler testHandler = new AbstractHandler() { + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { - PrintWriter writer = response.getWriter(); - writer.write(__content); - writer.close(); - + String etag="W/\"foobar\""; + response.setHeader(HttpHeader.ETAG.asString(),etag); + + String inm = request.getHeader(HttpHeader.IF_NONE_MATCH.asString()); + if (inm!=null && inm.contains(etag)) + { + response.setStatus(HttpStatus.NOT_MODIFIED_304); + } + else + { + PrintWriter writer = response.getWriter(); + writer.write(__content); + writer.close(); + } + baseRequest.setHandled(true); } }; @@ -100,20 +116,56 @@ public class GzipHandlerTest _server.join(); } + HttpTester.Request newRequest() + { + HttpTester.Request request = HttpTester.newRequest(); + request.setMethod("GET"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host","tester"); + request.setURI("/"); + return request; + } + + @Test + public void testNoGzipHandler() throws Exception + { + // generated and parsed test + HttpTester.Request request = newRequest(); + request.setHeader("accept-encoding","deflate"); + + HttpTester.Response response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + Assert.assertNull(response.get("Content-Encoding")); + assertEquals(__content, response.getContent()); + } + @Test public void testGzipHandler() throws Exception { // generated and parsed test - HttpTester.Request request = HttpTester.newRequest(); - HttpTester.Response response; - - request.setMethod("GET"); - request.setVersion("HTTP/1.0"); - request.setHeader("Host","tester"); + HttpTester.Request request = newRequest(); request.setHeader("accept-encoding","gzip"); - request.setURI("/"); - response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + HttpTester.Response response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + assertTrue(response.get("Content-Encoding").equalsIgnoreCase("gzip")); + InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes())); + ByteArrayOutputStream testOut = new ByteArrayOutputStream(); + IO.copy(testIn,testOut); + assertEquals(__content, testOut.toString("UTF8")); + + } + + @Test + public void testDeflateGzipHandler() throws Exception + { + // generated and parsed test + HttpTester.Request request = newRequest(); + request.setHeader("accept-encoding","deflate;q=0.1,gzip;q=0.5"); + + HttpTester.Response response = HttpTester.parseResponse(_connector.getResponses(request.generate())); assertTrue(response.get("Content-Encoding").equalsIgnoreCase("gzip")); assertEquals(HttpServletResponse.SC_OK,response.getStatus()); @@ -123,6 +175,59 @@ public class GzipHandlerTest IO.copy(testIn,testOut); assertEquals(__content, testOut.toString("UTF8")); + } + + + @Test + public void testEtagGzipHandler() throws Exception + { + // generated and parsed test + HttpTester.Request request = newRequest(); + HttpTester.Response response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + Assert.assertNull(response.get("Content-Encoding")); + assertEquals(__content, response.getContent()); + String etag = response.get(HttpHeader.ETAG.asString()); + String etag_gzip=etag.substring(0,etag.length()-1)+GzipHttpContent.ETAG_GZIP_QUOTE; + + request = newRequest(); + request.add(HttpHeader.IF_NONE_MATCH,"xxxx"); + response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + Assert.assertNull(response.get("Content-Encoding")); + assertEquals(__content, response.getContent()); + + request = newRequest(); + request.add(HttpHeader.IF_NONE_MATCH,etag); + response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + assertEquals(HttpServletResponse.SC_NOT_MODIFIED,response.getStatus()); + assertEquals(etag,response.get(HttpHeader.ETAG.asString())); + + request = newRequest(); + request.setHeader("accept-encoding","gzip"); + request.add(HttpHeader.IF_NONE_MATCH,etag); + response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + assertEquals(HttpServletResponse.SC_NOT_MODIFIED,response.getStatus()); + assertEquals(etag,response.get(HttpHeader.ETAG.asString())); + + request = newRequest(); + request.setHeader("accept-encoding","gzip"); + response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + assertTrue(response.get("Content-Encoding").equalsIgnoreCase("gzip")); + InputStream testIn = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes())); + ByteArrayOutputStream testOut = new ByteArrayOutputStream(); + IO.copy(testIn,testOut); + assertEquals(__content, testOut.toString("UTF8")); + assertEquals(etag_gzip,response.get(HttpHeader.ETAG.asString())); + + request = newRequest(); + request.setHeader("accept-encoding","gzip"); + request.add(HttpHeader.IF_NONE_MATCH,etag_gzip); + response = HttpTester.parseResponse(_connector.getResponses(request.generate())); + assertEquals(HttpServletResponse.SC_NOT_MODIFIED,response.getStatus()); + assertEquals(etag,response.get(HttpHeader.ETAG.asString())); + } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/GzipHttpContent.java b/jetty-http/src/main/java/org/eclipse/jetty/http/GzipHttpContent.java index ff70f3777f6..1354358def8 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/GzipHttpContent.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/GzipHttpContent.java @@ -32,8 +32,19 @@ public class GzipHttpContent implements HttpContent private final HttpContent _content; private final HttpContent _contentGz; public final static String ETAG_GZIP="--gzip"; + public final static String ETAG_GZIP_QUOTE="--gzip\""; public final static PreEncodedHttpField CONTENT_ENCODING_GZIP=new PreEncodedHttpField(HttpHeader.CONTENT_ENCODING,"gzip"); + public static String removeGzipFromETag(String etag) + { + if (etag==null) + return null; + int i = etag.indexOf(ETAG_GZIP_QUOTE); + if (i<0) + return etag; + return etag.substring(0,i)+'"'; + } + public GzipHttpContent(HttpContent content, HttpContent contentGz) { _content=content; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java index 1c0198bd19c..688ed4cebc6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.server.handler.gzip; +import static org.eclipse.jetty.http.GzipHttpContent.ETAG_GZIP_QUOTE; + import java.io.File; import java.io.IOException; import java.util.Set; @@ -62,7 +64,6 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory public final static String GZIP = "gzip"; public final static String DEFLATE = "deflate"; - public final static String ETAG = "o.e.j.s.Gzip.ETag"; public final static int DEFAULT_MIN_GZIP_SIZE=16; private int _minGzipSize=DEFAULT_MIN_GZIP_SIZE; private int _compressionLevel=Deflater.DEFAULT_COMPRESSION; @@ -415,11 +416,15 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory } // Special handling for etags - String etag = request.getHeader("If-None-Match"); + String etag = baseRequest.getHttpFields().get(HttpHeader.IF_NONE_MATCH); if (etag!=null) { - if (etag.contains(GzipHttpContent.ETAG_GZIP)) - request.setAttribute(ETAG,etag.replace(GzipHttpContent.ETAG_GZIP,"")); + int i=etag.indexOf(ETAG_GZIP_QUOTE); + while (i>=0) + { + baseRequest.getHttpFields().put(new HttpField(HttpHeader.ETAG,etag.substring(0,i)+etag.substring(i+GzipHttpContent.ETAG_GZIP.length()))); + i=etag.indexOf(ETAG_GZIP_QUOTE,i); + } } // install interceptor and handle diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java index 8041eb0687c..be9e81423fc 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java @@ -18,6 +18,9 @@ package org.eclipse.jetty.servlet; +import static org.eclipse.jetty.http.GzipHttpContent.ETAG_GZIP_QUOTE; +import static org.eclipse.jetty.http.GzipHttpContent.removeGzipFromETag; + import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -40,6 +43,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.DateParser; +import org.eclipse.jetty.http.GzipHttpContent; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; @@ -693,6 +697,7 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory if (request instanceof Request) { + // Find multiple fields by iteration as an optimization HttpFields fields = ((Request)request).getHttpFields(); for (int i=fields.size();i-->0;) { @@ -730,16 +735,17 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory { if (_etags) { + String etag=content.getETagValue(); if (ifm!=null) { boolean match=false; - if (content.getETagValue()!=null) + if (etag!=null) { QuotedStringTokenizer quoted = new QuotedStringTokenizer(ifm,", ",false,true); while (!match && quoted.hasMoreTokens()) { String tag = quoted.nextToken(); - if (content.getETagValue().equals(tag)) + if (etag.equals(tag) || tag.endsWith(ETAG_GZIP_QUOTE) && etag.equals(removeGzipFromETag(tag))) match=true; } } @@ -751,33 +757,25 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory } } - if (ifnm!=null && content.getETagValue()!=null) + if (ifnm!=null && etag!=null) { - // Look for Gzip'd version of etag - if (content.getETagValue().equals(request.getAttribute("o.e.j.s.Gzip.ETag"))) + // Handle special case of exact match OR gzip exact match + if (etag.equals(ifnm) || ifnm.endsWith(ETAG_GZIP_QUOTE) && ifnm.indexOf(',')<0 && etag.equals(removeGzipFromETag(etag))) { response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); response.setHeader(HttpHeader.ETAG.asString(),ifnm); return false; } - // Handle special case of exact match. - if (content.getETagValue().equals(ifnm)) - { - response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); - response.setHeader(HttpHeader.ETAG.asString(),content.getETagValue()); - return false; - } - // Handle list of tags QuotedStringTokenizer quoted = new QuotedStringTokenizer(ifnm,", ",false,true); while (quoted.hasMoreTokens()) { String tag = quoted.nextToken(); - if (content.getETagValue().equals(tag)) + if (etag.equals(tag) || tag.endsWith(ETAG_GZIP_QUOTE) && etag.equals(removeGzipFromETag(tag))) { response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); - response.setHeader(HttpHeader.ETAG.asString(),content.getETagValue()); + response.setHeader(HttpHeader.ETAG.asString(),tag); return false; } } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java index 8a5c3f2ca0e..174c85f87d0 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java @@ -731,6 +731,32 @@ public class DefaultServletTest assertResponseNotContains("Content-Encoding: gzip",response); assertResponseNotContains("ETag: "+etag_gzip,response); assertResponseContains("ETag: ",response); + + response = connector.getResponses("GET /context/data0.txt.gz HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: W/\"wobble\"\r\n\r\n"); + assertResponseContains("Content-Length: 9", response); + assertResponseContains("fake gzip",response); + assertResponseContains("Content-Type: application/gzip",response); + assertResponseNotContains("Vary: Accept-Encoding",response); + assertResponseNotContains("Content-Encoding: gzip",response); + assertResponseNotContains("ETag: "+etag_gzip,response); + assertResponseContains("ETag: ",response); + + response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: "+etag_gzip+"\r\n\r\n"); + assertResponseContains("304 Not Modified", response); + assertResponseContains("ETag: "+etag_gzip,response); + + response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: "+etag+"\r\n\r\n"); + assertResponseContains("304 Not Modified", response); + assertResponseContains("ETag: "+etag,response); + + response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: W/\"foobar\","+etag_gzip+"\r\n\r\n"); + assertResponseContains("304 Not Modified", response); + assertResponseContains("ETag: "+etag_gzip,response); + + response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: W/\"foobar\","+etag+"\r\n\r\n"); + assertResponseContains("304 Not Modified", response); + assertResponseContains("ETag: "+etag,response); + } @Test @@ -784,6 +810,22 @@ public class DefaultServletTest assertResponseNotContains("Content-Encoding: gzip",response); assertResponseNotContains("ETag: "+etag_gzip,response); assertResponseContains("ETag: ",response); + + response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: "+etag_gzip+"\r\n\r\n"); + assertResponseContains("304 Not Modified", response); + assertResponseContains("ETag: "+etag_gzip,response); + + response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: "+etag+"\r\n\r\n"); + assertResponseContains("304 Not Modified", response); + assertResponseContains("ETag: "+etag,response); + + response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: W/\"foobar\","+etag_gzip+"\r\n\r\n"); + assertResponseContains("304 Not Modified", response); + assertResponseContains("ETag: "+etag_gzip,response); + + response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\nHost:localhost:8080\r\nAccept-Encoding:gzip\r\nIf-None-Match: W/\"foobar\","+etag+"\r\n\r\n"); + assertResponseContains("304 Not Modified", response); + assertResponseContains("ETag: "+etag,response); } @Test