Issue #11925 - Fix Etag NPE when using URLResource and improve Base R…

…esource 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 <reuseForks> to false for problematic tests.
jetty · Jun 25, 2024 · f78f442 · f78f442
1 parent 718c6fc
commit f78f442
Show file tree

Hide file tree

Showing 11 changed files with 124 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -155,6 +155,6 @@ public Collection<Resource> 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
@@ -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
@@ -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<InputStream> implements Runnable

diff --git a/jetty-ee8/jetty-ee8-servlet/pom.xml b/jetty-ee8/jetty-ee8-servlet/pom.xml
@@ -74,6 +74,8 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
+          <!-- Reuse Forks causes test failures -->
+          <reuseForks>false</reuseForks>
           <argLine>@{argLine} ${jetty.surefire.argLine}
             --add-modules org.eclipse.jetty.util.ajax
             --add-reads org.eclipse.jetty.ee8.servlet=org.eclipse.jetty.logging</argLine>

diff --git a/jetty-ee9/jetty-ee9-servlet/pom.xml b/jetty-ee9/jetty-ee9-servlet/pom.xml
@@ -72,6 +72,8 @@
       <plugin>
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
+          <!-- Reuse Forks causes test failures -->
+          <reuseForks>false</reuseForks>
           <argLine>@{argLine} ${jetty.surefire.argLine}
             --add-modules org.eclipse.jetty.util.ajax
             --add-reads org.eclipse.jetty.ee9.servlet=org.eclipse.jetty.logging</argLine>

diff --git a/...ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java b/...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<Arguments>
     {
         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
@@ -66,15 +66,18 @@
 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;
 import org.slf4j.LoggerFactory;
 
 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
     {