From 0b0d7d328223577b2acda0fd9ab2231930762bb8 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 12 Aug 2020 12:50:02 -0500 Subject: [PATCH 1/5] Issue #5133 - Reworking WebAppContext.extraClasspath + Now parsed by WebAppContext into List + Reintroduced Resource.fromList + Refactored ResourceFactory to never return null and always throw an exception if unable to get/create/resolve the Resource Signed-off-by: Joakim Erdfelt --- .../osgi/annotations/AnnotationParser.java | 3 +- .../serverfactory/ServerInstanceWrapper.java | 4 +- .../webapp/OSGiWebappClassLoader.java | 45 +------ .../eclipse/jetty/osgi/boot/utils/Util.java | 4 +- .../jetty/server/CachedContentFactory.java | 47 +++++--- .../jetty/server/ResourceContentFactory.java | 4 +- .../jetty/server/handler/ResourceHandler.java | 40 +++++-- .../org/eclipse/jetty/util/StringUtil.java | 1 + .../eclipse/jetty/util/resource/Resource.java | 111 ++++++++++++++++-- .../util/resource/ResourceCollection.java | 79 +++++-------- .../jetty/util/resource/ResourceFactory.java | 17 ++- .../util/resource/ResourceCollectionTest.java | 4 +- .../jetty/webapp/MetaInfConfiguration.java | 101 ++++------------ .../jetty/webapp/WebAppClassLoader.java | 78 +++++------- .../eclipse/jetty/webapp/WebAppContext.java | 18 ++- 15 files changed, 278 insertions(+), 278 deletions(-) diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationParser.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationParser.java index f4b78fc8f3b..846f1098998 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationParser.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationParser.java @@ -32,7 +32,6 @@ import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jetty.osgi.boot.utils.BundleFileLocatorHelperFactory; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.resource.Resource; -import org.objectweb.asm.Opcodes; import org.osgi.framework.Bundle; import org.osgi.framework.Constants; @@ -145,7 +144,7 @@ public class AnnotationParser extends org.eclipse.jetty.annotations.AnnotationPa } }); boolean hasDotPath = false; - StringTokenizer tokenizer = new StringTokenizer(bundleClasspath, ",;", false); + StringTokenizer tokenizer = new StringTokenizer(bundleClasspath, StringUtil.DEFAULT_DELIMS, false); while (tokenizer.hasMoreTokens()) { String token = tokenizer.nextToken().trim(); diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/serverfactory/ServerInstanceWrapper.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/serverfactory/ServerInstanceWrapper.java index e8b2bcb3db2..586af60fc55 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/serverfactory/ServerInstanceWrapper.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/serverfactory/ServerInstanceWrapper.java @@ -233,7 +233,7 @@ public class ServerInstanceWrapper Thread.currentThread().setContextClassLoader(libExtClassLoader); String jettyConfigurationUrls = (String)props.get(OSGiServerConstants.MANAGED_JETTY_XML_CONFIG_URLS); - List jettyConfigurations = jettyConfigurationUrls != null ? Util.fileNamesAsURLs(jettyConfigurationUrls, Util.DEFAULT_DELIMS) : null; + List jettyConfigurations = jettyConfigurationUrls != null ? Util.fileNamesAsURLs(jettyConfigurationUrls, StringUtil.DEFAULT_DELIMS) : null; _server = configure(server, jettyConfigurations, props); @@ -418,7 +418,7 @@ public class ServerInstanceWrapper List libURLs = new ArrayList<>(); - StringTokenizer tokenizer = new StringTokenizer(sharedURLs, ",;", false); + StringTokenizer tokenizer = new StringTokenizer(sharedURLs, StringUtil.DEFAULT_DELIMS, false); while (tokenizer.hasMoreTokens()) { String tok = tokenizer.nextToken(); diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java index b4e4c9c554b..3a21d5d738b 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java @@ -23,12 +23,10 @@ import java.io.IOException; import java.lang.reflect.Field; import java.net.URL; import java.util.ArrayList; -import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.StringTokenizer; import java.util.jar.JarFile; import javax.servlet.http.HttpServlet; @@ -201,22 +199,16 @@ public class OSGiWebappClassLoader extends WebAppClassLoader implements BundleRe @Override public void addClassPath(String classPath) throws IOException { - - StringTokenizer tokenizer = new StringTokenizer(classPath, ",;"); - while (tokenizer.hasMoreTokens()) + for (Resource resource : Resource.fromList(classPath, false, (path) -> getContext().newResource(path))) { - String path = tokenizer.nextToken(); - Resource resource = getContext().newResource(path); - - // Resolve file path if possible File file = resource.getFile(); if (file != null && isAcceptableLibrary(file, JAR_WITH_SUCH_CLASS_MUST_BE_EXCLUDED)) { - super.addClassPath(path); + super.addClassPath(resource); } else { - LOG.info("Did not add " + path + " to the classloader of the webapp " + getContext()); + LOG.info("Did not add {} to the classloader of the webapp {}", resource, getContext()); } } } @@ -274,35 +266,4 @@ public class OSGiWebappClassLoader extends WebAppClassLoader implements BundleRe } private static Field _contextField; - - /** - * In the case of the generation of a webapp via a jetty context file we - * need a proper classloader to setup the app before we have the - * WebappContext So we place a fake one there to start with. We replace it - * with the actual webapp context with this method. We also apply the - * extraclasspath there at the same time. - * - * @param webappContext the web app context - */ - public void setWebappContext(WebAppContext webappContext) - { - try - { - if (_contextField == null) - { - _contextField = WebAppClassLoader.class.getDeclaredField("_context"); - _contextField.setAccessible(true); - } - _contextField.set(this, webappContext); - if (webappContext.getExtraClasspath() != null) - { - addClassPath(webappContext.getExtraClasspath()); - } - } - catch (Throwable t) - { - // humf that will hurt if it does not work. - LOG.warn("Unable to set webappcontext", t); - } - } } diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/utils/Util.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/utils/Util.java index b5e5b370945..997784dc484 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/utils/Util.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/utils/Util.java @@ -35,8 +35,6 @@ import org.osgi.framework.InvalidSyntaxException; */ public class Util { - public static final String DEFAULT_DELIMS = ",;"; - /** * Create an osgi filter for the given classname and server name. * @@ -101,7 +99,7 @@ public class Util public static List fileNamesAsURLs(String val, String delims) throws Exception { - String separators = DEFAULT_DELIMS; + String separators = StringUtil.DEFAULT_DELIMS; if (delims == null) delims = separators; 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 index 3d4cc0f52de..25cd879d620 100644 --- 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,8 +188,7 @@ public class CachedContentFactory implements HttpContent.ContentFactory if (_parent != null) { HttpContent httpContent = _parent.getContent(pathInContext, maxBufferSize); - if (httpContent != null) - return httpContent; + return httpContent; } return null; @@ -234,18 +233,26 @@ public class CachedContentFactory implements HttpContent.ContentFactory if (compressedContent == null || compressedContent.isValid()) { compressedContent = null; - Resource compressedResource = _factory.getResource(compressedPathInContext); - if (compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() && - compressedResource.length() < resource.length()) + try { - compressedContent = new CachedHttpContent(compressedPathInContext, compressedResource, null); - CachedHttpContent added = _cache.putIfAbsent(compressedPathInContext, compressedContent); - if (added != null) + Resource compressedResource = _factory.getResource(compressedPathInContext); + if (compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() && + compressedResource.length() < resource.length()) { - compressedContent.invalidate(); - compressedContent = added; + compressedContent = new CachedHttpContent(compressedPathInContext, compressedResource, null); + CachedHttpContent added = _cache.putIfAbsent(compressedPathInContext, compressedContent); + if (added != null) + { + 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); @@ -279,12 +286,20 @@ public class CachedContentFactory implements HttpContent.ContentFactory if (compressedContent != null && compressedContent.isValid() && compressedContent.getResource().lastModified() >= resource.lastModified()) compressedContents.put(format, compressedContent); - // 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)); + 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); + } } 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 index d235990aa52..a402ec3efcb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceContentFactory.java @@ -51,14 +51,12 @@ public class ResourceContentFactory implements ContentFactory @Override public HttpContent getContent(String pathInContext, int maxBufferSize) - throws IOException { try { // try loading the content from our factory. Resource resource = _factory.getResource(pathInContext); - HttpContent loaded = load(pathInContext, resource, maxBufferSize); - return loaded; + return load(pathInContext, resource, maxBufferSize); } catch (Throwable t) { 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 b3412f56f11..733af0e699b 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 @@ -20,7 +20,7 @@ package org.eclipse.jetty.server.handler; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.List; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -74,7 +74,7 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, { } }); - _resourceService.setGzipEquivalentFileExtensions(new ArrayList<>(Arrays.asList(new String[]{".svgz"}))); + _resourceService.setGzipEquivalentFileExtensions(new ArrayList<>(Collections.singletonList(".svgz"))); } @Override @@ -86,9 +86,18 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, for (int i = 0; i < _welcomes.length; i++) { String welcomeInContext = URIUtil.addPaths(pathInContext, _welcomes[i]); - Resource welcome = getResource(welcomeInContext); - if (welcome != null && welcome.exists()) - return welcomeInContext; + 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); + } } // not found return null; @@ -140,13 +149,19 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, } @Override - public Resource getResource(String path) + public Resource getResource(String path) throws IOException { if (LOG.isDebugEnabled()) - LOG.debug("{} getResource({})", _context == null ? _baseResource : _context, _baseResource, path); + LOG.debug("{} newResource({}): baseResource:{}", _context == null ? _baseResource : _context, path, _baseResource); - if (path == null || !path.startsWith("/")) - return null; + if (path == null) + { + throw new IOException("null path"); + } + else if (!path.startsWith("/")) + { + throw new IOException("Invalid path reference: " + path); + } try { @@ -161,7 +176,7 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, { if (LOG.isDebugEnabled()) LOG.debug("resource={} alias={}", r, r.getAlias()); - return null; + throw new IOException("Unacceptable alias reference: " + r); } } else if (_context != null) @@ -170,14 +185,15 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, if ((r == null || !r.exists()) && path.endsWith("/jetty-dir.css")) r = getStylesheet(); - return r; + if (r != null) + return r; } catch (Exception e) { LOG.debug("Unable to get Resource for {}", path, e); } - return null; + throw new IOException("Unable to find Resource for " + path); } /** diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java index 77aebf33779..2b6c1fe9fd4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java @@ -37,6 +37,7 @@ public class StringUtil public static final String ALL_INTERFACES = "0.0.0.0"; public static final String CRLF = "\r\n"; + public static final String DEFAULT_DELIMS = ",;"; public static final String __ISO_8859_1 = "iso-8859-1"; public static final String __UTF8 = "utf-8"; diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index a098a18e5e8..a15f466802c 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -34,10 +34,13 @@ import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.text.DateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Base64; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.List; +import java.util.StringTokenizer; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.Loader; @@ -419,22 +422,11 @@ public abstract class Resource implements ResourceFactory, Closeable /** * Get a resource from within this resource. - *

