diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 23e4a5ad7ed..08bb7eb52fd 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.util.resource; import java.io.File; +import java.io.IOError; import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; @@ -181,7 +182,7 @@ public class PathResource extends Resource // different number of segments return false; } - + // compare each segment of path, backwards for (int i = bCount; i-- > 0; ) { @@ -190,7 +191,7 @@ public class PathResource extends Resource return false; } } - + return true; } @@ -223,7 +224,23 @@ public class PathResource extends Resource */ public PathResource(Path path) { - this.path = path.toAbsolutePath(); + Path absPath = path; + try + { + absPath = path.toAbsolutePath(); + } + catch (IOError ioError) + { + // Not able to resolve absolute path from provided path + // This could be due to a glob reference, or a reference + // to a path that doesn't exist (yet) + if (LOG.isDebugEnabled()) + LOG.debug("Unable to get absolute path for {}", path, ioError); + } + + // cleanup any lingering relative path nonsense (like "/./" and "/../") + this.path = absPath.normalize(); + assertValidPath(path); this.uri = this.path.toUri(); this.alias = checkAliasPath(); 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 e6644971d00..a098a18e5e8 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 @@ -30,6 +30,7 @@ import java.net.URL; import java.nio.channels.ReadableByteChannel; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.text.DateFormat; import java.util.ArrayList; @@ -174,19 +175,8 @@ public abstract class Resource implements ResourceFactory, Closeable !resource.startsWith("file:") && !resource.startsWith("jar:")) { - try - { - // It's a file. - if (resource.startsWith("./")) - resource = resource.substring(2); - File file = new File(resource).getCanonicalFile(); - return new PathResource(file); - } - catch (IOException e2) - { - e2.addSuppressed(e); - throw e2; - } + // It's likely a file/path reference. + return new PathResource(Paths.get(resource)); } else { diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java index dd2188c080d..ddb5153ff6b 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java @@ -19,9 +19,11 @@ package org.eclipse.jetty.util.resource; import java.io.File; +import java.io.IOException; import java.io.InputStream; import java.net.URI; import java.net.URL; +import java.nio.file.Path; import java.util.ArrayList; import java.util.stream.Stream; @@ -30,6 +32,9 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.IO; import org.hamcrest.Matchers; import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -37,6 +42,7 @@ import org.junit.jupiter.params.provider.MethodSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.startsWith; +import static org.junit.jupiter.api.Assertions.assertNotNull; public class ResourceTest { @@ -279,4 +285,16 @@ public class ResourceTest String c = IO.toString(in); assertThat("Content: " + data.test, c, startsWith(data.content)); } + + @Test + @DisabledOnOs(OS.WINDOWS) // this uses forbidden characters on some Windows Environments + public void testGlobPath() throws IOException + { + Path testDir = MavenTestingUtils.getTargetTestingPath("testGlobPath"); + FS.ensureEmpty(testDir); + + String globReference = testDir.toAbsolutePath().toString() + File.separator + '*'; + Resource globResource = Resource.newResource(globReference); + assertNotNull(globResource, "Should have produced a Resource"); + } } 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 b45e52a088b..94cb1e8fb31 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 @@ -42,6 +42,7 @@ import java.util.jar.JarEntry; import java.util.jar.JarFile; 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; @@ -809,17 +810,51 @@ public class MetaInfConfiguration extends AbstractConfiguration if (context == null || context.getExtraClasspath() == null) return null; - List jarResources = new ArrayList(); + List jarResources = new ArrayList<>(); StringTokenizer tokenizer = new StringTokenizer(context.getExtraClasspath(), ",;"); while (tokenizer.hasMoreTokens()) { - Resource resource = context.newResource(tokenizer.nextToken().trim()); - String fnlc = resource.getName().toLowerCase(Locale.ENGLISH); - int dot = fnlc.lastIndexOf('.'); - String extension = (dot < 0 ? null : fnlc.substring(dot)); - if (extension != null && (extension.equals(".jar") || extension.equals(".zip"))) + String token = tokenizer.nextToken().trim(); + + // Is this a Glob Reference? + if (isGlobReference(token)) { - jarResources.add(resource); + 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); + } } } @@ -865,13 +900,20 @@ public class MetaInfConfiguration extends AbstractConfiguration if (context == null || context.getExtraClasspath() == null) return null; - List dirResources = new ArrayList(); + List dirResources = new ArrayList<>(); StringTokenizer tokenizer = new StringTokenizer(context.getExtraClasspath(), ",;"); while (tokenizer.hasMoreTokens()) { - Resource resource = context.newResource(tokenizer.nextToken().trim()); - if (resource.exists() && resource.isDirectory()) - dirResources.add(resource); + 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; @@ -889,4 +931,17 @@ public class MetaInfConfiguration extends AbstractConfiguration return "jar:" + uriString + suffix; } } + + private boolean isGlobReference(String token) + { + return token.endsWith("/*") || token.endsWith("\\*"); + } + + private boolean isFileSupported(Resource resource) + { + String filenameLowercase = resource.getName().toLowerCase(Locale.ENGLISH); + int dot = filenameLowercase.lastIndexOf('.'); + String extension = (dot < 0 ? null : filenameLowercase.substring(dot)); + return (extension != null && (extension.equals(".jar") || extension.equals(".zip"))); + } } 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 82806a3eb7e..22b1cc8c1df 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 @@ -320,29 +320,30 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility { if (lib.exists() && lib.isDirectory()) { - String[] files = lib.list(); - if (files != null) + String[] entries = lib.list(); + if (entries != null) { - Arrays.sort(files); - } - for (int f = 0; files != null && f < files.length; f++) - { - try + Arrays.sort(entries); + + for (String entry : entries) { - Resource fn = lib.addPath(files[f]); - if (LOG.isDebugEnabled()) - LOG.debug("addJar - {}", fn); - String fnlc = fn.getName().toLowerCase(Locale.ENGLISH); - // don't check if this is a directory (prevents use of symlinks), see Bug 353165 - if (isFileSupported(fnlc)) + try { - String jar = URIUtil.encodeSpecific(fn.toString(), ",;"); - addClassPath(jar); + Resource resource = lib.addPath(entry); + if (LOG.isDebugEnabled()) + LOG.debug("addJar - {}", resource); + String fnlc = resource.getName().toLowerCase(Locale.ENGLISH); + // don't check if this is a directory (prevents use of symlinks), see Bug 353165 + if (isFileSupported(fnlc)) + { + String jar = URIUtil.encodeSpecific(resource.toString(), ",;"); + addClassPath(jar); + } + } + catch (Exception ex) + { + LOG.warn("Unable to load WEB-INF/lib JAR {}", entry, ex); } - } - catch (Exception ex) - { - LOG.warn("Unable to load WEB-INF/lib JAR {}", files[f], ex); } } } diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextExtraClasspathTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextExtraClasspathTest.java new file mode 100644 index 00000000000..80e878db8af --- /dev/null +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextExtraClasspathTest.java @@ -0,0 +1,216 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.webapp; + +import java.io.File; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.util.component.LifeCycle; +import org.eclipse.jetty.util.resource.PathResource; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class WebAppContextExtraClasspathTest +{ + private Server server; + + private Server newServer() + { + server = new Server(); + ServerConnector connector = new ServerConnector(server); + connector.setPort(0); + server.addConnector(connector); + return server; + } + + @AfterEach + public void tearDown() + { + LifeCycle.stop(server); + } + + @Test + public void testBaseResourceAbsolutePath() throws Exception + { + Server server = newServer(); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/"); + + Path warPath = MavenTestingUtils.getTestResourcePathFile("wars/dump.war"); + warPath = warPath.toAbsolutePath(); + assertTrue(warPath.isAbsolute(), "Path should be absolute: " + warPath); + // Use String reference to war + // On Unix / Linux this should have no issue. + // On Windows with fully qualified paths such as "E:\mybase\webapps\dump.war" the + // resolution of the Resource can trigger various URI issues with the "E:" portion of the provided String. + context.setResourceBase(warPath.toString()); + + server.setHandler(context); + server.start(); + + assertTrue(context.isAvailable(), "WebAppContext should be available"); + } + + public static Stream extraClasspathGlob() + { + List references = new ArrayList<>(); + + Path extLibs = MavenTestingUtils.getTestResourcePathDir("ext"); + extLibs = extLibs.toAbsolutePath(); + + // Absolute reference with trailing slash and glob + references.add(Arguments.of(extLibs.toString() + File.separator + "*")); + + // Establish a relative extraClassPath reference + String relativeExtLibsDir = MavenTestingUtils.getBasePath().relativize(extLibs).toString(); + + // This will be in the String form similar to "src/test/resources/ext/*" (with trailing slash and glob) + references.add(Arguments.of(relativeExtLibsDir + File.separator + "*")); + + return references.stream(); + } + + /** + * Test using WebAppContext.setExtraClassPath(String) with a reference to a glob + */ + @ParameterizedTest + @MethodSource("extraClasspathGlob") + public void testExtraClasspathGlob(String extraClasspathGlobReference) throws Exception + { + Server server = newServer(); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/"); + Path warPath = MavenTestingUtils.getTestResourcePathFile("wars/dump.war"); + context.setBaseResource(new PathResource(warPath)); + context.setExtraClasspath(extraClasspathGlobReference); + + server.setHandler(context); + server.start(); + + // Should not have failed the start of the WebAppContext + assertTrue(context.isAvailable(), "WebAppContext should be available"); + + // Test WebAppClassLoader contents for expected jars + ClassLoader contextClassLoader = context.getClassLoader(); + assertThat(contextClassLoader, instanceOf(WebAppClassLoader.class)); + WebAppClassLoader webAppClassLoader = (WebAppClassLoader)contextClassLoader; + Path extLibsDir = MavenTestingUtils.getTestResourcePathDir("ext"); + extLibsDir = extLibsDir.toAbsolutePath(); + List expectedPaths = Files.list(extLibsDir) + .filter(Files::isRegularFile) + .filter((path) -> path.toString().endsWith(".jar")) + .collect(Collectors.toList()); + List actualPaths = new ArrayList<>(); + for (URL url : webAppClassLoader.getURLs()) + { + actualPaths.add(Paths.get(url.toURI())); + } + assertThat("WebAppClassLoader.urls.length", actualPaths.size(), is(expectedPaths.size())); + for (Path expectedPath : expectedPaths) + { + boolean found = false; + for (Path actualPath : actualPaths) + { + if (Files.isSameFile(actualPath, expectedPath)) + { + found = true; + } + } + assertTrue(found, "Not able to find expected jar in WebAppClassLoader: " + expectedPath); + } + } + + public static Stream extraClasspathDir() + { + List references = new ArrayList<>(); + + Path extLibs = MavenTestingUtils.getTestResourcePathDir("ext"); + extLibs = extLibs.toAbsolutePath(); + + // Absolute reference with trailing slash + references.add(Arguments.of(extLibs.toString() + File.separator)); + + // Absolute reference without trailing slash + references.add(Arguments.of(extLibs.toString())); + + // Establish a relative extraClassPath reference + String relativeExtLibsDir = MavenTestingUtils.getBasePath().relativize(extLibs).toString(); + + // This will be in the String form similar to "src/test/resources/ext/" (with trailing slash) + references.add(Arguments.of(relativeExtLibsDir + File.separator)); + + // This will be in the String form similar to "src/test/resources/ext/" (without trailing slash) + references.add(Arguments.of(relativeExtLibsDir)); + + return references.stream(); + } + + /** + * Test using WebAppContext.setExtraClassPath(String) with a reference to a directory + */ + @ParameterizedTest + @MethodSource("extraClasspathDir") + public void testExtraClasspathDir(String extraClassPathReference) throws Exception + { + Server server = newServer(); + + WebAppContext context = new WebAppContext(); + context.setContextPath("/"); + Path warPath = MavenTestingUtils.getTestResourcePathFile("wars/dump.war"); + context.setBaseResource(new PathResource(warPath)); + + context.setExtraClasspath(extraClassPathReference); + + server.setHandler(context); + server.start(); + + // Should not have failed the start of the WebAppContext + assertTrue(context.isAvailable(), "WebAppContext should be available"); + + // Test WebAppClassLoader contents for expected directory reference + ClassLoader contextClassLoader = context.getClassLoader(); + assertThat(contextClassLoader, instanceOf(WebAppClassLoader.class)); + WebAppClassLoader webAppClassLoader = (WebAppClassLoader)contextClassLoader; + URL[] urls = webAppClassLoader.getURLs(); + assertThat("URLs", urls.length, is(1)); + Path extLibs = MavenTestingUtils.getTestResourcePathDir("ext"); + extLibs = extLibs.toAbsolutePath(); + assertThat("URL[0]", urls[0].toURI(), is(extLibs.toUri())); + } +} diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java index 17c9fc6336c..f3c6b9b03bd 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java @@ -91,42 +91,42 @@ public class WebAppContextTest assertNotNull(webDefaultXml); assertNotNull(overrideWebXml); assertNotNull(webXmlEmptyPath); - + try { WebAppContext wac = new WebAppContext(); wac.setResourceBase(MavenTestingUtils.getTargetTestingDir().getAbsolutePath()); server.setHandler(wac); - + //test that an empty default-context-path defaults to root wac.setDescriptor(webXmlEmptyPath.getAbsolutePath()); server.start(); assertEquals("/", wac.getContextPath()); - + server.stop(); - + //test web-default.xml value is used wac.setDescriptor(null); wac.setDefaultsDescriptor(webDefaultXml.getAbsolutePath()); server.start(); assertEquals("/one", wac.getContextPath()); - + server.stop(); - + //test web.xml value is used wac.setDescriptor(webXml.getAbsolutePath()); server.start(); assertEquals("/two", wac.getContextPath()); - + server.stop(); - + //test override-web.xml value is used wac.setOverrideDescriptor(overrideWebXml.getAbsolutePath()); server.start(); assertEquals("/three", wac.getContextPath()); server.stop(); - + //test that explicitly set context path is used instead wac.setContextPath("/foo"); server.start(); @@ -314,7 +314,7 @@ public class WebAppContextTest try { String response = connector.getResponse("GET http://localhost:8080 HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n"); - assertThat(response, containsString("200 OK")); + assertThat("Response OK", response, containsString("200 OK")); } finally { diff --git a/jetty-webapp/src/test/resources/wars/dump.war b/jetty-webapp/src/test/resources/wars/dump.war new file mode 100644 index 00000000000..eb17c2e9883 Binary files /dev/null and b/jetty-webapp/src/test/resources/wars/dump.war differ