Skip to content

Add validation framework for imposing specific rules for configuration values #928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
csutherl wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add validation framework for imposing specific rules for configuration values #928

csutherl wants to merge 2 commits into from
+1,235 −21

Conversation

@csutherl
Copy link
Member

@csutherl csutherl commented Dec 5, 2025

This PR implements a proof-of-concept that enhances the configtest command with validation capabilities, focusing on port configuration as a valuable starting point from dev-list discussion. It includes the framework and a single validation class, PortValidator, for community review. It also adds a LifecycleListener that is capable of stopping the startup process when there are validation failures. This minimal/phase 1 implementation includes the listener as I thought it was a valuable addition and was pretty straightforward to implement. Therefore Phase 3 would just be the SPI implementation, if we want to go that route.

Note: The configtest behavior hasn't changed unless you use the --validate-only option to produce validation output instead of the typical server startup attempt. There's also a new command in Catalina for config-validate for ease of use.

Port validation detects:

  • Port conflicts (already in use)
  • Invalid port numbers (< 0 or > 65535)
  • Duplicate port assignments across connectors
  • Privileged ports (< 1024) without root access
  • Default/insecure shutdown commands
  • AJP connectors missing required 'secret' attribute
  • AJP connectors listening on all interfaces (0.0.0.0)

Future enhancements may include:

  • Additional validators (file resources, JNDI, security, known config issues)
  • ServiceLoader plugin architecture for custom validators
  • Configurable validator ordering
  • Overrides for default validator behavior

@csutherl
Add validation framework for imposing specific rules for configuration
9e8b2d4 
values and a single PortValidator as a POC, and add a LifecycleListener
that is capable of stopping the startup process when there are validation
failures.

This is a proof-of-concept implementation that enhances the configtest
command with validation capabilities, focusing on port configuration
as a valuable starting point from dev-list discussion.

The configtest behavior hasn't changed unless you use the
--validate-only option to produce validation output instead of the
typical server startup attempt. There's also a new command in catalina
for config-validate for ease of use.

