More optional etag gzip fixes for #5979

updates from review
jetty · Feb 18, 2021 · 9543255 · 9543255
1 parent 7d28b42
commit 9543255
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 8 deletions.
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java b/jetty-http/src/main/java/org/eclipse/jetty/http/CompressedContentFormat.java
@@ -82,6 +82,10 @@ public HttpField getContentEncoding()
         return _contentEncoding;
     }
 
+    /** Get an etag with suffix that represents this compressed type.
+     * @param etag An etag
+     * @return An etag with compression suffix, or the etag itself if no suffix is configured.
+     */
     public String etag(String etag)
     {
         if (StringUtil.isEmpty(ETAG_SEPARATOR))
@@ -98,6 +102,11 @@ public int hashCode()
         return Objects.hash(_encoding, _extension);
     }
 
+    /** Check etags for equality, accounting for quoting and compression suffixes.
+     * @param etag An etag without a compression suffix
+     * @param etagWithSuffix An etag optionally with a compression suffix.
+     * @return True if the tags are equal.
+     */
     public static boolean tagEquals(String etag, String etagWithSuffix)
     {
         // Handle simple equality

diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java b/jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java
@@ -167,7 +167,9 @@ public ReadableByteChannel getReadableByteChannel() throws IOException
     @Override
     public String toString()
     {
-        return String.format("PrecompressedHttpContent@%x{e=%s,r=%s|%s,lm=%s|%s,ct=%s}", hashCode(), _format,
+        return String.format("%s@%x{e=%s,r=%s|%s,lm=%s|%s,ct=%s}",
+            this.getClass().getSimpleName(), hashCode(),
+            _format,
             _content.getResource(), _precompressedContent.getResource(),
             _content.getResource().lastModified(), _precompressedContent.getResource().lastModified(),
             getContentType());

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java
@@ -307,7 +307,7 @@ public boolean isGzip()
     {
         for (CompressedContentFormat formats : _resourceService.getPrecompressedFormats())
         {
-            if (CompressedContentFormat.GZIP._encoding.equals(formats._encoding))
+            if (CompressedContentFormat.GZIP.getEncoding().equals(formats.getEncoding()))
                 return true;
         }
         return false;

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
@@ -152,6 +152,7 @@
  */
 public class GzipHandler extends HandlerWrapper implements GzipFactory
 {
+    public static final String GZIP_HANDLER_ETAGS = "o.e.j.s.h.gzip.GzipHandler.etag";
     public static final String GZIP = "gzip";
     public static final String DEFLATE = "deflate";
     public static final int DEFAULT_MIN_GZIP_SIZE = 32;
@@ -699,8 +700,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
                     if (!etagsNoSuffix.equals(etags))
                     {
                         fields.set(new HttpField(field.getHeader(), etagsNoSuffix));
-                        if (field.getHeader() == HttpHeader.IF_MATCH)
-                            baseRequest.setAttribute("o.e.j.s.h.gzip.GzipHandler.etag", etags);
+                        baseRequest.setAttribute(GZIP_HANDLER_ETAGS, etags);
                     }
                 }
             }

diff --git a/...server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java b/...server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java
@@ -153,7 +153,7 @@ protected void commit(ByteBuffer content, boolean complete, Callback callback)
 
             if (sc == HttpStatus.NOT_MODIFIED_304)
             {
-                String requestEtags = (String)_channel.getRequest().getAttribute("o.e.j.s.h.gzip.GzipHandler.etag");
+                String requestEtags = (String)_channel.getRequest().getAttribute(GzipHandler.GZIP_HANDLER_ETAGS);
                 String responseEtag = response.getHttpFields().get(HttpHeader.ETAG);
                 if (requestEtags != null && responseEtag != null)
                 {

diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java
@@ -88,7 +88,7 @@ public class GzipHandlerTest
     private static final String __micro = __content.substring(0, 10);
 
     private static final String __contentETag = String.format("W/\"%x\"", __content.hashCode());
-    private static final String __contentETagGzip = String.format("W/\"%x" + CompressedContentFormat.GZIP._etagExtension + "\"", __content.hashCode());
+    private static final String __contentETagGzip = String.format("W/\"%x" + CompressedContentFormat.GZIP.getEtagSuffix() + "\"", __content.hashCode());
     private static final String __icontent = "BEFORE" + __content + "AFTER";
 
     private Server _server;
@@ -592,7 +592,7 @@ public void testDeleteETagGzipHandler() throws Exception
         request.setURI("/ctx/content");
         request.setVersion("HTTP/1.0");
         request.setHeader("Host", "tester");
-        request.setHeader("If-Match", "WrongEtag" + CompressedContentFormat.GZIP._etagExtension);
+        request.setHeader("If-Match", "WrongEtag" + CompressedContentFormat.GZIP.getEtagSuffix());
         request.setHeader("accept-encoding", "gzip");
 
         response = HttpTester.parseResponse(_connector.getResponse(request.generate()));

diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipDefaultTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipDefaultTest.java
@@ -145,7 +145,7 @@ public void testIsGzipByMethod() throws Exception
             //A HEAD request should have similar headers, but no body
             response = tester.executeRequest("HEAD", "/context/file.txt", 5, TimeUnit.SECONDS);
             assertThat("Response status", response.getStatus(), is(HttpStatus.OK_200));
-            assertThat("ETag", response.get("ETag"), containsString(CompressedContentFormat.GZIP._etagExtension));
+            assertThat("ETag", response.get("ETag"), containsString(CompressedContentFormat.GZIP.getEtagSuffix()));
             assertThat("Content encoding", response.get("Content-Encoding"), containsString("gzip"));
             assertNull(response.get("Content-Length"), "Content length");