Issue #11574 war should not be extracted unless configured to do so (#11575)

* Issue #11574 war should not be extracted unless configured to do so
This commit is contained in:
Jan Bartel 2024-03-27 10:02:16 +01:00 committed by GitHub
parent 92cb3cbe7e
commit 94beede357
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 384 additions and 53 deletions

View File

@ -43,6 +43,7 @@ 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;
import org.eclipse.jetty.util.resource.ResourceCollators;
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.eclipse.jetty.util.resource.Resources;
import org.slf4j.Logger;
@ -283,27 +284,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
{
if (!Resources.isReadableDirectory(libs))
return;
for (Resource libDir: libs)
{
Path dir = libDir.getPath();
try (Stream<Path> streamEntries = Files.list(dir))
{
streamEntries
.filter(Files::isRegularFile)
.filter(this::isFileSupported)
.sorted(Comparator.naturalOrder())
.map(Path::toUri)
.map(URIUtil::toJarFileUri)
.map(_resourceFactory::newResource)
.forEach(this::addClassPath);
}
catch (IOException e)
{
LOG.warn("Unable to load WEB-INF/lib JARs: {}", dir, e);
}
}
libs.list().stream().filter(r -> isFileSupported(r.getName())).sorted(ResourceCollators.byName(true)).forEach(this::addClassPath);
}
@Override

View File

@ -218,11 +218,11 @@ public class WebInfConfiguration extends AbstractConfiguration
Resource originalWarResource = webApp;
// Is the WAR usable directly?
if (Resources.isReadableFile(webApp) && FileID.isJavaArchive(webApp.getURI()) && !webApp.getURI().getScheme().equalsIgnoreCase("jar"))
if (Resources.isReadableFile(webApp) && FileID.isArchive(webApp.getURI()) && !webApp.getURI().getScheme().equalsIgnoreCase("jar"))
{
// Turned this into a jar URL.
Resource jarWebApp = context.getResourceFactory().newJarFileResource(webApp.getURI());
if (Resources.isReadableFile(jarWebApp)) // but only if it is readable
if (Resources.isDirectory(jarWebApp))
webApp = jarWebApp;
}
@ -230,7 +230,7 @@ public class WebInfConfiguration extends AbstractConfiguration
if ((context.isCopyWebDir() && webApp.getPath() != null && originalWarResource.isDirectory()) ||
(context.isExtractWAR() && webApp.getPath() != null && !originalWarResource.isDirectory()) ||
(context.isExtractWAR() && webApp.getPath() == null) ||
!originalWarResource.isDirectory()
!webApp.isDirectory()
)
{
// Look for sibling directory.

View File

@ -872,9 +872,9 @@ public class WebAppContextTest
List<String> actualRefs = getWebAppClassLoaderUrlRefs(context);
String[] expectedRefs = new String[]{
"/webapp/WEB-INF/classes/",
"/webapp/WEB-INF/lib/acme.jar!/",
"/webapp/WEB-INF/lib/alpha.jar!/",
"/webapp/WEB-INF/lib/omega.jar!/"
"/webapp/WEB-INF/lib/acme.jar",
"/webapp/WEB-INF/lib/alpha.jar",
"/webapp/WEB-INF/lib/omega.jar"
};
assertThat("URLs (sub) refs", actualRefs, containsInAnyOrder(expectedRefs));
@ -892,9 +892,9 @@ public class WebAppContextTest
actualRefs = getWebAppClassLoaderUrlRefs(context);
expectedRefs = new String[]{
"/webapp/WEB-INF/classes/",
"/webapp/WEB-INF/lib/acme.jar!/",
"/webapp/WEB-INF/lib/alpha.jar!/",
"/webapp/WEB-INF/lib/omega.jar!/"
"/webapp/WEB-INF/lib/acme.jar",
"/webapp/WEB-INF/lib/alpha.jar",
"/webapp/WEB-INF/lib/omega.jar"
};
assertThat("URLs (sub) refs", actualRefs, containsInAnyOrder(expectedRefs));
}

