From a8b4927427fc37ee547e50b6cd79a8fd7556ec8b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 17 Feb 2021 22:19:23 +0100 Subject: [PATCH 1/3] Fix #5979 by allowing a configurable etag separator. (#5980) * Fix #5979 by allowing a configurable etag separator. Fix #5979 by allowing a configurable etag separator * updates from review * Updates from review Signed-off-by: Greg Wilkins --- .../jetty/http/CompressedContentFormat.java | 38 +++++++++++++------ .../server/handler/gzip/GzipHandler.java | 1 + .../jetty/servlet/GzipHandlerTest.java | 5 ++- 3 files changed, 30 insertions(+), 14 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 399eab97a96..1fccb00ad87 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 @@ -18,8 +18,21 @@ package org.eclipse.jetty.http; +import java.util.Objects; + +import org.eclipse.jetty.util.StringUtil; + public class CompressedContentFormat { + /** + * The separator within an etag used to indicate a compressed variant. By default the separator is "--" + * So etag for compressed resource that normally has an etag of W/"28c772d6" + * is W/"28c772d6--gzip". The separator may be changed by the + * "org.eclipse.jetty.http.CompressedContentFormat.ETAG_SEPARATOR" System property. If changed, it should be changed to a string + * that will not be found in a normal etag or at least is very unlikely to be a substring of a normal etag. + */ + public static final String ETAG_SEPARATOR = System.getProperty(CompressedContentFormat.class.getName() + ".ETAG_SEPARATOR", "--"); + public static final CompressedContentFormat GZIP = new CompressedContentFormat("gzip", ".gz"); public static final CompressedContentFormat BR = new CompressedContentFormat("br", ".br"); public static final CompressedContentFormat[] NONE = new CompressedContentFormat[0]; @@ -32,11 +45,11 @@ public class CompressedContentFormat public CompressedContentFormat(String encoding, String extension) { - _encoding = encoding; - _extension = extension; - _etag = "--" + encoding; + _encoding = StringUtil.asciiToLowerCase(encoding); + _extension = StringUtil.asciiToLowerCase(extension); + _etag = ETAG_SEPARATOR + _encoding; _etagQuote = _etag + "\""; - _contentEncoding = new PreEncodedHttpField(HttpHeader.CONTENT_ENCODING, encoding); + _contentEncoding = new PreEncodedHttpField(HttpHeader.CONTENT_ENCODING, _encoding); } @Override @@ -45,12 +58,13 @@ public class CompressedContentFormat if (!(o instanceof CompressedContentFormat)) return false; CompressedContentFormat ccf = (CompressedContentFormat)o; - if (_encoding == null && ccf._encoding != null) - return false; - if (_extension == null && ccf._extension != null) - return false; + return Objects.equals(_encoding, ccf._encoding) && Objects.equals(_extension, ccf._extension); + } - return _encoding.equalsIgnoreCase(ccf._encoding) && _extension.equalsIgnoreCase(ccf._extension); + @Override + public int hashCode() + { + return Objects.hash(_encoding, _extension); } public static boolean tagEquals(String etag, String tag) @@ -58,9 +72,9 @@ public class CompressedContentFormat if (etag.equals(tag)) return true; - int dashdash = tag.indexOf("--"); - if (dashdash > 0 && dashdash == etag.length() - 1) - return etag.regionMatches(0, tag, 0, dashdash); + int separator = tag.lastIndexOf(ETAG_SEPARATOR); + if (separator > 0 && separator == etag.length() - 1) + return etag.regionMatches(0, tag, 0, separator); 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 e443d474e73..71aa69421ad 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 @@ -123,6 +123,7 @@ import org.eclipse.jetty.util.log.Logger; * If a ETag is present in the Response headers, and GzipHandler is compressing the * contents, it will add the {@code --gzip} suffix before the Response headers are committed * and sent to the User Agent. + * Note that the suffix used is determined by {@link CompressedContentFormat#ETAG_SEPARATOR} *

*