Port validation detects:
- Port conflicts (already in use)
- Invalid port numbers (< 0 or > 65535)
- Duplicate port assignments across connectors
- Privileged ports (< 1024) without root access
- Default/insecure shutdown commands
- AJP connectors missing required 'secret' attribute
- AJP connectors listening on all interfaces (0.0.0.0)
csutherl
csutherl commented Dec 8, 2025
View reviewed changes
java/org/apache/catalina/startup/validator/StartupValidationListener.java Outdated
if (abortOnError && result.getErrorCount() > 0) {
String message = sm.getString("startupValidationListener.abortingOnErrors",
String.valueOf(result.getErrorCount()));
throw new IllegalArgumentException(message);
Copy link
Member Author

@csutherl csutherl Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we'd stop the startup process here to avoid unsightly stack traces and output that's already explained by error messages but I haven't quite figured out how. Thoughts?

Copy link
Member

@dsoumis dsoumis Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could throw a dedicated stackless exception e.g ValidationAbortException and handling it in loadInternal() by rethrowing it in the catch block so it doesn't get swallowed and without logging catalina.initError.

Copy link
Member Author

@csutherl csutherl Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that could work I think, but it may still be a bit ugly, no? Ideally I'd completely suppress the exception and just expect that the user take the SEVERE log line saying that it's aborting startup and that be enough while also not logging the stack for the Connector failure to bind to the port.

Current output:

23-Dec-2025 11:36:40.378 SEVERE [main] org.apache.catalina.startup.validator.StartupValidationListener.logFindings Port 8080 (HTTP/1.1, Catalina): Port 8080 is already in use 
23-Dec-2025 11:36:40.379 SEVERE [main] org.apache.catalina.startup.Catalina.loadInternal Error initializing Catalina
    org.apache.catalina.LifecycleException: Failed to initialize component [StandardServer[8005]]
        at org.apache.catalina.util.LifecycleBase.handleSubClassException(LifecycleBase.java:405)
        at org.apache.catalina.util.LifecycleBase.init(LifecycleBase.java:125)
        at org.apache.catalina.startup.Catalina.loadInternal(Catalina.java:728)
        ....
    Caused by: java.lang.IllegalArgumentException: Aborting startup due to 1 configuration errors
        at org.apache.catalina.startup.validator.StartupValidationListener.lifecycleEvent(StartupValidationListener.java:121)
        ....
23-Dec-2025 11:36:40.379 INFO [main] org.apache.catalina.startup.Catalina.loadInternal Server initialization in [98] milliseconds
23-Dec-2025 11:36:40.511 INFO [main] org.apache.catalina.core.StandardService.startInternal Starting service [Catalina]
23-Dec-2025 11:36:40.511 INFO [main] org.apache.catalina.core.StandardEngine.startInternal Starting Servlet engine: [Apache Tomcat/12.0.0-M1-dev]
....                                               
23-Dec-2025 11:36:40.763 INFO [main] org.apache.coyote.AbstractProtocol.init Initializing ProtocolHandler ["http-nio-8080"]
23-Dec-2025 11:36:40.766 SEVERE [main] org.apache.catalina.util.LifecycleBase.handleSubClassException Failed to initialize component [Connector["http-nio-8080"]]
    org.apache.catalina.LifecycleException: Protocol handler initialization failed
        ....
    Caused by: java.net.BindException: Address already in use
        ....
23-Dec-2025 11:36:40.766 INFO [main] org.apache.coyote.AbstractProtocol.start Starting ProtocolHandler ["http-nio-8080"]
23-Dec-2025 11:36:40.766 SEVERE [main] org.apache.catalina.util.LifecycleBase.handleSubClassException Failed to start component [Connector["http-nio-8080"]]
    org.apache.catalina.LifecycleException: Protocol handler start failed
        ....
    Caused by: java.net.BindException: Address already in use
        ....
23-Dec-2025 11:36:40.767 INFO [main] org.apache.catalina.startup.Catalina.start Server startup in [386] milliseconds
23-Dec-2025 11:36:40.768 INFO [main] org.apache.coyote.AbstractProtocol.pause Pausing ProtocolHandler ["http-nio-8080"]

Note that all of the component startup startup message appear as if there was no intentional abort happening :( We go straight from what looks like a successful Server startup to it being destroyed in the last two lines.

Better(?), cleaner output:

23-Dec-2025 11:36:40.378 SEVERE [main] org.apache.catalina.startup.validator.StartupValidationListener.logFindings Port 8080 (HTTP/1.1, Catalina): Port 8080 is already in use 
23-Dec-2025 11:36:40.379 SEVERE [main] org.apache.catalina.startup.Catalina.loadInternal Error initializing Catalina
<Messages indicating things are destroyed>

I don't think the current architecture really allows for what I want though. A System.exit() would do the trick :P

Copy link
Member

@dsoumis dsoumis Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a commit that implements just that by providing a clean abort with only the validation error logged.
In general I have rethrown the custom exception in every possible point :P Skipping redundant logs and stacktrace from Catalina.load().

Copy link
Member

@dsoumis dsoumis Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

output:

24-Dec-2025 14:38:48.195 INFO [main] org.apache.catalina.startup.validator.StartupValidationListener.lifecycleEvent Running configuration validation checks...
24-Dec-2025 14:38:48.201 WARNING [main] org.apache.catalina.startup.validator.StartupValidationListener.logFindings Shutdown Port: Shutdown command is set to default value "SHUTDOWN" on port 8005. Consider using a custom shutdown command.
24-Dec-2025 14:38:48.201 SEVERE [main] org.apache.catalina.startup.validator.StartupValidationListener.logFindings Port 8080 (HTTP/1.1, Catalina): Port 8080 is already in use
24-Dec-2025 14:38:48.201 SEVERE [main] org.apache.catalina.startup.validator.StartupValidationListener.lifecycleEvent Aborting startup due to 1 configuration errors

@csutherl
Copy link
Member Author

csutherl commented Dec 11, 2025

@ChristopherSchultz did you have any input about whether or not this addresses the concern you mentioned on the list thread?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dsoumis dsoumis dsoumis left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@csutherl @dsoumis