View File

@ -14,21 +14,40 @@
package org.eclipse.jetty.ee10.webapp;
import java.io.IOException;
import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenPaths;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
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.equalTo;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* WebInfConfigurationTest
@ -36,6 +55,38 @@ import static org.hamcrest.Matchers.is;
@ExtendWith(WorkDirExtension.class)
public class WebInfConfigurationTest
{
private static List<String> EXPECTED_JAR_NAMES = Arrays.asList(new String[]{"alpha.jar", "omega.jar", "acme.jar"});
private Server _server;
private static Path createWar(Path tempDir, String name) throws Exception
{
// Create war on the fly
Path testWebappDir = MavenPaths.projectBase().resolve("src/test/webapp");
assertTrue(Files.exists(testWebappDir));
Path warFile = tempDir.resolve(name);
Map<String, String> env = new HashMap<>();
env.put("create", "true");
URI uri = URI.create("jar:" + warFile.toUri().toASCIIString());
// Use ZipFS so that we can create paths that are just "/"
try (FileSystem zipfs = FileSystems.newFileSystem(uri, env))
{
Path root = zipfs.getPath("/");
IO.copyDir(testWebappDir, root);
}
return warFile;
}
@AfterEach
public void afterEach() throws Exception
{
if (_server == null)
return;
_server.stop();
}
public static Stream<Arguments> fileBaseResourceNames()
{
@ -80,4 +131,139 @@ public class WebInfConfigurationTest
assertThat(WebInfConfiguration.getResourceBaseName(resource), is(expectedName));
}
}
/**
* Test that if the WebAppContext is configured NOT to extract anything,
* nothing is extracted.
*/
@Test
public void testShouldNotUnpackWar(WorkDir workDir) throws Exception
{
Path testPath = MavenPaths.targetTestDir("testSimple");
FS.ensureDirExists(testPath);
FS.ensureEmpty(testPath);
_server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
Path warPath = createWar(testPath, "test.war");
context.setExtractWAR(false);
context.setCopyWebDir(false);
context.setCopyWebInf(false);
context.setWar(warPath.toUri().toURL().toString());
_server.setHandler(context);
_server.start();
Path unpackedDir = context.getTempDirectory().toPath().resolve("webapp");
Path unpackedWebInfDir = context.getTempDirectory().toPath().resolve("webinf");
assertFalse(Files.exists(unpackedDir)); //should not have unpacked
assertFalse(Files.exists(unpackedWebInfDir)); //should not have unpacked
}
/**
* Test that if the war should be extracted, it is.
*/
@Test
public void testShouldUnpackWar(WorkDir workDir) throws Exception
{
Path testPath = MavenPaths.targetTestDir("testSimple");
FS.ensureDirExists(testPath);
FS.ensureEmpty(testPath);
_server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
Path warPath = createWar(testPath, "test.war");
context.setExtractWAR(true);
context.setCopyWebDir(false);
context.setCopyWebInf(false);
context.setWar(warPath.toUri().toURL().toString());
_server.setHandler(context);
_server.start();
Path unpackedDir = context.getTempDirectory().toPath().resolve("webapp");
Path unpackedWebInfDir = context.getTempDirectory().toPath().resolve("webinf");
assertTrue(Files.exists(unpackedDir)); //should have unpacked whole war
assertFalse(Files.exists(unpackedWebInfDir)); //should not have re-unpacked WEB-INF
//Check the WEB-INF/lib jars are only in there once
checkNoDuplicateJars(EXPECTED_JAR_NAMES, (URLClassLoader)context.getClassLoader());
}
/**
* Test not unpacking the whole war, just WEB-INF
*/
@Test
public void testShouldUnpackWebInfOnly(WorkDir workDir) throws Exception
{
Path testPath = MavenPaths.targetTestDir("testSimple");
FS.ensureDirExists(testPath);
FS.ensureEmpty(testPath);
_server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
Path warPath = createWar(testPath, "test.war");
context.setExtractWAR(false);
context.setCopyWebDir(false);
context.setCopyWebInf(true);
System.err.println(warPath.toUri().toURL().toString());
context.setWar(warPath.toUri().toURL().toString());
_server.setHandler(context);
_server.start();
Path unpackedDir = context.getTempDirectory().toPath().resolve("webapp");
Path unpackedWebInfDir = context.getTempDirectory().toPath().resolve("webinf");
assertFalse(Files.exists(unpackedDir)); //should not have unpacked whole war
assertTrue(Files.exists(unpackedWebInfDir)); //should have unpacked WEB-INF
assertTrue(Files.exists(unpackedWebInfDir.resolve("WEB-INF").resolve("lib").resolve("alpha.jar")));
//Check the WEB-INF/lib jars are only in there once
checkNoDuplicateJars(EXPECTED_JAR_NAMES, (URLClassLoader)context.getClassLoader());
}
/**
* Odd combination, but it has always been available: test that both the
* whole war can be extracted, _and_ WEB-INF separately.
*/
@Test
public void testShouldUnpackWarAndWebInf(WorkDir workDir) throws Exception
{
Path testPath = MavenPaths.targetTestDir("testSimple");
FS.ensureDirExists(testPath);
FS.ensureEmpty(testPath);
_server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
Path warPath = createWar(testPath, "test.war");
context.setExtractWAR(true);
context.setCopyWebDir(false);
context.setCopyWebInf(true);
context.setWar(warPath.toUri().toURL().toString());
_server.setHandler(context);
_server.start();
Path unpackedDir = context.getTempDirectory().toPath().resolve("webapp");
Path unpackedWebInfDir = context.getTempDirectory().toPath().resolve("webinf");
assertTrue(Files.exists(unpackedDir)); //should have unpacked whole war
assertTrue(Files.exists(unpackedWebInfDir)); //should have re-unpacked WEB-INF
assertTrue(Files.exists(unpackedWebInfDir.resolve("WEB-INF").resolve("lib").resolve("alpha.jar")));
}
/**
* Assert that for each of the expected jar names (stripped of any path info),
* there is only 1 actual jar url in the context classloader
*
* @param expected name of a jar without any preceding path info eg "foo.jar"
* @param classLoader the context classloader
*/
private void checkNoDuplicateJars(List<String> expected, URLClassLoader classLoader)
{
List<String> actual = new ArrayList<>();
for (URL u : classLoader.getURLs())
actual.add(u.toExternalForm());
for (String e : expected)
{
long count = actual.stream().filter(s -> s.endsWith(e)).count();
assertThat(count, equalTo(1L));
}
}
}

