From 9543255d5b0b1918df6c41e850ee47e7772d8e9d Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 18 Feb 2021 14:04:53 +0100 Subject: [PATCH] More optional etag gzip fixes for #5979 updates from review --- .../org/eclipse/jetty/http/CompressedContentFormat.java | 9 +++++++++ .../org/eclipse/jetty/http/PrecompressedHttpContent.java | 4 +++- .../eclipse/jetty/server/handler/ResourceHandler.java | 2 +- .../eclipse/jetty/server/handler/gzip/GzipHandler.java | 4 ++-- .../server/handler/gzip/GzipHttpOutputInterceptor.java | 2 +- .../java/org/eclipse/jetty/servlet/GzipHandlerTest.java | 4 ++-- .../jetty/server/handler/gzip/GzipDefaultTest.java | 2 +- 7 files changed, 19 insertions(+), 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 index 06daf4d3c716..ce0620d161c5 100644 --- 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 index f7c5aba16eaa..9820493e01c5 100644 --- 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 index ffd0c416b007..b5b0db05611a 100644 --- 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 index b0c5085726a1..bec51004312b 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 @@ -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/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java index a93dc0c45677..3ba0b36455c9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java +++ b/jetty-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 index c4efff70a5c4..32c8bc9a55e9 100644 --- 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 index 9703523b0205..c00a356422c2 100644 --- 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");