Issue #5133 - Updates based on review

Signed-off-by: Joakim Erdfelt <[email protected]>
jetty · Aug 26, 2020 · ccc8637 · ccc8637
1 parent b35c433
commit ccc8637
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 137 deletions.
diff --git a/...boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java b/...boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java
@@ -20,7 +20,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.lang.reflect.Field;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Enumeration;
@@ -264,6 +263,4 @@ private boolean isAcceptableLibrary(File file, Set<String> pathToClassFiles)
         }
         return true;
     }
-
-    private static Field _contextField;
 }
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CachedContentFactory.java
@@ -188,7 +188,8 @@ public HttpContent getContent(String pathInContext, int maxBufferSize) throws IO
         if (_parent != null)
         {
             HttpContent httpContent = _parent.getContent(pathInContext, maxBufferSize);
-            return httpContent;
+            if (httpContent != null)
+                return httpContent;
         }
 
         return null;
@@ -209,7 +210,7 @@ protected boolean isCacheable(Resource resource)
         return (len > 0 && (_useFileMappedBuffer || (len < _maxCachedFileSize && len < _maxCacheSize)));
     }
 
-    private HttpContent load(String pathInContext, Resource resource, int maxBufferSize)
+    private HttpContent load(String pathInContext, Resource resource, int maxBufferSize) throws IOException
     {
         if (resource == null || !resource.exists())
             return null;
@@ -233,26 +234,18 @@ private HttpContent load(String pathInContext, Resource resource, int maxBufferS
                     if (compressedContent == null || compressedContent.isValid())
                     {
                         compressedContent = null;
-                        try
+                        Resource compressedResource = _factory.getResource(compressedPathInContext);
+                        if (compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() &&
+                            compressedResource.length() < resource.length())
                         {
-                            Resource compressedResource = _factory.getResource(compressedPathInContext);
-                            if (compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() &&
-                                compressedResource.length() < resource.length())
+                            compressedContent = new CachedHttpContent(compressedPathInContext, compressedResource, null);
+                            CachedHttpContent added = _cache.putIfAbsent(compressedPathInContext, compressedContent);
+                            if (added != null)
                             {
-                                compressedContent = new CachedHttpContent(compressedPathInContext, compressedResource, null);
-                                CachedHttpContent added = _cache.putIfAbsent(compressedPathInContext, compressedContent);
-                                if (added != null)
-                                {
-                                    compressedContent.invalidate();
-                                    compressedContent = added;
-                                }
+                                compressedContent.invalidate();
+                                compressedContent = added;
                             }
                         }
-                        catch (IOException e)
-                        {
-                            if (LOG.isDebugEnabled())
-                                LOG.debug("Unable to find compressed path in context: {}", compressedPathInContext, e);
-                        }
                     }
                     if (compressedContent != null)
                         precompresssedContents.put(format, compressedContent);
@@ -286,20 +279,12 @@ private HttpContent load(String pathInContext, Resource resource, int maxBufferS
                 if (compressedContent != null && compressedContent.isValid() && compressedContent.getResource().lastModified() >= resource.lastModified())
                     compressedContents.put(format, compressedContent);
 
-                try
-                {
-                    // Is there a precompressed resource?
-                    Resource compressedResource = _factory.getResource(compressedPathInContext);
-                    if (compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() &&
-                        compressedResource.length() < resource.length())
-                        compressedContents.put(format,
-                            new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext), maxBufferSize));
-                }
-                catch (IOException e)
-                {
-                    if (LOG.isDebugEnabled())
-                        LOG.debug("Unable to find compressed path in context: {}", compressedPathInContext, e);
-                }
+                // Is there a precompressed resource?
+                Resource compressedResource = _factory.getResource(compressedPathInContext);
+                if (compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() &&
+                    compressedResource.length() < resource.length())
+                    compressedContents.put(format,
+                        new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext), maxBufferSize));
             }
             if (!compressedContents.isEmpty())
                 return new ResourceHttpContent(resource, mt, maxBufferSize, compressedContents);

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java
@@ -50,7 +50,7 @@ public ResourceContentFactory(ResourceFactory factory, MimeTypes mimeTypes, Comp
     }
 
     @Override