* This implementation relies on an Jetty internal {@link org.eclipse.jetty.server.HttpOutput.Interceptor} 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 4f92ef36952..ff831315414 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 @@ -44,6 +44,7 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.server.HttpOutput; import org.eclipse.jetty.server.LocalConnector; @@ -87,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--gzip\"", __content.hashCode()); + private static final String __contentETagGzip = String.format("W/\"%x" + CompressedContentFormat.GZIP._etag + "\"", __content.hashCode()); private static final String __icontent = "BEFORE" + __content + "AFTER"; private Server _server; @@ -591,7 +592,7 @@ public class GzipHandlerTest request.setURI("/ctx/content"); request.setVersion("HTTP/1.0"); request.setHeader("Host", "tester"); - request.setHeader("If-Match", "WrongEtag--gzip"); + request.setHeader("If-Match", "WrongEtag" + CompressedContentFormat.GZIP._etag); request.setHeader("accept-encoding", "gzip"); response = HttpTester.parseResponse(_connector.getResponse(request.generate())); From addfbe81c14ac59a5a071d667362f7760aa4a277 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Thu, 18 Feb 2021 07:29:24 +1000 Subject: [PATCH 2/3] Issue #5977 do not force Cache-Control header if already set (#5978) * Issue #5977 do not force Cache-Control header if already set Signed-off-by: olivier lamy --- .../eclipse/jetty/server/ResourceService.java | 8 +- .../eclipse/jetty/servlet/DefaultServlet.java | 2 +- .../jetty/servlet/CacheControlHeaderTest.java | 157 ++++++++++++++++++ 3 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 jetty-servlet/src/test/java/org/eclipse/jetty/servlet/CacheControlHeaderTest.java diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index c3866cd5f01..d9b980efc8d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -829,19 +829,19 @@ public class ResourceService Response r = (Response)response; r.putHeaders(content, contentLength, _etags); HttpFields f = r.getHttpFields(); - if (_acceptRanges) + if (_acceptRanges && !response.containsHeader(HttpHeader.ACCEPT_RANGES.asString())) f.put(ACCEPT_RANGES); - if (_cacheControl != null) + if (_cacheControl != null && !response.containsHeader(HttpHeader.CACHE_CONTROL.asString())) f.put(_cacheControl); } else { Response.putHeaders(response, content, contentLength, _etags); - if (_acceptRanges) + if (_acceptRanges && !response.containsHeader(HttpHeader.ACCEPT_RANGES.name())) response.setHeader(ACCEPT_RANGES.getName(), ACCEPT_RANGES.getValue()); - if (_cacheControl != null) + if (_cacheControl != null && !response.containsHeader(HttpHeader.CACHE_CONTROL.name())) response.setHeader(_cacheControl.getName(), _cacheControl.getValue()); } } 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 1aaf297215d..101a292a86f 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 @@ -283,7 +283,7 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory, Welc _resourceService.setContentFactory(contentFactory); _resourceService.setWelcomeFactory(this); - List gzipEquivalentFileExtensions = new ArrayList(); + List gzipEquivalentFileExtensions = new ArrayList<>(); String otherGzipExtensions = getInitParameter("otherGzipFileExtensions"); if (otherGzipExtensions != null) { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/CacheControlHeaderTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/CacheControlHeaderTest.java new file mode 100644 index 00000000000..72eb82a5ea6 --- /dev/null +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/CacheControlHeaderTest.java @@ -0,0 +1,157 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.servlet; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.EnumSet; +import javax.servlet.DispatcherType; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; + +public class CacheControlHeaderTest +{ + private Server server; + private LocalConnector connector; + + public static class ForceCacheControlFilter implements Filter + { + @Override + public void init(FilterConfig filterConfig) throws ServletException + { + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException + { + HttpServletResponse httpResponse = (HttpServletResponse)response; + httpResponse.setHeader(HttpHeader.CACHE_CONTROL.asString(), "max-age=0,private"); + chain.doFilter(request, response); + } + + @Override + public void destroy() + { + } + } + + public void startServer(boolean forceFilter) throws Exception + { + server = new Server(); + + HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory(new HttpConfiguration()); + connector = new LocalConnector(server, null, null, null, -1, httpConnectionFactory); + + ServletContextHandler context = new ServletContextHandler(); + + ServletHolder servletHolder = new ServletHolder(); + servletHolder.setServlet(new DefaultServlet()); + servletHolder.setInitParameter("cacheControl", "max-age=3600,public"); + Path resBase = MavenTestingUtils.getTestResourcePathDir("contextResources"); + servletHolder.setInitParameter("resourceBase", resBase.toFile().toURI().toASCIIString()); + context.addServlet(servletHolder, "/*"); + if (forceFilter) + { + context.addFilter(ForceCacheControlFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST)); + } + server.setHandler(context); + server.addConnector(connector); + + server.start(); + } + + public void stopServer() throws Exception + { + if (server != null && server.isRunning()) + { + server.stop(); + } + } + + @Test + public void testCacheControlFilterOverride() throws Exception + { + try + { + startServer(true); + StringBuffer req1 = new StringBuffer(); + req1.append("GET /content.txt HTTP/1.1\r\n"); + req1.append("Host: local\r\n"); + req1.append("Accept: */*\r\n"); + req1.append("Connection: close\r\n"); + req1.append("\r\n"); + + String response = connector.getResponse(req1.toString()); + assertThat("Response status", + response, + containsString("HTTP/1.1 200 OK")); + assertThat("Response headers", + response, + containsString(HttpHeader.CACHE_CONTROL.asString() + ": max-age=0,private")); + } + finally + { + stopServer(); + } + } + + @Test + public void testCacheControlDefaultServlet() throws Exception + { + try + { + startServer(false); + StringBuffer req1 = new StringBuffer(); + req1.append("GET /content.txt HTTP/1.1\r\n"); + req1.append("Host: local\r\n"); + req1.append("Accept: */*\r\n"); + req1.append("Connection: close\r\n"); + req1.append("\r\n"); + + String response = connector.getResponse(req1.toString()); + assertThat("Response status", + response, + containsString("HTTP/1.1 200 OK")); + assertThat("Response headers", + response, + containsString(HttpHeader.CACHE_CONTROL.asString() + ": max-age=3600,public")); + } + finally + { + stopServer(); + } + } + +} From 5857496ef29541743a35121c7e9e588bd087b33b Mon Sep 17 00:00:00 2001 From: olivier lamy Date: Thu, 18 Feb 2021 09:50:14 +1000 Subject: [PATCH 3/3] fix header Signed-off-by: olivier lamy --- .../jetty/servlet/CacheControlHeaderTest.java | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/CacheControlHeaderTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/CacheControlHeaderTest.java index 72eb82a5ea6..eba0b42b93d 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/CacheControlHeaderTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/CacheControlHeaderTest.java @@ -1,19 +1,14 @@ // -// ======================================================================== -// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. // -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. // -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== // package org.eclipse.jetty.servlet;