This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/struts.git
commit 1986ef59ed87970faffa27abb22c54db88fe16f5 Author: Lukasz Lenart <[email protected]> AuthorDate: Fri Oct 23 15:30:57 2026 +0200 WW-5791 fix(boot): address Sonar findings in the starter The Tomcat base directory is created owner-only (rwx------) where the file system supports POSIX permissions, and the shutdown hook is registered inside the guarded start block so a failure there still closes the server. Settings keys get a _KEY suffix, the empty register() says why it is empty, and test lambdas keep a single throwing call. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../main/java/org/apache/struts2/boot/BootSettings.java | 14 +++++++------- .../apache/struts2/boot/RegistryConfigurationProvider.java | 1 + .../java/org/apache/struts2/boot/StrutsBootServer.java | 10 ++++++++-- .../java/org/apache/struts2/boot/ActionRegistryTest.java | 4 ++-- .../java/org/apache/struts2/boot/BootSettingsTest.java | 4 +++- .../struts2/boot/RegistryConfigurationProviderTest.java | 7 ++++--- .../java/org/apache/struts2/boot/StrutsBootTomcatTest.java | 13 ++++++++++++- 7 files changed, 37 insertions(+), 16 deletions(-) diff --git a/boot/src/main/java/org/apache/struts2/boot/BootSettings.java b/boot/src/main/java/org/apache/struts2/boot/BootSettings.java index 534389a79..8accb60d4 100644 --- a/boot/src/main/java/org/apache/struts2/boot/BootSettings.java +++ b/boot/src/main/java/org/apache/struts2/boot/BootSettings.java @@ -28,9 +28,9 @@ import java.util.Properties; public final class BootSettings { private static final String PREFIX = "struts.boot."; - private static final String PORT = PREFIX + "port"; - private static final String CONTEXT_PATH = PREFIX + "context-path"; - private static final String ADDRESS = PREFIX + "address"; + private static final String PORT_KEY = PREFIX + "port"; + private static final String CONTEXT_PATH_KEY = PREFIX + "context-path"; + private static final String ADDRESS_KEY = PREFIX + "address"; private static final String ARGUMENT_PREFIX = "--"; private final int port; @@ -54,9 +54,9 @@ public final class BootSettings { applyArgument(merged, arg); } return new BootSettings( - parsePort(merged.getProperty(PORT, "8080")), - normaliseContextPath(merged.getProperty(CONTEXT_PATH, "")), - blankToNull(merged.getProperty(ADDRESS))); + parsePort(merged.getProperty(PORT_KEY, "8080")), + normaliseContextPath(merged.getProperty(CONTEXT_PATH_KEY, "")), + blankToNull(merged.getProperty(ADDRESS_KEY))); } public int port() { @@ -101,7 +101,7 @@ public final class BootSettings { try { return Integer.parseInt(value.trim()); } catch (NumberFormatException e) { - throw new ConfigurationException("Invalid " + PORT + ": '" + value + "' is not a number", e); + throw new ConfigurationException("Invalid " + PORT_KEY + ": '" + value + "' is not a number", e); } } diff --git a/boot/src/main/java/org/apache/struts2/boot/RegistryConfigurationProvider.java b/boot/src/main/java/org/apache/struts2/boot/RegistryConfigurationProvider.java index 699525210..dcce8a5b8 100644 --- a/boot/src/main/java/org/apache/struts2/boot/RegistryConfigurationProvider.java +++ b/boot/src/main/java/org/apache/struts2/boot/RegistryConfigurationProvider.java @@ -74,6 +74,7 @@ public class RegistryConfigurationProvider implements ServletContextAwareConfigu @Override public void register(ContainerBuilder builder, LocatableProperties props) { + // the registry contributes packages only, no beans or constants } @Override diff --git a/boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java b/boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java index 14763c81e..21bd6c4b6 100644 --- a/boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java +++ b/boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java @@ -33,8 +33,10 @@ import org.apache.tomcat.util.descriptor.web.FilterMap; import java.io.IOException; import java.io.UncheckedIOException; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; import java.util.Comparator; import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; @@ -78,6 +80,7 @@ public final class StrutsBootServer implements AutoCloseable { tomcat.start(); assertContextStarted(context, filter); server.openConnector(); + Runtime.getRuntime().addShutdownHook(server.shutdownHook); } catch (Exception e) { ConfigurationException failure = e instanceof ConfigurationException configurationException ? configurationException @@ -88,7 +91,6 @@ public final class StrutsBootServer implements AutoCloseable { server.closeAfterFailure(e); throw e; } - Runtime.getRuntime().addShutdownHook(server.shutdownHook); return server; } @@ -196,7 +198,11 @@ public final class StrutsBootServer implements AutoCloseable { private static Path createBaseDir() { try { - return Files.createTempDirectory("struts-boot-" + UUID.randomUUID()); + String prefix = "struts-boot-" + UUID.randomUUID(); + if (FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) { + return Files.createTempDirectory(prefix, PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------"))); + } + return Files.createTempDirectory(prefix); } catch (IOException e) { throw new ConfigurationException("Cannot create the Tomcat base directory", e); } diff --git a/boot/src/test/java/org/apache/struts2/boot/ActionRegistryTest.java b/boot/src/test/java/org/apache/struts2/boot/ActionRegistryTest.java index 06e7d067f..5ef5a3ef2 100644 --- a/boot/src/test/java/org/apache/struts2/boot/ActionRegistryTest.java +++ b/boot/src/test/java/org/apache/struts2/boot/ActionRegistryTest.java @@ -73,8 +73,8 @@ public class ActionRegistryTest { @Test public void testNonFtlTemplateThrows() { - ActionRegistry r = new ActionRegistry(); - assertThatThrownBy(() -> r.action("hello", Object.class).success("hello.jsp")) + ActionDefinition hello = new ActionRegistry().action("hello", Object.class); + assertThatThrownBy(() -> hello.success("hello.jsp")) .isInstanceOf(ConfigurationException.class).hasMessageContaining("FreeMarker"); } diff --git a/boot/src/test/java/org/apache/struts2/boot/BootSettingsTest.java b/boot/src/test/java/org/apache/struts2/boot/BootSettingsTest.java index 5f7b836bc..593be770f 100644 --- a/boot/src/test/java/org/apache/struts2/boot/BootSettingsTest.java +++ b/boot/src/test/java/org/apache/struts2/boot/BootSettingsTest.java @@ -114,7 +114,9 @@ public class BootSettingsTest { @Test public void testNonNumericPortThrows() { - assertThatThrownBy(() -> BootSettings.load(new Properties(), new String[]{"--struts.boot.port=abc"})) + Properties fileProperties = new Properties(); + String[] args = {"--struts.boot.port=abc"}; + assertThatThrownBy(() -> BootSettings.load(fileProperties, args)) .isInstanceOf(ConfigurationException.class); } } diff --git a/boot/src/test/java/org/apache/struts2/boot/RegistryConfigurationProviderTest.java b/boot/src/test/java/org/apache/struts2/boot/RegistryConfigurationProviderTest.java index c19342c44..7a4b2460c 100644 --- a/boot/src/test/java/org/apache/struts2/boot/RegistryConfigurationProviderTest.java +++ b/boot/src/test/java/org/apache/struts2/boot/RegistryConfigurationProviderTest.java @@ -152,7 +152,7 @@ public class RegistryConfigurationProviderTest { } @Test - public void testNoStackMeansPackageDefault() throws Exception { + public void testNoStackMeansPackageDefault() { Configuration configuration = load(usersRegistry()); assertThat(action(configuration, "struts-boot/admin", "users").getInterceptors()).isEmpty(); @@ -246,13 +246,14 @@ public class RegistryConfigurationProviderTest { @Test public void testClashWithXmlThrows() { - assertThatThrownBy(() -> load(usersRegistry(), "struts-boot-clash.xml")) + ActionRegistry registry = usersRegistry(); + assertThatThrownBy(() -> load(registry, "struts-boot-clash.xml")) .isInstanceOf(ConfigurationException.class) .rootCause().hasMessageContaining("xml-pkg").hasMessageContaining("users"); } @Test - public void testSameNamespaceAsXmlWithDistinctNamesLoads() throws Exception { + public void testSameNamespaceAsXmlWithDistinctNamesLoads() { ActionRegistry registry = new ActionRegistry(); registry.namespace("/admin", ns -> ns.action("list", UserAction.class).method("list").success("admin/users.ftl")); diff --git a/boot/src/test/java/org/apache/struts2/boot/StrutsBootTomcatTest.java b/boot/src/test/java/org/apache/struts2/boot/StrutsBootTomcatTest.java index 7a8397f25..c3009eb25 100644 --- a/boot/src/test/java/org/apache/struts2/boot/StrutsBootTomcatTest.java +++ b/boot/src/test/java/org/apache/struts2/boot/StrutsBootTomcatTest.java @@ -38,8 +38,10 @@ import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -168,7 +170,8 @@ public class StrutsBootTomcatTest { public void testBusyPortFailsCleanly() throws Exception { Set<Path> before = bootTempDirs(); try (ServerSocket busy = new ServerSocket(0)) { - assertThatThrownBy(() -> StrutsBoot.start(HelloApp.class, "--struts.boot.port=" + busy.getLocalPort())) + String portArgument = "--struts.boot.port=" + busy.getLocalPort(); + assertThatThrownBy(() -> StrutsBoot.start(HelloApp.class, portArgument)) .isInstanceOf(ConfigurationException.class); } @@ -199,6 +202,14 @@ public class StrutsBootTomcatTest { assertThat(Files.exists(baseDir)).isFalse(); } + @Test + public void testBaseDirIsOwnerOnly() throws IOException { + assumeTrue(FileSystems.getDefault().supportedFileAttributeViews().contains("posix")); + + assertThat(Files.getPosixFilePermissions(server.baseDir())) + .isEqualTo(PosixFilePermissions.fromString("rwx------")); + } + private static HttpResponse<String> get(String url) throws IOException, InterruptedException { return CLIENT.send(HttpRequest.newBuilder(URI.create(url)).GET().build(), HttpResponse.BodyHandlers.ofString()); }