-    public HttpContent getContent(String pathInContext, int maxBufferSize)
+    public HttpContent getContent(String pathInContext, int maxBufferSize) throws IOException
     {
         try
         {
@@ -60,8 +60,16 @@ public HttpContent getContent(String pathInContext, int maxBufferSize)
         }
         catch (Throwable t)
         {
-            // Any error has potential to reveal fully qualified path
-            throw (InvalidPathException)new InvalidPathException(pathInContext, "Invalid PathInContext").initCause(t);
+            // There are many potential Exceptions that can reveal a fully qualified path.
+            // See Issue #2560 - Always wrap a Throwable here in an InvalidPathException
+            // that is limited to only the provided pathInContext.
+            // The cause (which might reveal a fully qualified path) is still available,
+            // on the Exception and the logging, but is not reported in normal error page situations.
+            // This specific exception also allows WebApps to specifically hook into a known / reliable
+            // Exception type for ErrorPageErrorHandling logic.
+            InvalidPathException saferException = new InvalidPathException(pathInContext, "Invalid PathInContext");
+            saferException.initCause(t);
+            throw saferException;
         }
     }
 

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
@@ -881,6 +881,6 @@ public interface WelcomeFactory
          * @param pathInContext the path of the request
          * @return The path of the matching welcome file in context or null.
          */
-        String getWelcomeFile(String pathInContext);
+        String getWelcomeFile(String pathInContext) throws IOException;
     }
 }
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
@@ -18,9 +18,10 @@
 
 package org.eclipse.jetty.server.handler;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Arrays;
 import java.util.List;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -36,6 +37,7 @@
 import org.eclipse.jetty.server.ResourceService;
 import org.eclipse.jetty.server.ResourceService.WelcomeFactory;
 import org.eclipse.jetty.server.handler.ContextHandler.Context;
+import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.util.resource.ResourceFactory;
@@ -74,30 +76,21 @@ protected void notFound(HttpServletRequest request, HttpServletResponse response
             {
             }
         });
-        _resourceService.setGzipEquivalentFileExtensions(new ArrayList<>(Collections.singletonList(".svgz")));
+        _resourceService.setGzipEquivalentFileExtensions(new ArrayList<>(Arrays.asList(new String[]{".svgz"})));
     }
 
     @Override
-    public String getWelcomeFile(String pathInContext)
+    public String getWelcomeFile(String pathInContext) throws IOException
     {
         if (_welcomes == null)
             return null;
 
         for (int i = 0; i < _welcomes.length; i++)
         {
             String welcomeInContext = URIUtil.addPaths(pathInContext, _welcomes[i]);
-            try
-            {
-                Resource welcome = getResource(welcomeInContext);
-                if (welcome.exists())
-                    return welcomeInContext;
-            }
-            catch (IOException e)
-            {
-                // this happens on a critical failure of Resource
-                if (LOG.isDebugEnabled())
-                    LOG.debug("Failed to resolve welcome file: {}", welcomeInContext);
-            }
+            Resource welcome = getResource(welcomeInContext);
+            if (welcome.exists())
+                return welcomeInContext;
         }
         // not found
         return null;
@@ -154,37 +147,46 @@ public Resource getResource(String path) throws IOException
         if (LOG.isDebugEnabled())
             LOG.debug("{} getResource({}): baseResource:{}", _context == null ? _baseResource : _context, path, _baseResource);
 
-        if (path != null && path.startsWith("/"))
+        if (StringUtil.isBlank(path))
         {
-            Resource r = null;
+            throw new IllegalArgumentException("Path is blank");
+        }
 
-            if (_baseResource != null)
-            {
-                path = URIUtil.canonicalPath(path);
-                r = _baseResource.addPath(path);
-
-                if (r.isAlias() && (_context == null || !_context.checkAlias(path, r)))
-                {
-                    if (LOG.isDebugEnabled())
-                        LOG.debug("Rejected alias resource={} alias={}", r, r.getAlias());
-                    throw new IOException("Rejected (see debug logs)");
-                }
-            }
-            else if (_context != null)
-            {
-                r = _context.getResource(path);
-                if (r != null)
-                    return r;
-            }
+        if (!path.startsWith("/"))
+        {
+            throw new IllegalArgumentException("Path reference invalid: " + path);
+        }
 
-            if ((r == null || !r.exists()) && path.endsWith("/jetty-dir.css"))
-                r = getStylesheet();
+        Resource r = null;
 
+        if (_baseResource != null)
+        {
+            path = URIUtil.canonicalPath(path);
+            r = _baseResource.addPath(path);
+
+            if (r.isAlias() && (_context == null || !_context.checkAlias(path, r)))
+            {
+                if (LOG.isDebugEnabled())
+                    LOG.debug("Rejected alias resource={} alias={}", r, r.getAlias());
+                throw new IllegalStateException("Rejected alias reference: " + path);
+            }
+        }
+        else if (_context != null)
+        {
+            r = _context.getResource(path);
             if (r != null)
                 return r;
         }
 
-        throw new IOException("Unable to find Resource for " + path);
+        if ((r == null || !r.exists()) && path.endsWith("/jetty-dir.css"))
+            r = getStylesheet();
+
+        if (r == null)
+        {
+            throw new FileNotFoundException("Resource: " + path);
+        }
+
+        return r;
     }
 
     /**