Skip to content

Commit

Permalink
Issue #11574 war should not be extracted unless configured to do so (#…
Browse files Browse the repository at this point in the history 
…11575)

* Issue #11574 war should not be extracted unless configured to do so
  • Loading branch information
@janbartel @sbordet
janbartel authored and sbordet committed Mar 28, 2024
1 parent 2c5534f commit 302ae64
Show file tree
Hide file tree
Showing 7 changed files with 384 additions and 53 deletions.
23 changes: 2 additions & 21 deletions ...ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebAppClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
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;
Expand Down Expand Up @@ -283,27 +284,7 @@ public void addJars(Resource libs)
{
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
Expand Down
6 changes: 3 additions & 3 deletions ...10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/WebInfConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,19 @@ public void unpack(WebAppContext context) throws IOException
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;
}

// If we should extract or the URL is still not usable
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.
Expand Down
12 changes: 6 additions & 6 deletions ...ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebAppContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -872,9 +872,9 @@ public void testRestartWebApp(WorkDir workDir) throws Exception
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));
Expand All @@ -892,9 +892,9 @@ public void testRestartWebApp(WorkDir workDir) throws Exception
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));
}
Expand Down
186 changes: 186 additions & 0 deletions ...etty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/WebInfConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,79 @@
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
*/
@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()
{
Expand Down Expand Up @@ -80,4 +131,139 @@ public void testPathGetResourceBaseName(String basePath, String expectedName, Wo
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));
}
}
}
22 changes: 2 additions & 20 deletions jetty-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebAppClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
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;
Expand Down Expand Up @@ -287,26 +288,7 @@ public void addJars(Resource libs)
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
Expand Down
6 changes: 3 additions & 3 deletions ...-ee9/jetty-ee9-webapp/src/main/java/org/eclipse/jetty/ee9/webapp/WebInfConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,11 @@ public void unpack(WebAppContext context) throws IOException
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;
}

Expand All @@ -233,7 +233,7 @@ public void unpack(WebAppContext context) throws IOException
(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.
Expand Down
Loading

0 comments on commit 302ae64

Please sign in to comment.