From e2753e6f5f040e99cd16f8d1f9d02f6ea2a257c5 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 15 Aug 2024 23:09:34 -0500 Subject: [PATCH] Add Missing changes to ee11 WebInfConfiguration (#12168) * Missing changes to WebInfConfiguration from commit 558da27c2dc4c189e47f4b64c4e0f9aa4fbaa453 * use better matchers for files assert Signed-off-by: Olivier Lamy --------- Signed-off-by: Olivier Lamy Co-authored-by: Olivier Lamy --- .../jetty/ee10/webapp/TempDirTest.java | 8 +- .../ee11/webapp/WebInfConfiguration.java | 127 ++---------------- .../jetty/ee11/webapp/TempDirTest.java | 8 +- .../eclipse/jetty/ee9/webapp/TempDirTest.java | 12 +- 4 files changed, 24 insertions(+), 131 deletions(-) diff --git a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/TempDirTest.java b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/TempDirTest.java index a0b7bdb3dfb..1bde6b24919 100644 --- a/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/TempDirTest.java +++ b/jetty-ee10/jetty-ee10-webapp/src/test/java/org/eclipse/jetty/ee10/webapp/TempDirTest.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.ee10.webapp; import java.io.File; -import java.nio.file.Files; import java.nio.file.Path; import jakarta.servlet.ServletContext; @@ -33,6 +32,7 @@ import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.io.FileMatchers.anExistingDirectory; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -85,7 +85,7 @@ public class TempDirTest WebAppContext webAppContext = new WebAppContext(); server.setHandler(webAppContext); Path tmpDir = path.resolve("foo_did_not_exist"); - assertThat(Files.exists(tmpDir), is(false)); + assertThat(tmpDir.toFile(), not(anExistingDirectory())); switch (type) { @@ -119,7 +119,7 @@ public class TempDirTest WebInfConfiguration webInfConfiguration = new WebInfConfiguration(); webInfConfiguration.resolveTempDirectory(webAppContext); File tempDirectory = webAppContext.getTempDirectory(); - assertThat(tempDirectory.exists(), is(true)); + assertThat(tempDirectory, anExistingDirectory()); assertThat(tempDirectory.getParentFile().toPath(), PathMatchers.isSame(tmpDir)); } @@ -200,7 +200,7 @@ public class TempDirTest _server.start(); File tempDirectory = webAppContext.getTempDirectory(); _server.stop(); - assertThat("Temp dir exists", !Files.exists(tempDirectory.toPath())); + assertThat(tempDirectory, not(anExistingDirectory())); assertNull(webAppContext.getTempDirectory()); } diff --git a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebInfConfiguration.java b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebInfConfiguration.java index 78ddf648c66..5c308727523 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebInfConfiguration.java +++ b/jetty-ee11/jetty-ee11-webapp/src/main/java/org/eclipse/jetty/ee11/webapp/WebInfConfiguration.java @@ -18,11 +18,8 @@ import java.io.IOException; import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; -import java.util.List; import jakarta.servlet.ServletContext; -import org.eclipse.jetty.server.Connector; -import org.eclipse.jetty.server.NetworkConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.FileID; import org.eclipse.jetty.util.IO; @@ -39,7 +36,6 @@ public class WebInfConfiguration extends AbstractConfiguration { private static final Logger LOG = LoggerFactory.getLogger(WebInfConfiguration.class); - public static final String TEMPDIR_CONFIGURED = "org.eclipse.jetty.tmpdirConfigured"; public static final String TEMPORARY_RESOURCE_BASE = "org.eclipse.jetty.webapp.tmpResourceBase"; public static final String ORIGINAL_RESOURCE_BASE = "org.eclipse.jetty.webapp.originalResourceBase"; @@ -89,10 +85,6 @@ public class WebInfConfiguration extends AbstractConfiguration @Override public void deconfigure(WebAppContext context) throws Exception { - //if it wasn't explicitly configured by the user, then unset it - if (!(context.getAttribute(TEMPDIR_CONFIGURED) instanceof Boolean tmpdirConfigured && tmpdirConfigured)) - context.setTempDirectory(null); - //reset the base resource back to what it was before we did any unpacking of resources Resource originalBaseResource = (Resource)context.removeAttribute(ORIGINAL_RESOURCE_BASE); context.setBaseResource(originalBaseResource); @@ -133,7 +125,6 @@ public class WebInfConfiguration extends AbstractConfiguration File tempDirectory = context.getTempDirectory(); if (tempDirectory != null) { - context.setAttribute(TEMPDIR_CONFIGURED, Boolean.TRUE); //the tmp dir was set explicitly return; } @@ -148,37 +139,14 @@ public class WebInfConfiguration extends AbstractConfiguration return; } - makeTempDirectory(context.getServer().getContext().getTempDirectory(), context); + context.makeTempDirectory(); } + @Deprecated (forRemoval = true, since = "12.0.12") public void makeTempDirectory(File parent, WebAppContext context) throws Exception { - if (parent == null || !parent.exists() || !parent.canWrite() || !parent.isDirectory()) - throw new IllegalStateException("Parent for temp dir not configured correctly: " + (parent == null ? "null" : "writeable=" + parent.canWrite())); - - boolean persistent = context.isTempDirectoryPersistent() || "work".equals(parent.toPath().getFileName().toString()); - - //Create a name for the webapp - String temp = getCanonicalNameForWebAppTmpDir(context); - File tmpDir; - if (persistent) - { - //if it is to be persisted, make sure it will be the same name - //by not using File.createTempFile, which appends random digits - tmpDir = new File(parent, temp); - } - else - { - // ensure dir will always be unique by having classlib generate random path name - tmpDir = Files.createTempDirectory(parent.toPath(), temp).toFile(); - tmpDir.deleteOnExit(); - } - - if (LOG.isDebugEnabled()) - LOG.debug("Set temp dir {}", tmpDir); - context.setTempDirectory(tmpDir); - context.setTempDirectoryPersistent(persistent); + context.makeTempDirectory(); } public void unpack(WebAppContext context) throws IOException @@ -395,91 +363,20 @@ public class WebInfConfiguration extends AbstractConfiguration * * @param context the context to get the canonical name from * @return the canonical name for the webapp temp directory + * @deprecated this method is no longer used */ + @Deprecated(forRemoval = true, since = "12.0.12") public static String getCanonicalNameForWebAppTmpDir(WebAppContext context) { - StringBuilder canonicalName = new StringBuilder(); - canonicalName.append("jetty-"); - - //get the host and the port from the first connector - Server server = context.getServer(); - if (server != null) - { - Connector[] connectors = server.getConnectors(); - - if (connectors.length > 0) - { - //Get the host - String host = null; - int port = 0; - if (connectors[0] instanceof NetworkConnector connector) - { - host = connector.getHost(); - port = connector.getLocalPort(); - if (port < 0) - port = connector.getPort(); - } - if (host == null) - host = "0.0.0.0"; - canonicalName.append(host); - canonicalName.append("-"); - canonicalName.append(port); - canonicalName.append("-"); - } - } - - // Resource base - try - { - Resource resource = context.getBaseResource(); - if (resource == null) - { - if (StringUtil.isBlank(context.getWar())) - throw new IllegalStateException("No resourceBase or war set for context"); - - // Set dir or WAR to resource - resource = context.newResource(context.getWar()); - } - - String resourceBaseName = getResourceBaseName(resource); - canonicalName.append(resourceBaseName); - canonicalName.append("-"); - } - catch (Exception e) - { - if (LOG.isDebugEnabled()) - LOG.debug("Can't get resource base name", e); - - canonicalName.append("-"); // empty resourceBaseName segment - } - - //Context name - String contextPath = context.getContextPath(); - contextPath = contextPath.replace('/', '_'); - contextPath = contextPath.replace('\\', '_'); - canonicalName.append(contextPath); - - //Virtual host (if there is one) - canonicalName.append("-"); - List vhosts = context.getVirtualHosts(); - if (vhosts == null || vhosts.size() <= 0) - canonicalName.append("any"); - else - canonicalName.append(vhosts.get(0)); - - // sanitize - for (int i = 0; i < canonicalName.length(); i++) - { - char c = canonicalName.charAt(i); - if (!Character.isJavaIdentifierPart(c) && "-.".indexOf(c) < 0) - canonicalName.setCharAt(i, '.'); - } - - canonicalName.append("-"); - - return StringUtil.sanitizeFileSystemName(canonicalName.toString()); + return context.getCanonicalNameForTmpDir(); } + /** + * @param resource the Resource for which to extract a short name + * @return extract a short name for the resource + * @deprecated this method is no longer needed + */ + @Deprecated(forRemoval = true, since = "12.0.12") protected static String getResourceBaseName(Resource resource) { // Use File System and File interface if present diff --git a/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/TempDirTest.java b/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/TempDirTest.java index 3104ce4d9fb..6ade861bce1 100644 --- a/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/TempDirTest.java +++ b/jetty-ee11/jetty-ee11-webapp/src/test/java/org/eclipse/jetty/ee11/webapp/TempDirTest.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.ee11.webapp; import java.io.File; -import java.nio.file.Files; import java.nio.file.Path; import jakarta.servlet.ServletContext; @@ -33,6 +32,7 @@ import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.io.FileMatchers.anExistingDirectory; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -85,7 +85,7 @@ public class TempDirTest WebAppContext webAppContext = new WebAppContext(); server.setHandler(webAppContext); Path tmpDir = path.resolve("foo_did_not_exist"); - assertThat(Files.exists(tmpDir), is(false)); + assertThat(tmpDir.toFile(), not(anExistingDirectory())); switch (type) { @@ -119,7 +119,7 @@ public class TempDirTest WebInfConfiguration webInfConfiguration = new WebInfConfiguration(); webInfConfiguration.resolveTempDirectory(webAppContext); File tempDirectory = webAppContext.getTempDirectory(); - assertThat(tempDirectory.exists(), is(true)); + assertThat(tempDirectory, is(anExistingDirectory())); assertThat(tempDirectory.getParentFile().toPath(), PathMatchers.isSame(tmpDir)); } @@ -200,7 +200,7 @@ public class TempDirTest _server.start(); File tempDirectory = webAppContext.getTempDirectory(); _server.stop(); - assertThat("Temp dir exists", !Files.exists(tempDirectory.toPath())); + assertThat(tempDirectory, not(anExistingDirectory())); assertNull(webAppContext.getTempDirectory()); } diff --git a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/TempDirTest.java b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/TempDirTest.java index 83b1f0e4d62..0b213efb739 100644 --- a/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/TempDirTest.java +++ b/jetty-ee9/jetty-ee9-webapp/src/test/java/org/eclipse/jetty/ee9/webapp/TempDirTest.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.ee9.webapp; import java.io.File; -import java.nio.file.Files; import java.nio.file.Path; import jakarta.servlet.ServletContext; @@ -24,20 +23,17 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.PathMatchers; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; -import org.eclipse.jetty.util.resource.FileSystemPool; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; 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.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.io.FileMatchers.anExistingDirectory; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -93,7 +89,7 @@ public class TempDirTest WebAppContext webAppContext = new WebAppContext(); _server.setHandler(webAppContext); Path tmpDir = jettyBase.resolve("foo_did_not_exist"); - assertFalse(Files.exists(tmpDir)); + assertThat(tmpDir.toFile(), not(anExistingDirectory())); switch (type) { @@ -128,7 +124,7 @@ public class TempDirTest WebInfConfiguration webInfConfiguration = new WebInfConfiguration(); webInfConfiguration.resolveTempDirectory(webAppContext); File tempDirectory = webAppContext.getTempDirectory(); - assertTrue(tempDirectory.exists()); + assertThat(tempDirectory, anExistingDirectory()); assertThat(tempDirectory.getParentFile().toPath(), is(tmpDir)); assertThat(webAppContext.getAttribute(ServletContext.TEMPDIR), is(webAppContext.getTempDirectory())); } @@ -169,7 +165,7 @@ public class TempDirTest assertThat(webAppContext.getAttribute(ServletContext.TEMPDIR), is(webAppContext.getTempDirectory())); _server.stop(); assertNull(webAppContext.getTempDirectory()); - assertThat("Temp dir exists", !Files.exists(tempDirectory.toPath())); + assertThat(tempDirectory, not(anExistingDirectory())); } @Test