From f78f4422f5913820bead90cfa7a337415ff8935d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 25 Jun 2024 08:11:35 -0500 Subject: [PATCH] Issue #11925 - Fix Etag NPE when using URLResource and improve Base Resource is alias warning (#11930) * Issue #11925 - ee9 DefaultServlet and suffix url-patterns. * Issue #11925 - Fix NPE in EtagUtils with URLResource * Issue #11925 - Make error message "Base Resource should not be an alias" more useful. * Set to false for problematic tests. --- .../org/eclipse/jetty/http/EtagUtils.java | 13 ++-- .../eclipse/jetty/maven/MavenResource.java | 6 ++ .../jetty/maven/SelectiveJarResource.java | 6 ++ .../jetty/server/handler/ContextHandler.java | 9 ++- .../jetty/util/resource/MemoryResource.java | 2 +- .../eclipse/jetty/util/resource/Resource.java | 9 +++ .../util/resource/URLResourceFactory.java | 2 +- jetty-ee8/jetty-ee8-servlet/pom.xml | 2 + jetty-ee9/jetty-ee9-servlet/pom.xml | 2 + .../jetty/ee9/servlet/DefaultServletTest.java | 74 +++++++++++++++++++ .../jetty/ee9/servlet/DispatcherTest.java | 9 +++ 11 files changed, 124 insertions(+), 10 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/EtagUtils.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/EtagUtils.java index 2effc40960cc..9221cef39995 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/EtagUtils.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/EtagUtils.java @@ -66,13 +66,9 @@ public static HttpField createWeakEtagField(Resource resource) */ public static HttpField createWeakEtagField(Resource resource, String etagSuffix) { - Path path = resource.getPath(); - if (path == null) - return null; - - String etagValue = EtagUtils.computeWeakEtag(path, etagSuffix); - if (etagValue != null) - return new PreEncodedHttpField(HttpHeader.ETAG, etagValue); + String etag = EtagUtils.computeWeakEtag(resource, etagSuffix); + if (etag != null) + return new PreEncodedHttpField(HttpHeader.ETAG, etag); return null; } @@ -213,6 +209,9 @@ private static String computeWeakEtag(String name, Instant lastModified, long si */ public static String rewriteWithSuffix(String etag, String newSuffix) { + if (etag == null) + return null; + StringBuilder ret = new StringBuilder(); boolean weak = etag.startsWith("W/"); int start = 0; diff --git a/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/MavenResource.java b/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/MavenResource.java index 42475de50fda..aefe9e1d134a 100644 --- a/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/MavenResource.java +++ b/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/MavenResource.java @@ -217,4 +217,10 @@ public Resource resolve(String subUriPath) return null; return _resource.resolve(subUriPath); } + + @Override + public String toString() + { + return "(Maven) " + _resource.toString(); + } } diff --git a/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/SelectiveJarResource.java b/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/SelectiveJarResource.java index 8e8c44f983e6..22079cc47b1c 100644 --- a/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/SelectiveJarResource.java +++ b/jetty-core/jetty-maven/src/main/java/org/eclipse/jetty/maven/SelectiveJarResource.java @@ -277,4 +277,10 @@ public void copyTo(Path directory) throws IOException } } } + + @Override + public String toString() + { + return "(Selective Jar/Maven) " + _delegate.toString(); + } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 84ef89a15e36..1a237c236ada 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -715,7 +715,14 @@ protected void doStart() throws Exception if (!Resources.isReadable(baseResource)) throw new IllegalArgumentException("Base Resource is not valid: " + baseResource); if (baseResource.isAlias()) - LOG.warn("Base Resource should not be an alias"); + { + URI realUri = baseResource.getRealURI(); + if (realUri == null) + LOG.warn("Base Resource should not be an alias (100% of requests to context are subject to Security/Alias Checks): {}", baseResource); + else + LOG.warn("Base Resource should not be an alias (100% of requests to context are subject to Security/Alias Checks): {} points to {}", + baseResource, realUri.toASCIIString()); + } } _availability.set(Availability.STARTING); diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/MemoryResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/MemoryResource.java index 31970e17cf17..5215218021c8 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/MemoryResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/MemoryResource.java @@ -155,6 +155,6 @@ public Collection getAllResources() @Override public String toString() { - return getName(); + return "(Memory) " + _uri.toASCIIString(); } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index d88930e1d6c0..b5fc9657b268 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -460,4 +460,13 @@ public boolean isSameFile(Path path) } return false; } + + public String toString() + { + String str = getName(); + URI uri = getURI(); + if (uri != null) + str = getURI().toASCIIString(); + return "(" + this.getClass().getSimpleName() + ") " + str; + } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResourceFactory.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResourceFactory.java index f186bb23c72e..61db423d822e 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResourceFactory.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResourceFactory.java @@ -308,7 +308,7 @@ public URI getRealURI() @Override public String toString() { - return String.format("URLResource@%X(%s)", this.uri.hashCode(), this.uri.toASCIIString()); + return "(URL) " + this.uri.toASCIIString(); } private static class InputStreamReference extends AtomicReference implements Runnable diff --git a/jetty-ee8/jetty-ee8-servlet/pom.xml b/jetty-ee8/jetty-ee8-servlet/pom.xml index 396c69093156..0d3a23f1a77b 100644 --- a/jetty-ee8/jetty-ee8-servlet/pom.xml +++ b/jetty-ee8/jetty-ee8-servlet/pom.xml @@ -74,6 +74,8 @@ org.apache.maven.plugins maven-surefire-plugin + + false @{argLine} ${jetty.surefire.argLine} --add-modules org.eclipse.jetty.util.ajax --add-reads org.eclipse.jetty.ee8.servlet=org.eclipse.jetty.logging diff --git a/jetty-ee9/jetty-ee9-servlet/pom.xml b/jetty-ee9/jetty-ee9-servlet/pom.xml index 7fc24c7605f2..6e0634c64f7b 100644 --- a/jetty-ee9/jetty-ee9-servlet/pom.xml +++ b/jetty-ee9/jetty-ee9-servlet/pom.xml @@ -72,6 +72,8 @@ maven-surefire-plugin + + false @{argLine} ${jetty.surefire.argLine} --add-modules org.eclipse.jetty.util.ajax --add-reads org.eclipse.jetty.ee9.servlet=org.eclipse.jetty.logging diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java index 7c157f58f267..f6eb2bbb39e5 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java @@ -13,6 +13,8 @@ package org.eclipse.jetty.ee9.servlet; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.net.URL; @@ -28,6 +30,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Stream; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; import jakarta.servlet.DispatcherType; import jakarta.servlet.Filter; @@ -59,10 +63,12 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.resource.FileSystemPool; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; +import org.eclipse.jetty.util.resource.URLResourceFactory; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -2560,6 +2566,49 @@ public void testGetUtf8NfdFile() throws Exception } } + @Test + public void testGetPrecompressedSuffixMapping() throws Exception + { + Path docRoot = workDir.getEmptyPathDir().resolve("docroot"); + FS.ensureDirExists(docRoot); + + startServer((context) -> + { + ResourceFactory.registerResourceFactory("file", new URLResourceFactory()); + Resource resource = ResourceFactory.of(context).newResource(docRoot); + assertThat("Expecting URLResource", resource.getClass().getName(), endsWith("URLResource")); + context.setBaseResource(resource); + + ServletHolder defholder = context.addServlet(DefaultServlet.class, "*.js"); + defholder.setInitParameter("cacheControl", "no-store"); + defholder.setInitParameter("dirAllowed", "false"); + defholder.setInitParameter("gzip", "false"); + defholder.setInitParameter("precompressed", "gzip=.gz"); + }); + + + FS.ensureDirExists(docRoot.resolve("scripts")); + + String scriptText = "This is a script"; + Files.writeString(docRoot.resolve("scripts/script.js"), scriptText, UTF_8); + + byte[] compressedBytes = compressGzip(scriptText); + Files.write(docRoot.resolve("scripts/script.js.gz"), compressedBytes); + + String rawResponse = connector.getResponse(""" + GET /context/scripts/script.js HTTP/1.1 + Host: test + Accept-Encoding: gzip + Connection: close + + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat("Suffix url-pattern mapping not used", response.get(HttpHeader.CACHE_CONTROL), is("no-store")); + String responseDecompressed = decompressGzip(response.getContentBytes()); + assertThat(responseDecompressed, is("This is a script")); + } + @Test public void testHead() throws Exception { @@ -2950,6 +2999,31 @@ private static Path assumeMkDirSupported(Path path, String subpath) return ret; } + private static byte[] compressGzip(String textToCompress) throws IOException + { + try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); + GZIPOutputStream gzipOut = new GZIPOutputStream(baos); + ByteArrayInputStream input = new ByteArrayInputStream(textToCompress.getBytes(UTF_8))) + { + IO.copy(input, gzipOut); + gzipOut.flush(); + gzipOut.finish(); + return baos.toByteArray(); + } + } + + private static String decompressGzip(byte[] compressedContent) throws IOException + { + try (ByteArrayInputStream input = new ByteArrayInputStream(compressedContent); + GZIPInputStream gzipInput = new GZIPInputStream(input); + ByteArrayOutputStream output = new ByteArrayOutputStream()) + { + IO.copy(gzipInput, output); + output.flush(); + return output.toString(UTF_8); + } + } + public static class Scenarios extends ArrayList { public void addScenario(String description, String rawRequest, int expectedStatus) diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DispatcherTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DispatcherTest.java index 2b1abfdc82ab..03bccebb041e 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DispatcherTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DispatcherTest.java @@ -66,8 +66,10 @@ import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.UrlEncoded; import org.eclipse.jetty.util.component.LifeCycle; +import org.eclipse.jetty.util.resource.FileSystemPool; import org.hamcrest.Matcher; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -75,6 +77,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -124,6 +127,12 @@ protected ContextHandlerCollection createDefaultContextHandlerCollection() return _contextCollection; } + @BeforeEach + public void ensureFileSystemPoolIsSane() + { + assertThat(FileSystemPool.INSTANCE.mounts(), empty()); + } + @AfterEach public void destroy() throws Exception {