[
https://issues.apache.org/jira/browse/WW-5791?focusedWorklogId=1049693&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1049693
]
ASF GitHub Bot logged work on WW-5791:
--------------------------------------
Author: ASF GitHub Bot
Created on: 23/Oct/26 13:46
Start Date: 23/Oct/26 13:46
Worklog Time Spent: 10m
Work Description: Copilot commented on code in PR #2053:
URL: https://github.com/apache/struts/pull/2053#discussion_r4360207309
##########
boot/src/main/java/org/apache/struts2/boot/ActionDefinition.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.boot;
+
+import org.apache.struts2.config.ConfigurationException;
+import org.apache.struts2.interceptor.httpmethod.HttpMethod;
+
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+public final class ActionDefinition {
+
+ public record ResultDefinition(Kind kind, String value) {
+
+ public enum Kind {
+ TEMPLATE, REDIRECT_ACTION
+ }
+ }
+
+ private final String name;
+ private final Class<?> actionClass;
+ private final Map<String, ResultDefinition> results = new
LinkedHashMap<>();
+ private final Set<HttpMethod> httpMethods =
EnumSet.noneOf(HttpMethod.class);
+ private String methodName = "execute";
+ private String interceptorStack;
+
+ ActionDefinition(String name, Class<?> actionClass) {
+ this.name = name;
+ this.actionClass = actionClass;
+ }
+
+ public ActionDefinition method(String methodName) {
+ this.methodName = methodName;
+ return this;
+ }
Review Comment:
The public fluent API allows `method(null)` and `result(null, "x.ftl")` (or
blank names). Those invalid values can later trigger less-clear failures (e.g.,
NPEs or invalid Struts config) when building `ActionConfig`/`ResultConfig`. Add
validation in these setters so invalid method/result names fail immediately
with a `ConfigurationException` that points to the action and the invalid value.
##########
boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java:
##########
@@ -0,0 +1,244 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.boot;
+
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.ServletException;
+import org.apache.catalina.Context;
+import org.apache.catalina.LifecycleState;
+import org.apache.catalina.connector.Connector;
+import org.apache.catalina.servlets.DefaultServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.valves.ErrorReportValve;
+import org.apache.struts2.config.ConfigurationException;
+import org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter;
+import org.apache.tomcat.util.descriptor.web.FilterDef;
+import org.apache.tomcat.util.descriptor.web.FilterMap;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
+
+public final class StrutsBootServer implements AutoCloseable {
+
+ private static final String FILTER_NAME = "struts";
+
+ private final Tomcat tomcat;
+ private final Path baseDir;
+ private final BootSettings settings;
+ private final AtomicBoolean closed = new AtomicBoolean();
+ private final Thread shutdownHook = new Thread(this::close,
"struts-boot-shutdown");
+ private Connector connector;
+
+ private StrutsBootServer(Tomcat tomcat, Path baseDir, BootSettings
settings) {
+ this.tomcat = tomcat;
+ this.baseDir = baseDir;
+ this.settings = settings;
+ }
+
+ static StrutsBootServer start(ActionRegistry registry, BootSettings
settings) {
+ Path baseDir = createBaseDir();
+ Tomcat tomcat = new Tomcat();
+ StrutsBootServer server = new StrutsBootServer(tomcat, baseDir,
settings);
+ CapturingStrutsFilter filter = new CapturingStrutsFilter();
+ try {
+ tomcat.setBaseDir(baseDir.toString());
+ hideErrorReportDetails(tomcat);
+ Path docBase = Files.createDirectory(baseDir.resolve("webapp"));
+ Context context = tomcat.addContext(settings.contextPath(),
docBase.toString());
+ context.addServletContainerInitializer(
+ (classes, servletContext) ->
servletContext.setAttribute(ActionRegistry.SERVLET_CONTEXT_ATTRIBUTE, registry),
+ null);
+ Tomcat.addServlet(context, "default", new DefaultServlet());
+ context.addServletMappingDecoded("/", "default");
+ addStrutsFilter(context, filter);
+
+ tomcat.start();
+ assertContextStarted(context, filter);
+ server.openConnector();
+ } catch (Exception e) {
+ ConfigurationException failure = e instanceof
ConfigurationException configurationException
+ ? configurationException
+ : new ConfigurationException("Struts Boot failed to start:
" + e.getMessage(), e);
+ server.closeAfterFailure(failure);
+ throw failure;
+ } catch (Error e) {
+ server.closeAfterFailure(e);
+ throw e;
+ }
+ Runtime.getRuntime().addShutdownHook(server.shutdownHook);
+ return server;
+ }
+
+ public int port() {
+ return connector.getLocalPort();
+ }
+
+ public String baseUrl() {
+ return "http://"; + urlHost() + ":" + port() + settings.contextPath();
+ }
+
+ public Path baseDir() {
+ return baseDir;
+ }
+
+ public void await() {
+ tomcat.getServer().await();
+ }
+
+ @Override
+ public void close() {
+ if (!closed.compareAndSet(false, true)) {
+ return;
+ }
+ try {
+ tomcat.stop();
+ tomcat.destroy();
+ } catch (Exception e) {
+ throw new ConfigurationException("Struts Boot failed to stop
Tomcat", e);
+ } finally {
+ deleteRecursively(baseDir);
+ removeShutdownHook();
+ }
+ }
+
+ private void closeAfterFailure(Throwable failure) {
+ try {
+ close();
+ } catch (RuntimeException closeFailure) {
+ failure.addSuppressed(closeFailure);
+ }
+ }
+
+ private void openConnector() {
+ Connector created = new Connector();
+ created.setPort(settings.port());
+ settings.address().ifPresent(address -> {
+ if (!created.setProperty("address", address)) {
+ throw new ConfigurationException("Invalid struts.boot.address:
'" + address + "'");
+ }
+ });
+ connector = created;
+ tomcat.setConnector(created);
+ if (created.getState() != LifecycleState.STARTED ||
created.getLocalPort() < 0) {
+ throw new ConfigurationException("Struts Boot failed to start:
connector state "
+ + created.getState() + " on port " + settings.port());
+ }
+ }
Review Comment:
`openConnector()` creates and attaches the Tomcat `Connector` *after*
`tomcat.start()`, but never starts it. In this flow `created.getState()` is
expected to remain `NEW` and `getLocalPort()` `-1`, so `port()`/`baseUrl()` can
be wrong or startup can fail unexpectedly. Configure and attach the connector
before calling `tomcat.start()` (or explicitly add/start the connector via the
Tomcat `Service`), and only then read `getLocalPort()` for port `0` cases.
##########
boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java:
##########
@@ -0,0 +1,244 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.boot;
+
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.ServletException;
+import org.apache.catalina.Context;
+import org.apache.catalina.LifecycleState;
+import org.apache.catalina.connector.Connector;
+import org.apache.catalina.servlets.DefaultServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.valves.ErrorReportValve;
+import org.apache.struts2.config.ConfigurationException;
+import org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter;
+import org.apache.tomcat.util.descriptor.web.FilterDef;
+import org.apache.tomcat.util.descriptor.web.FilterMap;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
+
+public final class StrutsBootServer implements AutoCloseable {
+
+ private static final String FILTER_NAME = "struts";
+
+ private final Tomcat tomcat;
+ private final Path baseDir;
+ private final BootSettings settings;
+ private final AtomicBoolean closed = new AtomicBoolean();
+ private final Thread shutdownHook = new Thread(this::close,
"struts-boot-shutdown");
+ private Connector connector;
+
+ private StrutsBootServer(Tomcat tomcat, Path baseDir, BootSettings
settings) {
+ this.tomcat = tomcat;
+ this.baseDir = baseDir;
+ this.settings = settings;
+ }
+
+ static StrutsBootServer start(ActionRegistry registry, BootSettings
settings) {
+ Path baseDir = createBaseDir();
+ Tomcat tomcat = new Tomcat();
+ StrutsBootServer server = new StrutsBootServer(tomcat, baseDir,
settings);
+ CapturingStrutsFilter filter = new CapturingStrutsFilter();
+ try {
+ tomcat.setBaseDir(baseDir.toString());
+ hideErrorReportDetails(tomcat);
+ Path docBase = Files.createDirectory(baseDir.resolve("webapp"));
+ Context context = tomcat.addContext(settings.contextPath(),
docBase.toString());
+ context.addServletContainerInitializer(
+ (classes, servletContext) ->
servletContext.setAttribute(ActionRegistry.SERVLET_CONTEXT_ATTRIBUTE, registry),
+ null);
+ Tomcat.addServlet(context, "default", new DefaultServlet());
+ context.addServletMappingDecoded("/", "default");
+ addStrutsFilter(context, filter);
+
+ tomcat.start();
+ assertContextStarted(context, filter);
+ server.openConnector();
+ } catch (Exception e) {
+ ConfigurationException failure = e instanceof
ConfigurationException configurationException
Review Comment:
`openConnector()` creates and attaches the Tomcat `Connector` *after*
`tomcat.start()`, but never starts it. In this flow `created.getState()` is
expected to remain `NEW` and `getLocalPort()` `-1`, so `port()`/`baseUrl()` can
be wrong or startup can fail unexpectedly. Configure and attach the connector
before calling `tomcat.start()` (or explicitly add/start the connector via the
Tomcat `Service`), and only then read `getLocalPort()` for port `0` cases.
##########
boot/src/main/java/org/apache/struts2/boot/ActionDefinition.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.boot;
+
+import org.apache.struts2.config.ConfigurationException;
+import org.apache.struts2.interceptor.httpmethod.HttpMethod;
+
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+public final class ActionDefinition {
+
+ public record ResultDefinition(Kind kind, String value) {
+
+ public enum Kind {
+ TEMPLATE, REDIRECT_ACTION
+ }
+ }
+
+ private final String name;
+ private final Class<?> actionClass;
+ private final Map<String, ResultDefinition> results = new
LinkedHashMap<>();
+ private final Set<HttpMethod> httpMethods =
EnumSet.noneOf(HttpMethod.class);
+ private String methodName = "execute";
+ private String interceptorStack;
+
+ ActionDefinition(String name, Class<?> actionClass) {
+ this.name = name;
+ this.actionClass = actionClass;
+ }
+
+ public ActionDefinition method(String methodName) {
+ this.methodName = methodName;
+ return this;
+ }
+
+ public ActionDefinition success(String templatePath) {
+ return result("success", templatePath);
+ }
+
+ public ActionDefinition result(String name, String templatePath) {
+ if (templatePath == null || !templatePath.endsWith(".ftl")) {
+ throw new ConfigurationException("Action '" + this.name + "'
result '" + name + "' uses '" + templatePath
+ + "': Struts Boot renders FreeMarker templates only, the
path must end with .ftl");
+ }
+ results.put(name, new ResultDefinition(ResultDefinition.Kind.TEMPLATE,
templatePath));
+ return this;
+ }
Review Comment:
The public fluent API allows `method(null)` and `result(null, "x.ftl")` (or
blank names). Those invalid values can later trigger less-clear failures (e.g.,
NPEs or invalid Struts config) when building `ActionConfig`/`ResultConfig`. Add
validation in these setters so invalid method/result names fail immediately
with a `ConfigurationException` that points to the action and the invalid value.
##########
boot/src/main/java/org/apache/struts2/boot/BootSettings.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.boot;
+
+import org.apache.struts2.config.ConfigurationException;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Optional;
+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 ARGUMENT_PREFIX = "--";
+
+ private final int port;
+ private final String contextPath;
+ private final String address;
+
+ private BootSettings(int port, String contextPath, String address) {
+ this.port = port;
+ this.contextPath = contextPath;
+ this.address = address;
+ }
+
+ public static BootSettings load(String[] args) {
+ return load(readStrutsProperties(), args);
+ }
+
+ public static BootSettings load(Properties fileProperties, String[] args) {
+ Properties merged = new Properties();
+ merged.putAll(fileProperties);
+ for (String arg : args) {
+ applyArgument(merged, arg);
+ }
+ return new BootSettings(
+ parsePort(merged.getProperty(PORT, "8080")),
+ normaliseContextPath(merged.getProperty(CONTEXT_PATH, "")),
+ blankToNull(merged.getProperty(ADDRESS)));
+ }
+
+ public int port() {
+ return port;
+ }
+
+ public String contextPath() {
+ return contextPath;
+ }
+
+ public Optional<String> address() {
+ return Optional.ofNullable(address);
+ }
+
+ private static void applyArgument(Properties target, String arg) {
+ if (!arg.startsWith(ARGUMENT_PREFIX + PREFIX)) {
+ return;
+ }
+ int equals = arg.indexOf('=');
+ if (equals < 0) {
+ return;
+ }
+ target.setProperty(arg.substring(ARGUMENT_PREFIX.length(), equals),
arg.substring(equals + 1));
+ }
+
+ private static String normaliseContextPath(String value) {
+ String path = value.trim();
+ while (path.endsWith("/")) {
+ path = path.substring(0, path.length() - 1);
+ }
+ if (path.isEmpty()) {
+ return "";
+ }
+ return path.startsWith("/") ? path : "/" + path;
+ }
+
+ private static String blankToNull(String value) {
+ return value == null || value.isBlank() ? null : value.trim();
+ }
+
+ private static int parsePort(String value) {
+ try {
+ return Integer.parseInt(value.trim());
+ } catch (NumberFormatException e) {
+ throw new ConfigurationException("Invalid " + PORT + ": '" + value
+ "' is not a number", e);
+ }
+ }
Review Comment:
`parsePort` accepts negative values or values > 65535, which will later fail
during socket bind with a less targeted error. Validate allowed port range
(0–65535) here and throw a `ConfigurationException` with a clear message when
out of range.
##########
boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java:
##########
@@ -0,0 +1,244 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.boot;
+
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.ServletException;
+import org.apache.catalina.Context;
+import org.apache.catalina.LifecycleState;
+import org.apache.catalina.connector.Connector;
+import org.apache.catalina.servlets.DefaultServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.valves.ErrorReportValve;
+import org.apache.struts2.config.ConfigurationException;
+import org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter;
+import org.apache.tomcat.util.descriptor.web.FilterDef;
+import org.apache.tomcat.util.descriptor.web.FilterMap;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
+
+public final class StrutsBootServer implements AutoCloseable {
+
+ private static final String FILTER_NAME = "struts";
+
+ private final Tomcat tomcat;
+ private final Path baseDir;
+ private final BootSettings settings;
+ private final AtomicBoolean closed = new AtomicBoolean();
+ private final Thread shutdownHook = new Thread(this::close,
"struts-boot-shutdown");
+ private Connector connector;
+
+ private StrutsBootServer(Tomcat tomcat, Path baseDir, BootSettings
settings) {
+ this.tomcat = tomcat;
+ this.baseDir = baseDir;
+ this.settings = settings;
+ }
+
+ static StrutsBootServer start(ActionRegistry registry, BootSettings
settings) {
+ Path baseDir = createBaseDir();
+ Tomcat tomcat = new Tomcat();
+ StrutsBootServer server = new StrutsBootServer(tomcat, baseDir,
settings);
+ CapturingStrutsFilter filter = new CapturingStrutsFilter();
+ try {
+ tomcat.setBaseDir(baseDir.toString());
+ hideErrorReportDetails(tomcat);
+ Path docBase = Files.createDirectory(baseDir.resolve("webapp"));
+ Context context = tomcat.addContext(settings.contextPath(),
docBase.toString());
+ context.addServletContainerInitializer(
+ (classes, servletContext) ->
servletContext.setAttribute(ActionRegistry.SERVLET_CONTEXT_ATTRIBUTE, registry),
+ null);
+ Tomcat.addServlet(context, "default", new DefaultServlet());
+ context.addServletMappingDecoded("/", "default");
+ addStrutsFilter(context, filter);
+
+ tomcat.start();
+ assertContextStarted(context, filter);
+ server.openConnector();
+ } catch (Exception e) {
+ ConfigurationException failure = e instanceof
ConfigurationException configurationException
+ ? configurationException
+ : new ConfigurationException("Struts Boot failed to start:
" + e.getMessage(), e);
+ server.closeAfterFailure(failure);
+ throw failure;
+ } catch (Error e) {
+ server.closeAfterFailure(e);
+ throw e;
+ }
+ Runtime.getRuntime().addShutdownHook(server.shutdownHook);
+ return server;
+ }
+
+ public int port() {
+ return connector.getLocalPort();
+ }
+
+ public String baseUrl() {
+ return "http://"; + urlHost() + ":" + port() + settings.contextPath();
+ }
+
+ public Path baseDir() {
+ return baseDir;
+ }
+
+ public void await() {
+ tomcat.getServer().await();
+ }
+
+ @Override
+ public void close() {
+ if (!closed.compareAndSet(false, true)) {
+ return;
+ }
+ try {
+ tomcat.stop();
+ tomcat.destroy();
+ } catch (Exception e) {
+ throw new ConfigurationException("Struts Boot failed to stop
Tomcat", e);
+ } finally {
+ deleteRecursively(baseDir);
+ removeShutdownHook();
+ }
+ }
+
+ private void closeAfterFailure(Throwable failure) {
Review Comment:
`close()` can throw a `ConfigurationException`, but it is invoked directly
as a JVM shutdown hook (`new Thread(this::close, ...)`). If Tomcat stop/destroy
fails during shutdown, this will raise an uncaught exception on shutdown (and
may create confusing shutdown logs). Consider making the shutdown hook
catch-and-suppress failures (still attempting cleanup), and keeping `close()`
best-effort for resource cleanup (especially for `AutoCloseable` usage) rather
than throwing on stop/destroy failures.
##########
boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java:
##########
@@ -0,0 +1,244 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.boot;
+
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.ServletException;
+import org.apache.catalina.Context;
+import org.apache.catalina.LifecycleState;
+import org.apache.catalina.connector.Connector;
+import org.apache.catalina.servlets.DefaultServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.valves.ErrorReportValve;
+import org.apache.struts2.config.ConfigurationException;
+import org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter;
+import org.apache.tomcat.util.descriptor.web.FilterDef;
+import org.apache.tomcat.util.descriptor.web.FilterMap;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
+
+public final class StrutsBootServer implements AutoCloseable {
+
+ private static final String FILTER_NAME = "struts";
+
+ private final Tomcat tomcat;
+ private final Path baseDir;
+ private final BootSettings settings;
+ private final AtomicBoolean closed = new AtomicBoolean();
+ private final Thread shutdownHook = new Thread(this::close,
"struts-boot-shutdown");
+ private Connector connector;
+
Review Comment:
`close()` can throw a `ConfigurationException`, but it is invoked directly
as a JVM shutdown hook (`new Thread(this::close, ...)`). If Tomcat stop/destroy
fails during shutdown, this will raise an uncaught exception on shutdown (and
may create confusing shutdown logs). Consider making the shutdown hook
catch-and-suppress failures (still attempting cleanup), and keeping `close()`
best-effort for resource cleanup (especially for `AutoCloseable` usage) rather
than throwing on stop/destroy failures.
##########
boot/src/main/java/org/apache/struts2/boot/StrutsBootServer.java:
##########
@@ -0,0 +1,244 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.boot;
+
+import jakarta.servlet.FilterConfig;
+import jakarta.servlet.ServletException;
+import org.apache.catalina.Context;
+import org.apache.catalina.LifecycleState;
+import org.apache.catalina.connector.Connector;
+import org.apache.catalina.servlets.DefaultServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.valves.ErrorReportValve;
+import org.apache.struts2.config.ConfigurationException;
+import org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter;
+import org.apache.tomcat.util.descriptor.web.FilterDef;
+import org.apache.tomcat.util.descriptor.web.FilterMap;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
+
+public final class StrutsBootServer implements AutoCloseable {
+
+ private static final String FILTER_NAME = "struts";
+
+ private final Tomcat tomcat;
+ private final Path baseDir;
+ private final BootSettings settings;
+ private final AtomicBoolean closed = new AtomicBoolean();
+ private final Thread shutdownHook = new Thread(this::close,
"struts-boot-shutdown");
+ private Connector connector;
+
+ private StrutsBootServer(Tomcat tomcat, Path baseDir, BootSettings
settings) {
+ this.tomcat = tomcat;
+ this.baseDir = baseDir;
+ this.settings = settings;
+ }
+
+ static StrutsBootServer start(ActionRegistry registry, BootSettings
settings) {
+ Path baseDir = createBaseDir();
+ Tomcat tomcat = new Tomcat();
+ StrutsBootServer server = new StrutsBootServer(tomcat, baseDir,
settings);
+ CapturingStrutsFilter filter = new CapturingStrutsFilter();
+ try {
+ tomcat.setBaseDir(baseDir.toString());
+ hideErrorReportDetails(tomcat);
+ Path docBase = Files.createDirectory(baseDir.resolve("webapp"));
+ Context context = tomcat.addContext(settings.contextPath(),
docBase.toString());
+ context.addServletContainerInitializer(
+ (classes, servletContext) ->
servletContext.setAttribute(ActionRegistry.SERVLET_CONTEXT_ATTRIBUTE, registry),
+ null);
+ Tomcat.addServlet(context, "default", new DefaultServlet());
+ context.addServletMappingDecoded("/", "default");
+ addStrutsFilter(context, filter);
+
+ tomcat.start();
+ assertContextStarted(context, filter);
+ server.openConnector();
+ } catch (Exception e) {
+ ConfigurationException failure = e instanceof
ConfigurationException configurationException
+ ? configurationException
+ : new ConfigurationException("Struts Boot failed to start:
" + e.getMessage(), e);
+ server.closeAfterFailure(failure);
+ throw failure;
+ } catch (Error e) {
+ server.closeAfterFailure(e);
+ throw e;
+ }
+ Runtime.getRuntime().addShutdownHook(server.shutdownHook);
+ return server;
+ }
+
+ public int port() {
+ return connector.getLocalPort();
+ }
+
+ public String baseUrl() {
+ return "http://"; + urlHost() + ":" + port() + settings.contextPath();
+ }
+
+ public Path baseDir() {
+ return baseDir;
+ }
+
+ public void await() {
+ tomcat.getServer().await();
+ }
+
+ @Override
+ public void close() {
+ if (!closed.compareAndSet(false, true)) {
+ return;
+ }
+ try {
+ tomcat.stop();
+ tomcat.destroy();
+ } catch (Exception e) {
+ throw new ConfigurationException("Struts Boot failed to stop
Tomcat", e);
+ } finally {
+ deleteRecursively(baseDir);
+ removeShutdownHook();
+ }
+ }
+
+ private void closeAfterFailure(Throwable failure) {
+ try {
+ close();
+ } catch (RuntimeException closeFailure) {
+ failure.addSuppressed(closeFailure);
+ }
+ }
+
+ private void openConnector() {
+ Connector created = new Connector();
+ created.setPort(settings.port());
+ settings.address().ifPresent(address -> {
+ if (!created.setProperty("address", address)) {
+ throw new ConfigurationException("Invalid struts.boot.address:
'" + address + "'");
+ }
+ });
+ connector = created;
+ tomcat.setConnector(created);
+ if (created.getState() != LifecycleState.STARTED ||
created.getLocalPort() < 0) {
+ throw new ConfigurationException("Struts Boot failed to start:
connector state "
+ + created.getState() + " on port " + settings.port());
+ }
+ }
+
+ private String urlHost() {
+ return settings.address()
+ .filter(address -> !address.equals("0.0.0.0") &&
!address.equals("::"))
+ .map(address -> address.contains(":") ? "[" + address + "]" :
address)
+ .orElse("127.0.0.1");
+ }
+
+ private void removeShutdownHook() {
+ if (Thread.currentThread() == shutdownHook) {
+ return;
+ }
+ try {
+ Runtime.getRuntime().removeShutdownHook(shutdownHook);
+ } catch (IllegalStateException ignored) {
+ // the JVM is already shutting down
+ }
+ }
+
+ private static void assertContextStarted(Context context,
CapturingStrutsFilter filter) {
+ Throwable initFailure = filter.failure.get();
+ if (initFailure != null || context.getState() !=
LifecycleState.STARTED) {
+ String reason = initFailure == null ? "context state " +
context.getState() : rootMessage(initFailure);
+ throw new ConfigurationException("Struts Boot failed to start: " +
reason, initFailure);
+ }
+ }
+
+ private static void hideErrorReportDetails(Tomcat tomcat) {
+ ErrorReportValve valve = new ErrorReportValve();
+ valve.setShowReport(false);
+ valve.setShowServerInfo(false);
+ tomcat.getHost().getPipeline().addValve(valve);
+ }
+
+ private static void addStrutsFilter(Context context, CapturingStrutsFilter
filter) {
+ FilterDef filterDef = new FilterDef();
+ filterDef.setFilterName(FILTER_NAME);
+ filterDef.setFilter(filter);
+ filterDef.setFilterClass(filter.getClass().getName());
+ filterDef.addInitParameter("configProviders",
RegistryConfigurationProvider.class.getName());
+ context.addFilterDef(filterDef);
+ FilterMap filterMap = new FilterMap();
+ filterMap.setFilterName(FILTER_NAME);
+ filterMap.addURLPattern("/*");
+ context.addFilterMap(filterMap);
+ }
+
+ private static Path createBaseDir() {
+ try {
+ return Files.createTempDirectory("struts-boot-" +
UUID.randomUUID());
+ } catch (IOException e) {
+ throw new ConfigurationException("Cannot create the Tomcat base
directory", e);
+ }
+ }
+
+ private static void deleteRecursively(Path dir) {
+ if (!Files.exists(dir)) {
+ return;
+ }
+ try (Stream<Path> paths = Files.walk(dir)) {
+ paths.sorted(Comparator.reverseOrder()).forEach(path -> {
+ try {
+ Files.delete(path);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
+
+ private static String rootMessage(Throwable throwable) {
+ Throwable root = throwable;
+ while (root.getCause() != null && root.getCause() != root) {
+ root = root.getCause();
+ }
Review Comment:
Cleanup currently throws `UncheckedIOException` if any file deletion fails
(e.g., transient Windows file locks), which can cause shutdown/close to fail
even though it’s primarily a cleanup path. Consider making recursive deletion
best-effort (attempt all deletions, optionally collecting failures) and avoid
throwing from cleanup unless the caller explicitly needs strict cleanup
semantics.
Issue Time Tracking
-------------------
Worklog Id: (was: 1049693)
Time Spent: 40m (was: 0.5h)
> Struts Boot: run a Struts application from main() with an embedded container
> and programmatic action registration
> -----------------------------------------------------------------------------------------------------------------
>
> Key: WW-5791
> URL: https://issues.apache.org/jira/browse/WW-5791
> Project: Struts 2
> Issue Type: New Feature
> Components: Core
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.5.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> h2. Goal
> Give newcomers a working Struts application with almost no setup: add one
> dependency, write an action class, run {{main()}}, see the page. No
> {{web.xml}}, no {{struts.xml}}, no external servlet container. Standalone —
> no Spring dependency, no changes to core.
> h2. Scope
> * New {{struts2-boot-starter}} artifact (module {{boot/}}, listed in the BOM)
> pulling in core and embedded Tomcat, registering
> {{StrutsPrepareAndExecuteFilter}} in code. Tomcat follows core's Jakarta
> line: 10.1 by default, 11 under {{-Pjakartaee11}}.
> * {{StrutsBoot.run(App.class, args)}} entry point (blocks);
> {{StrutsBoot.start(...)}} returns a closeable server handle for tests. The
> application class implements {{StrutsBootApplication}}.
> * Fluent action registration, fed into the configuration through a
> {{ConfigurationProvider}} hooked in via the existing {{configProviders}}
> filter init parameter. No Convention plugin, no classpath scanning.
> {code:java}public class HelloApp implements StrutsBootApplication {
> public static void main(String[] args) {
> StrutsBoot.run(HelloApp.class, args);
> }
> @Override
> public void actions(ActionRegistry actions) {
> actions.action("hello", HelloAction.class)
> .success("hello.ftl");
> actions.namespace("/admin", admin -> {
> admin.action("users", UserAction.class)
> .method("list")
> .success("admin/users.ftl");
> admin.action("saveUser", UserAction.class)
> .method("save")
> .httpMethods(POST)
> .result("input", "admin/userForm.ftl")
> .redirectAction("success", "users");
> });
> }
> }
> {code}
> * Defaults: each namespace is a package extending {{struts-default}}; top
> level is namespace {{""}}; {{method}} defaults to {{execute}};
> {{success(path)}} is shorthand for {{result("success", path)}}; interceptors
> default to {{defaultStack}}, overridable per action or namespace by stack
> name; {{httpMethods(...)}} maps to the http-method restriction from WW-3717.
> * Registry config is never broader than the equivalent {{struts.xml}}: Strict
> Method Invocation stays on, action classes go into the OGNL allowlist like
> XML-declared ones, and each action allows only its registered method.
> * Views: FreeMarker only, chosen by {{.ftl}} extension, resolved from
> {{classpath:/templates/}}.
> * Startup fails with a {{ConfigurationException}} for: slashes in action
> names (use namespaces), non-{{.ftl}} views, missing templates, unknown
> interceptor stacks, non-public action classes or ones without a public no-arg
> constructor, and duplicate actions in the same namespace (within the registry
> or against {{struts.xml}}).
> * Configuration: the existing {{struts.properties}}; Boot settings
> {{struts.boot.port}} (default 8080), {{struts.boot.address}} (default: all
> interfaces) and {{struts.boot.context-path}} (default empty), overridable by
> {{--key=value}} arguments. Not {{application.properties}}, to avoid clashing
> with Spring Boot.
> * Container defaults: error pages carry no stack trace or server version;
> {{DefaultServlet}} serves only an empty per-run docBase; Tomcat's base
> directory is a fresh temp directory deleted on shutdown; {{devMode}} is never
> switched on.
> * A {{struts.xml}} on the classpath is still loaded alongside the registry.
> * Example app {{apps/boot-hello}}, shaded into a runnable jar with
> {{maven-shade-plugin}}; not deployed to Maven Central.
> * {{THREAT_MODEL.md}} gains a Boot section: the starter's container defaults
> are in model.
> h2. Parked for later
> * Running an existing Struts WAR as a self-contained jar (migration path).
> * JSP views in the embedded container.
> * Gradle plugin.
> * Alternative containers (Jetty, Undertow).
> * Fluent definition of custom interceptor stacks.
> * Global results, global exception mappings, wildcard actions in the registry.
> * Optional bridge to Convention plugin annotations.
> * Dev-mode reload of templates and registry.
> * Project generator / quick-start archetype.
> * Health and info endpoints.
> * A dedicated Maven plugin or nested-jar launcher, needed before plugins can
> be used in Boot apps (shading overwrites duplicate {{struts-plugin.xml}}
> files).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