- * This method is essentially an alias for {@link #addPath(String)}, but without checked exceptions. - * This method satisfied the {@link ResourceFactory} interface. */ @Override - public Resource getResource(String path) + public Resource getResource(String path) throws IOException { - try - { - return addPath(path); - } - catch (Exception e) - { - LOG.debug("Unable to addPath", e); - return null; - } + return addPath(path); } // FIXME: this appears to not be used @@ -921,4 +913,97 @@ public abstract class Resource implements ResourceFactory, Closeable { return file.toURI().toURL(); } + + /** + * Parse a list of String delimited resources and + * return the List of Resources instances it represents. + *

+ * Supports glob references that end in {@code /*} or {@code \*}. + * Glob references will only iterate through the level specified and will not traverse + * found directories within the glob reference. + *

+ * + * @param delimitedReferences the comma {@code ,} or semicolon {@code ;} delimited + * String of resource references. + * @param globDirs true if glob references return directories within the glob as well + * @return the list of resources parsed from input string. + */ + public static List fromList(String delimitedReferences, boolean globDirs) throws IOException + { + return fromList(delimitedReferences, globDirs, Resource::newResource); + } + + /** + * Parse a delimited String of resource references and + * return the List of Resources instances it represents. + *

+ * Supports glob references that end in {@code /*} or {@code \*}. + * Glob references will only iterate through the level specified and will not traverse + * found directories within the glob reference. + *

+ * + * @param delimitedReferences the comma {@code ,} or semicolon {@code ;} delimited + * String of resource references. + * @param globDirs true if glob references return directories within the glob as well + * @param resourceFactory the ResourceFactory used to create new Resource references + * @return the list of resources parsed from input string. + */ + public static List fromList(String delimitedReferences, boolean globDirs, ResourceFactory resourceFactory) throws IOException + { + if (StringUtil.isBlank(delimitedReferences)) + { + return Collections.emptyList(); + } + + List resources = new ArrayList<>(); + + StringTokenizer tokenizer = new StringTokenizer(delimitedReferences, StringUtil.DEFAULT_DELIMS); + while (tokenizer.hasMoreTokens()) + { + String token = tokenizer.nextToken().trim(); + + // Is this a glob reference? + if (token.endsWith("/*") || token.endsWith("\\*")) + { + String dir = token.substring(0, token.length() - 2); + // Use directory + Resource dirResource = resourceFactory.getResource(dir); + if (dirResource.exists() && dirResource.isDirectory()) + { + // To obtain the list of entries + String[] entries = dirResource.list(); + if (entries != null) + { + Arrays.sort(entries); + for (String entry : entries) + { + try + { + Resource resource = dirResource.addPath(entry); + if (!resource.isDirectory()) + { + resources.add(resource); + } + else if (globDirs) + { + resources.add(resource); + } + } + catch (Exception ex) + { + LOG.warn("Bad glob [{}] entry: {}", token, entry, ex); + } + } + } + } + } + else + { + // Simple reference, add as-is + resources.add(resourceFactory.getResource(token)); + } + } + + return resources; + } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index ab3371df2c0..d2159656106 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -29,8 +29,8 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.StringTokenizer; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; /** @@ -131,8 +131,9 @@ public class ResourceCollection extends Resource * Instantiates a new resource collection. * * @param csvResources the string containing comma-separated resource strings + * @throws IOException if any listed resource is not valid */ - public ResourceCollection(String csvResources) + public ResourceCollection(String csvResources) throws IOException { setResourcesAsCSV(csvResources); } @@ -147,6 +148,22 @@ public class ResourceCollection extends Resource return _resources; } + /** + * Sets the resource collection's resources. + * + * @param res the resources to set + */ + public void setResources(List res) + { + if (res.isEmpty()) + { + _resources = null; + return; + } + + _resources = res.toArray(new Resource[0]); + } + /** * Sets the resource collection's resources. * @@ -167,13 +184,7 @@ public class ResourceCollection extends Resource res.add(resource); } - if (res.isEmpty()) - { - _resources = null; - return; - } - - _resources = res.toArray(new Resource[0]); + setResources(res); } /** @@ -182,56 +193,22 @@ public class ResourceCollection extends Resource * * @param csvResources the comma-separated string containing * one or more resource strings. + * @throws IOException if unable resource declared is not valid */ - public void setResourcesAsCSV(String csvResources) + public void setResourcesAsCSV(String csvResources) throws IOException { - if (csvResources == null) + if (StringUtil.isBlank(csvResources)) { - throw new IllegalArgumentException("CSV String is null"); - } - - StringTokenizer tokenizer = new StringTokenizer(csvResources, ",;"); - int len = tokenizer.countTokens(); - if (len == 0) - { - throw new IllegalArgumentException("ResourceCollection@setResourcesAsCSV(String) " + - " argument must be a string containing one or more comma-separated resource strings."); + throw new IllegalArgumentException("CSV String is blank"); } List res = new ArrayList<>(); - - try + for (Resource resource : Resource.fromList(csvResources, false)) { - while (tokenizer.hasMoreTokens()) - { - String token = tokenizer.nextToken().trim(); - // TODO: If we want to support CSV tokens with spaces then we should not trim here - // However, if we decide to to this, then CVS formatting/syntax becomes more strict. - if (token.length() == 0) - { - continue; // skip - } - Resource resource = Resource.newResource(token); - assertResourceValid(resource); - res.add(resource); - } - - if (res.isEmpty()) - { - _resources = null; - return; - } - - _resources = res.toArray(new Resource[0]); - } - catch (RuntimeException e) - { - throw e; - } - catch (Exception e) - { - throw new RuntimeException(e); + assertResourceValid(resource); + res.add(resource); } + setResources(res); } /** diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java index aa272f26c51..0b70e6dd597 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java @@ -18,17 +18,26 @@ package org.eclipse.jetty.util.resource; +import java.io.IOException; + /** * ResourceFactory. */ public interface ResourceFactory { - /** - * Get a resource for a path. + * Get a Resource from a provided String. + *

+ * The behavior here is dependent on the + * implementation of ResourceFactory. + * The provided path can be resolved + * against a known Resource, or can + * be a from-scratch Resource. + *

* * @param path The path to the resource - * @return The resource or null + * @return The resource + * @throws IOException if unable to create Resource */ - Resource getResource(String path); + Resource getResource(String path) throws IOException; } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java index 94c0e692f67..e09a441418f 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java @@ -113,7 +113,7 @@ public class ResourceCollectionTest } @Test - public void testSetResourceNullThrowsISE() + public void testSetResourceArrayNullThrowsISE() { // Create a ResourceCollection with one valid entry Path path = MavenTestingUtils.getTargetPath(); @@ -121,7 +121,7 @@ public class ResourceCollectionTest ResourceCollection coll = new ResourceCollection(resource); // Reset collection to invalid state - coll.setResources(null); + coll.setResources((Resource[])null); assertThrowIllegalStateException(coll); } diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java index 94cb1e8fb31..1619819df46 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaInfConfiguration.java @@ -36,13 +36,12 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.StringTokenizer; import java.util.concurrent.ConcurrentHashMap; import java.util.jar.JarEntry; import java.util.jar.JarFile; +import java.util.stream.Collectors; import org.eclipse.jetty.util.PatternMatcher; -import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.resource.EmptyResource; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceCollection; @@ -802,7 +801,7 @@ public class MetaInfConfiguration extends AbstractConfiguration * * @param context the context to find extra classpath jars in * @return the list of Resources with the extra classpath, or null if not found - * @throws Exception if unable to find the extra classpath jars + * @throws Exception if unable to resolve the extra classpath jars */ protected List findExtraClasspathJars(WebAppContext context) throws Exception @@ -810,55 +809,10 @@ public class MetaInfConfiguration extends AbstractConfiguration if (context == null || context.getExtraClasspath() == null) return null; - List jarResources = new ArrayList<>(); - StringTokenizer tokenizer = new StringTokenizer(context.getExtraClasspath(), ",;"); - while (tokenizer.hasMoreTokens()) - { - String token = tokenizer.nextToken().trim(); - - // Is this a Glob Reference? - if (isGlobReference(token)) - { - String dir = token.substring(0, token.length() - 2); - // Use directory - Resource dirResource = context.newResource(dir); - if (dirResource.exists() && dirResource.isDirectory()) - { - // To obtain the list of files - String[] entries = dirResource.list(); - if (entries != null) - { - Arrays.sort(entries); - for (String entry : entries) - { - try - { - Resource fileResource = dirResource.addPath(entry); - if (isFileSupported(fileResource)) - { - jarResources.add(fileResource); - } - } - catch (Exception ex) - { - LOG.warn(Log.EXCEPTION, ex); - } - } - } - } - } - else - { - // Simple reference, add as-is - Resource resource = context.newResource(token); - if (isFileSupported(resource)) - { - jarResources.add(resource); - } - } - } - - return jarResources; + return context.getExtraClasspath() + .stream() + .filter(this::isFileSupported) + .collect(Collectors.toList()); } /** @@ -892,7 +846,7 @@ public class MetaInfConfiguration extends AbstractConfiguration * * @param context the context to look for extra classpaths in * @return the list of Resources to the extra classpath - * @throws Exception if unable to find the extra classpaths + * @throws Exception if unable to resolve the extra classpath resources */ protected List findExtraClasspathDirs(WebAppContext context) throws Exception @@ -900,23 +854,10 @@ public class MetaInfConfiguration extends AbstractConfiguration if (context == null || context.getExtraClasspath() == null) return null; - List dirResources = new ArrayList<>(); - StringTokenizer tokenizer = new StringTokenizer(context.getExtraClasspath(), ",;"); - while (tokenizer.hasMoreTokens()) - { - String token = tokenizer.nextToken().trim(); - // ignore glob references, they only refer to lists of jars/zips anyway - if (!isGlobReference(token)) - { - Resource resource = context.newResource(token); - if (resource.exists() && resource.isDirectory()) - { - dirResources.add(resource); - } - } - } - - return dirResources; + return context.getExtraClasspath() + .stream() + .filter(Resource::isDirectory) + .collect(Collectors.toList()); } private String uriJarPrefix(URI uri, String suffix) @@ -932,13 +873,23 @@ public class MetaInfConfiguration extends AbstractConfiguration } } - private boolean isGlobReference(String token) - { - return token.endsWith("/*") || token.endsWith("\\*"); - } - private boolean isFileSupported(Resource resource) { + try + { + if (resource.isDirectory()) + return false; + + if (resource.getFile() == null) + return false; + } + catch (Throwable t) + { + if (LOG.isDebugEnabled()) + LOG.debug("Bad Resource reference: {}", resource, t); + return false; + } + String filenameLowercase = resource.getName().toLowerCase(Locale.ENGLISH); int dot = filenameLowercase.lastIndexOf('.'); String extension = (dot < 0 ? null : filenameLowercase.substring(dot)); diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java index 22b1cc8c1df..f9702e8b8ee 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java @@ -41,6 +41,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.util.ClassVisibilityChecker; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.resource.Resource; @@ -112,7 +113,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility */ boolean isParentLoaderPriority(); - String getExtraClasspath(); + List getExtraClasspath(); boolean isServerResource(String name, URL parentUrl); @@ -185,7 +186,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility String extensions = System.getProperty(WebAppClassLoader.class.getName() + ".extensions"); if (extensions != null) { - StringTokenizer tokenizer = new StringTokenizer(extensions, ",;"); + StringTokenizer tokenizer = new StringTokenizer(extensions, StringUtil.DEFAULT_DELIMS); while (tokenizer.hasMoreTokens()) { _extensions.add(tokenizer.nextToken().trim()); @@ -193,7 +194,12 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility } if (context.getExtraClasspath() != null) - addClassPath(context.getExtraClasspath()); + { + for (Resource resource : context.getExtraClasspath()) + { + addClassPath(resource); + } + } } /** @@ -235,7 +241,23 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility } else { - addClassPath(resource.toString()); + // Resolve file path if possible + File file = resource.getFile(); + if (file != null) + { + URL url = resource.getURI().toURL(); + addURL(url); + } + else if (resource.isDirectory()) + { + addURL(resource.getURI().toURL()); + } + else + { + if (LOG.isDebugEnabled()) + LOG.debug("Check file exists and is not nested jar: " + resource); + throw new IllegalArgumentException("File not resolvable or incompatible with URLClassloader: " + resource); + } } } @@ -251,53 +273,9 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility if (classPath == null) return; - StringTokenizer tokenizer = new StringTokenizer(classPath, ",;"); - while (tokenizer.hasMoreTokens()) + for (Resource resource : Resource.fromList(classPath, false, _context::newResource)) { - String token = tokenizer.nextToken().trim(); - - if (token.endsWith("*")) - { - if (token.length() > 1) - { - token = token.substring(0, token.length() - 1); - Resource resource = _context.newResource(token); - if (LOG.isDebugEnabled()) - LOG.debug("Glob Path resource=" + resource); - resource = _context.newResource(token); - addJars(resource); - } - return; - } - - Resource resource = _context.newResource(token); - if (LOG.isDebugEnabled()) - LOG.debug("Path resource=" + resource); - - if (resource.isDirectory() && resource instanceof ResourceCollection) - { - addClassPath(resource); - } - else - { - // Resolve file path if possible - File file = resource.getFile(); - if (file != null) - { - URL url = resource.getURI().toURL(); - addURL(url); - } - else if (resource.isDirectory()) - { - addURL(resource.getURI().toURL()); - } - else - { - if (LOG.isDebugEnabled()) - LOG.debug("Check file exists and is not nested jar: " + resource); - throw new IllegalArgumentException("File not resolvable or incompatible with URLClassloader: " + resource); - } - } + addClassPath(resource); } } diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index a494972dc14..3dcb1b5e850 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -204,7 +204,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL private boolean _persistTmpDir = false; private String _war; - private String _extraClasspath; + private List _extraClasspath; private Throwable _unavailableException; private Map _resourceAliases; @@ -1227,17 +1227,29 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL */ @Override @ManagedAttribute(value = "extra classpath for context classloader", readonly = true) - public String getExtraClasspath() + public List getExtraClasspath() { return _extraClasspath; } /** + * Set the Extra ClassPath via delimited String. + *

+ * This is a convenience method for {@link #setExtraClasspath(List)} + *

+ * * @param extraClasspath Comma or semicolon separated path of filenames or URLs * pointing to directories or jar files. Directories should end * with '/'. + * @throws IOException if unable to resolve the resources referenced + * @see #setExtraClasspath(List) */ - public void setExtraClasspath(String extraClasspath) + public void setExtraClasspath(String extraClasspath) throws IOException + { + setExtraClasspath(Resource.fromList(extraClasspath, false, this::newResource)); + } + + public void setExtraClasspath(List extraClasspath) { _extraClasspath = extraClasspath; } From 6848403d3480f07fb41048108d9946fab542db35 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 12 Aug 2020 13:06:09 -0500 Subject: [PATCH 2/5] Issue #5133 - Reworking WebAppContext.extraClasspath + Reverting name ResourceFactory.newResource(String) to .getResource(String) + Reintroducing Resource.getResource(String) + ResourceHandler.getResource(String) cleaned up in light of Exception handling requirement + Resource.addPath(String) implementations can never return null now Signed-off-by: Joakim Erdfelt --- .../jetty/server/handler/ResourceHandler.java | 27 +++++++------------ .../jetty/util/resource/EmptyResource.java | 2 +- .../eclipse/jetty/util/resource/Resource.java | 2 +- .../util/resource/ResourceCollection.java | 6 +++-- .../jetty/util/resource/URLResource.java | 4 ++- .../jetty/webapp/WebAppClassLoader.java | 1 - 6 files changed, 18 insertions(+), 24 deletions(-) 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 733af0e699b..9cb9b7487b0 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 @@ -152,18 +152,9 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, public Resource getResource(String path) throws IOException { if (LOG.isDebugEnabled()) - LOG.debug("{} newResource({}): baseResource:{}", _context == null ? _baseResource : _context, path, _baseResource); + LOG.debug("{} getResource({}): baseResource:{}", _context == null ? _baseResource : _context, path, _baseResource); - if (path == null) - { - throw new IOException("null path"); - } - else if (!path.startsWith("/")) - { - throw new IOException("Invalid path reference: " + path); - } - - try + if (path != null && path.startsWith("/")) { Resource r = null; @@ -172,15 +163,19 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, path = URIUtil.canonicalPath(path); r = _baseResource.addPath(path); - if (r != null && r.isAlias() && (_context == null || !_context.checkAlias(path, r))) + if (r.isAlias() && (_context == null || !_context.checkAlias(path, r))) { if (LOG.isDebugEnabled()) - LOG.debug("resource={} alias={}", r, r.getAlias()); - throw new IOException("Unacceptable alias reference: " + r); + 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 ((r == null || !r.exists()) && path.endsWith("/jetty-dir.css")) r = getStylesheet(); @@ -188,10 +183,6 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, if (r != null) return r; } - catch (Exception e) - { - LOG.debug("Unable to get Resource for {}", path, e); - } throw new IOException("Unable to find Resource for " + path); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/EmptyResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/EmptyResource.java index 07831e37b11..ebce0643939 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/EmptyResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/EmptyResource.java @@ -124,6 +124,6 @@ public class EmptyResource extends Resource @Override public Resource addPath(String path) throws IOException, MalformedURLException { - return null; + return this; } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index a15f466802c..0ee6c0f34f9 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -413,7 +413,7 @@ public abstract class Resource implements ResourceFactory, Closeable * given name. * * @param path The path segment to add, which is not encoded - * @return the Resource for the resolved path within this Resource. + * @return the Resource for the resolved path within this Resource, never null * @throws IOException if unable to resolve the path * @throws MalformedURLException if the resolution of the path fails because the input path parameter is malformed. */ diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index d2159656106..87321e19412 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -222,7 +222,7 @@ public class ResourceCollection extends Resource if (path == null) { - throw new MalformedURLException(); + throw new MalformedURLException("null path"); } if (path.length() == 0 || URIUtil.SLASH.equals(path)) @@ -270,11 +270,13 @@ public class ResourceCollection extends Resource { return resource; } + if (resources != null) { return new ResourceCollection(resources.toArray(new Resource[0])); } - return null; + + throw new MalformedURLException("path does not result in Resource: " + path); } @Override diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java index 93ab4fa2ffc..58e65bc0f99 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java @@ -290,7 +290,9 @@ public class URLResource extends Resource throws IOException, MalformedURLException { if (path == null) - return null; + { + throw new MalformedURLException("null path"); + } path = URIUtil.canonicalPath(path); diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java index f9702e8b8ee..efc5d7ce3c9 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java @@ -88,7 +88,6 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility */ public interface Context extends ClassVisibilityChecker { - /** * Convert a URL or path to a Resource. * The default implementation From b35c4332b2ceda2ee7fd9a80027424175405fad2 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 20 Aug 2020 13:35:48 -0500 Subject: [PATCH 3/5] Issue #5133 - Fixing ResourceCollection(String) when no entries provided. Signed-off-by: Joakim Erdfelt --- .../jetty/util/resource/ResourceCollection.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index 87321e19412..920506e8b2a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -202,13 +202,18 @@ public class ResourceCollection extends Resource throw new IllegalArgumentException("CSV String is blank"); } - List res = new ArrayList<>(); - for (Resource resource : Resource.fromList(csvResources, false)) + List resources = Resource.fromList(csvResources, false); + if (resources.isEmpty()) + { + throw new IllegalArgumentException("CSV String contains no entries"); + } + List ret = new ArrayList<>(); + for (Resource resource : resources) { assertResourceValid(resource); - res.add(resource); + ret.add(resource); } - setResources(res); + setResources(ret); } /** From ccc863726b60c9d5bf6e068f639b8786aa5c83ec Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 26 Aug 2020 12:19:22 -0500 Subject: [PATCH 4/5] Issue #5133 - Updates based on review Signed-off-by: Joakim Erdfelt --- .../webapp/OSGiWebappClassLoader.java | 3 - .../jetty/server/CachedContentFactory.java | 49 +++----- .../jetty/server/ResourceContentFactory.java | 14 ++- .../eclipse/jetty/server/ResourceService.java | 2 +- .../jetty/server/handler/ResourceHandler.java | 78 ++++++------- .../util/resource/ResourceCollection.java | 105 +++++++++--------- .../jetty/util/resource/ResourceFactory.java | 2 +- .../jetty/util/resource/URLResource.java | 8 +- .../util/resource/ResourceCollectionTest.java | 2 +- 9 files changed, 126 insertions(+), 137 deletions(-) diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java index 3a21d5d738b..636ccc5fd6a 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java @@ -20,7 +20,6 @@ package org.eclipse.jetty.osgi.boot.internal.webapp; 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 @@ public class OSGiWebappClassLoader extends WebAppClassLoader implements BundleRe } 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 index 25cd879d620..de1aae8f96a 100644 --- 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 class CachedContentFactory implements HttpContent.ContentFactory if (_parent != null) { HttpContent httpContent = _parent.getContent(pathInContext, maxBufferSize); - return httpContent; + if (httpContent != null) + return httpContent; } return null; @@ -209,7 +210,7 @@ public class CachedContentFactory implements HttpContent.ContentFactory 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 @@ public class CachedContentFactory implements HttpContent.ContentFactory 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 @@ public class CachedContentFactory implements HttpContent.ContentFactory 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 index a402ec3efcb..bc80e4d5ae0 100644 --- 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 class ResourceContentFactory implements ContentFactory } @Override - public HttpContent getContent(String pathInContext, int maxBufferSize) + public HttpContent getContent(String pathInContext, int maxBufferSize) throws IOException { try { @@ -60,8 +60,16 @@ public class ResourceContentFactory implements ContentFactory } 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 index 0528fb30f49..56e5438ba0e 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 @@ -881,6 +881,6 @@ public class ResourceService * @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 index 9cb9b7487b0..a0f20c6f2de 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 @@ -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.ResourceContentFactory; 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,11 +76,11 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, { } }); - _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; @@ -86,18 +88,9 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory, 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 class ResourceHandler extends HandlerWrapper implements ResourceFactory, 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) + if (!path.startsWith("/")) + { + throw new IllegalArgumentException("Path reference invalid: " + path); + } + + Resource r = null; + + if (_baseResource != null) + { + path = URIUtil.canonicalPath(path); + r = _baseResource.addPath(path); + + if (r.isAlias() && (_context == null || !_context.checkAlias(path, r))) { - 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)"); - } + 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; - } - - if ((r == null || !r.exists()) && path.endsWith("/jetty-dir.css")) - r = getStylesheet(); - + } + 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; } /** diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index 920506e8b2a..d7da2a2f500 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -25,7 +25,7 @@ import java.net.MalformedURLException; import java.net.URI; import java.nio.channels.ReadableByteChannel; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -42,7 +42,7 @@ import org.eclipse.jetty.util.URIUtil; */ public class ResourceCollection extends Resource { - private Resource[] _resources; + private List _resources; /** * Instantiates an empty resource collection. @@ -51,7 +51,7 @@ public class ResourceCollection extends Resource */ public ResourceCollection() { - _resources = new Resource[0]; + _resources = new ArrayList<>(); } /** @@ -61,7 +61,7 @@ public class ResourceCollection extends Resource */ public ResourceCollection(Resource... resources) { - List list = new ArrayList<>(); + _resources = new ArrayList<>(); for (Resource r : resources) { if (r == null) @@ -70,15 +70,25 @@ public class ResourceCollection extends Resource } if (r instanceof ResourceCollection) { - Collections.addAll(list, ((ResourceCollection)r).getResources()); + _resources.addAll(((ResourceCollection)r).getResources()); } else { assertResourceValid(r); - list.add(r); + _resources.add(r); } } - _resources = list.toArray(new Resource[0]); + } + + /** + * Instantiates a new resource collection. + * + * @param resources the resources to be added to collection + */ + public ResourceCollection(Collection resources) + { + _resources = new ArrayList<>(); + _resources.addAll(resources); } /** @@ -88,14 +98,13 @@ public class ResourceCollection extends Resource */ public ResourceCollection(String[] resources) { + _resources = new ArrayList<>(); + if (resources == null || resources.length == 0) { - _resources = null; return; } - ArrayList res = new ArrayList<>(); - try { for (String strResource : resources) @@ -106,16 +115,13 @@ public class ResourceCollection extends Resource } Resource resource = Resource.newResource(strResource); assertResourceValid(resource); - res.add(resource); + _resources.add(resource); } - if (res.isEmpty()) + if (_resources.isEmpty()) { - _resources = null; - return; + throw new IllegalArgumentException("resources cannot be empty or null"); } - - _resources = res.toArray(new Resource[0]); } catch (RuntimeException e) { @@ -141,9 +147,9 @@ public class ResourceCollection extends Resource /** * Retrieves the resource collection's resources. * - * @return the resource array + * @return the resource collection */ - public Resource[] getResources() + public List getResources() { return _resources; } @@ -155,13 +161,13 @@ public class ResourceCollection extends Resource */ public void setResources(List res) { + _resources = new ArrayList<>(); if (res.isEmpty()) { - _resources = null; return; } - _resources = res.toArray(new Resource[0]); + _resources.addAll(res); } /** @@ -235,53 +241,46 @@ public class ResourceCollection extends Resource return this; } - Resource resource = null; - ArrayList resources = null; - int i = 0; - for (; i < _resources.length; i++) + // Attempt a simple (single) Resource lookup that exists + for (Resource res : _resources) { - resource = _resources[i].addPath(path); - if (resource.exists()) + Resource fileResource = res.addPath(path); + if (fileResource.exists()) { - if (resource.isDirectory()) + if (!fileResource.isDirectory()) { - break; + return fileResource; } - return resource; } } - for (i++; i < _resources.length; i++) + // Create a list of potential resource for directories of this collection + ArrayList potentialResources = null; + for (Resource res : _resources) { - Resource r = _resources[i].addPath(path); + Resource r = res.addPath(path); if (r.exists() && r.isDirectory()) { - if (resources == null) + if (potentialResources == null) { - resources = new ArrayList<>(); + potentialResources = new ArrayList<>(); } - if (resource != null) - { - resources.add(resource); - resource = null; - } - - resources.add(r); + potentialResources.add(r); } } - if (resource != null) + if (potentialResources == null || potentialResources.isEmpty()) { - return resource; + throw new MalformedURLException("path does not result in Resource: " + path); } - if (resources != null) + if (potentialResources.size() == 1) { - return new ResourceCollection(resources.toArray(new Resource[0])); + return potentialResources.get(0); } - throw new MalformedURLException("path does not result in Resource: " + path); + return new ResourceCollection(potentialResources); } @Override @@ -421,9 +420,8 @@ public class ResourceCollection extends Resource { Collections.addAll(set, r.list()); } - String[] result = set.toArray(new String[0]); - Arrays.sort(result); - return result; + + return (String[])set.stream().sorted().toArray(); } @Override @@ -449,9 +447,10 @@ public class ResourceCollection extends Resource { assertResourcesSet(); - for (int r = _resources.length; r-- > 0; ) + // Copy in reverse order + for (int r = _resources.size(); r-- > 0; ) { - _resources[r].copyTo(destination); + _resources.get(r).copyTo(destination); } } @@ -461,12 +460,12 @@ public class ResourceCollection extends Resource @Override public String toString() { - if (_resources == null || _resources.length == 0) + if (_resources.isEmpty()) { return "[]"; } - return String.valueOf(Arrays.asList(_resources)); + return String.valueOf(_resources); } @Override @@ -478,7 +477,7 @@ public class ResourceCollection extends Resource private void assertResourcesSet() { - if (_resources == null || _resources.length == 0) + if (_resources == null || _resources.isEmpty()) { throw new IllegalStateException("*resources* not set."); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java index 0b70e6dd597..6797d5bb0c7 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactory.java @@ -36,7 +36,7 @@ public interface ResourceFactory *

* * @param path The path to the resource - * @return The resource + * @return The resource, that might not actually exist (yet). * @throws IOException if unable to create Resource */ Resource getResource(String path) throws IOException; diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java index 58e65bc0f99..3dbd48ffa02 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java @@ -27,6 +27,7 @@ import java.net.URISyntaxException; import java.net.URL; import java.net.URLConnection; import java.nio.channels.ReadableByteChannel; +import java.util.Objects; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.thread.AutoLock; @@ -287,12 +288,9 @@ public class URLResource extends Resource */ @Override public Resource addPath(String path) - throws IOException, MalformedURLException + throws IOException { - if (path == null) - { - throw new MalformedURLException("null path"); - } + Objects.requireNonNull(path, "Path may not be null"); path = URIUtil.canonicalPath(path); diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java index e09a441418f..32d9c643ea0 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceCollectionTest.java @@ -152,7 +152,7 @@ public class ResourceCollectionTest assertThrows(IllegalStateException.class, () -> coll.setResources(new Resource[]{null, null, null})); // Ensure not modified. - assertThat(coll.getResources().length, is(1)); + assertThat(coll.getResources().size(), is(1)); } private void assertThrowIllegalStateException(ResourceCollection coll) From f6bcbda689607e140cdd0db93d4ff724a51ecb52 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 21 Sep 2020 09:42:26 -0500 Subject: [PATCH 5/5] Issue #5133 - Changes from Review with jan & greg Signed-off-by: Joakim Erdfelt --- .../jetty/server/handler/ContextHandler.java | 7 +++ .../eclipse/jetty/util/resource/Resource.java | 28 ++++----- .../util/resource/ResourceCollection.java | 63 ++++++++----------- 3 files changed, 46 insertions(+), 52 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index c3cfd42eb27..593d5029e2c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -1894,6 +1894,13 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu return Collections.unmodifiableMap(_localeEncodingMap); } + /** + * Attempt to get a Resource from the Context. + * + * @param path the path within the resource to attempt to get + * @return the resource, or null if not available. + * @throws MalformedURLException if unable to form a Resource from the provided path + */ public Resource getResource(String path) throws MalformedURLException { if (path == null || !path.startsWith(URIUtil.SLASH)) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 0ee6c0f34f9..63ebdf673e2 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -923,14 +923,14 @@ public abstract class Resource implements ResourceFactory, Closeable * found directories within the glob reference. *

* - * @param delimitedReferences the comma {@code ,} or semicolon {@code ;} delimited + * @param resources the comma {@code ,} or semicolon {@code ;} delimited * String of resource references. - * @param globDirs true if glob references return directories within the glob as well + * @param globDirs true to return directories in addition to files at the level of the glob * @return the list of resources parsed from input string. */ - public static List fromList(String delimitedReferences, boolean globDirs) throws IOException + public static List fromList(String resources, boolean globDirs) throws IOException { - return fromList(delimitedReferences, globDirs, Resource::newResource); + return fromList(resources, globDirs, Resource::newResource); } /** @@ -942,22 +942,22 @@ public abstract class Resource implements ResourceFactory, Closeable * found directories within the glob reference. *

* - * @param delimitedReferences the comma {@code ,} or semicolon {@code ;} delimited + * @param resources the comma {@code ,} or semicolon {@code ;} delimited * String of resource references. - * @param globDirs true if glob references return directories within the glob as well + * @param globDirs true to return directories in addition to files at the level of the glob * @param resourceFactory the ResourceFactory used to create new Resource references * @return the list of resources parsed from input string. */ - public static List fromList(String delimitedReferences, boolean globDirs, ResourceFactory resourceFactory) throws IOException + public static List fromList(String resources, boolean globDirs, ResourceFactory resourceFactory) throws IOException { - if (StringUtil.isBlank(delimitedReferences)) + if (StringUtil.isBlank(resources)) { return Collections.emptyList(); } - List resources = new ArrayList<>(); + List returnedResources = new ArrayList<>(); - StringTokenizer tokenizer = new StringTokenizer(delimitedReferences, StringUtil.DEFAULT_DELIMS); + StringTokenizer tokenizer = new StringTokenizer(resources, StringUtil.DEFAULT_DELIMS); while (tokenizer.hasMoreTokens()) { String token = tokenizer.nextToken().trim(); @@ -982,11 +982,11 @@ public abstract class Resource implements ResourceFactory, Closeable Resource resource = dirResource.addPath(entry); if (!resource.isDirectory()) { - resources.add(resource); + returnedResources.add(resource); } else if (globDirs) { - resources.add(resource); + returnedResources.add(resource); } } catch (Exception ex) @@ -1000,10 +1000,10 @@ public abstract class Resource implements ResourceFactory, Closeable else { // Simple reference, add as-is - resources.add(resourceFactory.getResource(token)); + returnedResources.add(resourceFactory.getResource(token)); } } - return resources; + return returnedResources; } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java index d7da2a2f500..e716cc19333 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java @@ -141,7 +141,7 @@ public class ResourceCollection extends Resource */ public ResourceCollection(String csvResources) throws IOException { - setResourcesAsCSV(csvResources); + setResources(csvResources); } /** @@ -197,24 +197,25 @@ public class ResourceCollection extends Resource * Sets the resources as string of comma-separated values. * This method should be used when configuring jetty-maven-plugin. * - * @param csvResources the comma-separated string containing + * @param resources the comma-separated string containing * one or more resource strings. * @throws IOException if unable resource declared is not valid + * @see Resource#fromList(String, boolean) */ - public void setResourcesAsCSV(String csvResources) throws IOException + public void setResources(String resources) throws IOException { - if (StringUtil.isBlank(csvResources)) + if (StringUtil.isBlank(resources)) { - throw new IllegalArgumentException("CSV String is blank"); + throw new IllegalArgumentException("String is blank"); } - List resources = Resource.fromList(csvResources, false); - if (resources.isEmpty()) + List list = Resource.fromList(resources, false); + if (list.isEmpty()) { - throw new IllegalArgumentException("CSV String contains no entries"); + throw new IllegalArgumentException("String contains no entries"); } List ret = new ArrayList<>(); - for (Resource resource : resources) + for (Resource resource : list) { assertResourceValid(resource); ret.add(resource); @@ -241,46 +242,32 @@ public class ResourceCollection extends Resource return this; } + ArrayList resources = null; + // Attempt a simple (single) Resource lookup that exists for (Resource res : _resources) - { - Resource fileResource = res.addPath(path); - if (fileResource.exists()) - { - if (!fileResource.isDirectory()) - { - return fileResource; - } - } - } - - // Create a list of potential resource for directories of this collection - ArrayList potentialResources = null; - for (Resource res : _resources) { Resource r = res.addPath(path); - if (r.exists() && r.isDirectory()) + if (!r.isDirectory() && r.exists()) { - if (potentialResources == null) - { - potentialResources = new ArrayList<>(); - } - - potentialResources.add(r); + // Return simple (non-directory) Resource + return r; } + + if (resources == null) + { + resources = new ArrayList<>(); + } + + resources.add(r); } - if (potentialResources == null || potentialResources.isEmpty()) + if (resources.size() == 1) { - throw new MalformedURLException("path does not result in Resource: " + path); + return resources.get(0); } - if (potentialResources.size() == 1) - { - return potentialResources.get(0); - } - - return new ResourceCollection(potentialResources); + return new ResourceCollection(resources); } @Override