View File

@ -43,6 +43,7 @@ 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;
import org.eclipse.jetty.util.resource.ResourceCollators;
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.eclipse.jetty.util.resource.Resources;
import org.slf4j.Logger;
@ -287,26 +288,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility
if (!Resources.isReadableDirectory(libs))
return;
for (Resource libDir: libs)
{
Path dir = libDir.getPath();
try (Stream<Path> streamEntries = Files.list(dir))
{
streamEntries
.filter(Files::isRegularFile)
.filter(this::isFileSupported)
.sorted(Comparator.naturalOrder())
.map(Path::toUri)
.map(URIUtil::toJarFileUri)
.map(_resourceFactory::newResource)
.forEach(this::addClassPath);
}
catch (IOException e)
{
LOG.warn("Unable to load WEB-INF/lib JARs: {}", dir, e);
}
}
libs.list().stream().filter(r -> isFileSupported(r.getName())).sorted(ResourceCollators.byName(true)).forEach(this::addClassPath);
}
@Override

View File

@ -220,11 +220,11 @@ public class WebInfConfiguration extends AbstractConfiguration
Resource originalWarResource = webApp;
// Is the WAR usable directly?
if (Resources.isReadableFile(webApp) && FileID.isJavaArchive(webApp.getURI()) && !webApp.getURI().getScheme().equalsIgnoreCase("jar"))
if (Resources.isReadableFile(webApp) && FileID.isArchive(webApp.getURI()) && !webApp.getURI().getScheme().equalsIgnoreCase("jar"))
{
// Turned this into a jar URL.
Resource jarWebApp = context.getResourceFactory().newJarFileResource(webApp.getURI());
if (Resources.isReadableFile(jarWebApp)) // but only if it is readable
if (Resources.isDirectory(jarWebApp))
webApp = jarWebApp;
}
@ -233,7 +233,7 @@ public class WebInfConfiguration extends AbstractConfiguration
(context.isCopyWebDir() && webApp.getPath() != null && originalWarResource.isDirectory()) ||
(context.isExtractWAR() && webApp.getPath() != null && !originalWarResource.isDirectory()) ||
(context.isExtractWAR() && webApp.getPath() == null) ||
!originalWarResource.isDirectory())
!webApp.isDirectory())
)
{
// Look for sibling directory.

View File

@ -14,21 +14,41 @@
package org.eclipse.jetty.ee9.webapp;
import java.io.IOException;
import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenPaths;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
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.equalTo;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* WebInfConfigurationTest
@ -36,6 +56,37 @@ import static org.hamcrest.Matchers.is;
@ExtendWith(WorkDirExtension.class)
public class WebInfConfigurationTest
{
private static List<String> EXPECTED_JAR_NAMES = Arrays.asList(new String[]{"alpha.jar", "omega.jar", "acme.jar"});
private Server _server;
@AfterEach
public void afterEach() throws Exception
{
if (_server == null)
return;
_server.stop();
}
private static Path createWar(Path tempDir, String name) throws Exception
{
// Create war on the fly
Path testWebappDir = MavenTestingUtils.getTargetPath("test-classes/webapp");
assertTrue(Files.exists(testWebappDir));
Path warFile = tempDir.resolve(name);
Map<String, String> env = new HashMap<>();
env.put("create", "true");
URI uri = URI.create("jar:" + warFile.toUri().toASCIIString());
// Use ZipFS so that we can create paths that are just "/"
try (FileSystem zipfs = FileSystems.newFileSystem(uri, env))
{
Path root = zipfs.getPath("/");
IO.copyDir(testWebappDir, root);
}
return warFile;
}
public static Stream<Arguments> fileBaseResourceNames()
{
@ -80,4 +131,135 @@ public class WebInfConfigurationTest
assertThat(WebInfConfiguration.getResourceBaseName(resource), is(expectedName));
}
}
/**
* Test that if the WebAppContext is configured NOT to extract anything,
* nothing is extracted.
*/
@Test
public void testShouldNotUnpackWar(WorkDir workDir) throws Exception
{
Path testPath = MavenPaths.targetTestDir("testSimple");
FS.ensureDirExists(testPath);
FS.ensureEmpty(testPath);
_server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
Path warPath = createWar(testPath, "test.war");
context.setExtractWAR(false);
context.setCopyWebDir(false);
context.setCopyWebInf(false);
context.setWar(warPath.toUri().toURL().toString());
_server.setHandler(context);
_server.start();
Path unpackedDir = context.getTempDirectory().toPath().resolve("webapp");
Path unpackedWebInfDir = context.getTempDirectory().toPath().resolve("webinf");
assertFalse(Files.exists(unpackedDir)); //should not have unpacked
assertFalse(Files.exists(unpackedWebInfDir)); //should not have unpacked
}
/**
* Test that if the war should be extracted, it is.
*/
@Test
public void testShouldUnpackWar(WorkDir workDir) throws Exception
{
Path testPath = MavenPaths.targetTestDir("testSimple");
FS.ensureDirExists(testPath);
FS.ensureEmpty(testPath);
_server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
Path warPath = createWar(testPath, "test.war");
context.setExtractWAR(true);
context.setCopyWebDir(false);
context.setCopyWebInf(false);
context.setWar(warPath.toUri().toURL().toString());
_server.setHandler(context);
_server.start();
Path unpackedDir = context.getTempDirectory().toPath().resolve("webapp");
Path unpackedWebInfDir = context.getTempDirectory().toPath().resolve("webinf");
assertTrue(Files.exists(unpackedDir)); //should have unpacked whole war
assertFalse(Files.exists(unpackedWebInfDir)); //should not have re-unpacked WEB-INF
checkNoDuplicateJars(EXPECTED_JAR_NAMES, (URLClassLoader)context.getClassLoader());
}
/**
* Test not unpacking the whole war, just WEB-INF
*/
@Test
public void testShouldUnpackWebInfOnly(WorkDir workDir) throws Exception
{
Path testPath = MavenPaths.targetTestDir("testSimple");
FS.ensureDirExists(testPath);
FS.ensureEmpty(testPath);
_server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
Path warPath = createWar(testPath, "test.war");
context.setExtractWAR(false);
context.setCopyWebDir(false);
context.setCopyWebInf(true);
context.setWar(warPath.toUri().toURL().toString());
_server.setHandler(context);
_server.start();
Path unpackedDir = context.getTempDirectory().toPath().resolve("webapp");
Path unpackedWebInfDir = context.getTempDirectory().toPath().resolve("webinf");
assertFalse(Files.exists(unpackedDir)); //should not have unpacked whole war
assertTrue(Files.exists(unpackedWebInfDir)); //should have unpacked WEB-INF
assertTrue(Files.exists(unpackedWebInfDir.resolve("WEB-INF").resolve("lib").resolve("alpha.jar")));
checkNoDuplicateJars(EXPECTED_JAR_NAMES, (URLClassLoader)context.getClassLoader());
}
/**
* Odd combination, but it has always been available: test that both the
* whole war can be extracted, _and_ WEB-INF separately.
*/
@Test
public void testShouldUnpackWarAndWebInf(WorkDir workDir) throws Exception
{
Path testPath = MavenPaths.targetTestDir("testSimple");
FS.ensureDirExists(testPath);
FS.ensureEmpty(testPath);
_server = new Server();
WebAppContext context = new WebAppContext();
context.setContextPath("/");
Path warPath = createWar(testPath, "test.war");
context.setExtractWAR(true);
context.setCopyWebDir(false);
context.setCopyWebInf(true);
context.setWar(warPath.toUri().toURL().toString());
_server.setHandler(context);
_server.start();
Path unpackedDir = context.getTempDirectory().toPath().resolve("webapp");
Path unpackedWebInfDir = context.getTempDirectory().toPath().resolve("webinf");
assertTrue(Files.exists(unpackedDir)); //should have unpacked whole war
assertTrue(Files.exists(unpackedWebInfDir)); //should have re-unpacked WEB-INF
assertTrue(Files.exists(unpackedWebInfDir.resolve("WEB-INF").resolve("lib").resolve("alpha.jar")));
}
/**
* Assert that for each of the expected jar names (stripped of any path info),
* there is only 1 actual jar url in the context classloader
*
* @param expected name of a jar without any preceding path info eg "foo.jar"
* @param classLoader the context classloader
*/
private void checkNoDuplicateJars(List<String> expected, URLClassLoader classLoader)
{
List<String> actual = new ArrayList<>();
for (URL u : classLoader.getURLs())
actual.add(u.toExternalForm());
for (String e : expected)
{
long count = actual.stream().filter(s -> s.endsWith(e)).count();
assertThat(count, equalTo(1L));
}
}
}