fix: don't auto-start jetty service for negative port

[Dogacel]

Prevent Jetty Web Server from auto-starting and binding to a random port if server.port is set to a negative value. This issue is also reported in Armeria repository: line/armeria#5039. We want to prevent Jetty/Tomcat from binding to a random port so that the traffic can be only handled via Armeria server and forwarded to Jetty.

Currently, when server.port is set to -1 in Tomcat servlet, it doesn't start the tomcat server on a random port.

2025-03-07T21:17:51.954-06:00 INFO 30944 --- [ main] s.tomcat.SampleTomcatApplication : ServletContext initialized
2025-03-07T21:17:53.129-06:00 INFO 30944 --- [ main] o.s.b.w.embedded.tomcat.TomcatWebServer : Tomcat started on port -1 (http) with context path '/'
2025-03-07T21:17:53.148-06:00 INFO 30944 --- [ main] s.tomcat.SampleTomcatApplication : Started SampleTomcatApplication in 1.97 seconds (process running for 2.223)

However, same behavior results in Jetty to start on a random port.

2025-03-07T21:15:34.947-06:00 INFO 30835 --- [ main] org.eclipse.jetty.server.Server : Started oejs.Server@7a904f32{STARTING}[12.0.16,sto=0] @1145ms
2025-03-07T21:15:35.104-06:00 INFO 30835 --- [ main] o.s.b.web.embedded.jetty.JettyWebServer : Jetty started on port 56372 (http/1.1) with context path '/'
2025-03-07T21:15:35.108-06:00 INFO 30835 --- [ main] smoketest.jetty.SampleJettyApplication : Started SampleJettyApplication in 1.046 seconds (process running for 1.306)

After applying those changes, I have manually tested by creating application.yml under resources/config folder and set server.port: -1 for both Jetty and Tomcat. My changes resulted Jetty to show a similar behavior with Tomcat.

2025-03-07T21:15:34.947-06:00 INFO 30835 --- [ main] org.eclipse.jetty.server.Server : Started oejs.Server@7a904f32{STARTING}[12.0.16,sto=0] @1145ms
2025-03-07T21:15:35.104-06:00 INFO 30835 --- [ main] o.s.b.web.embedded.jetty.JettyWebServer : Jetty started on port -1 (http/1.1) with context path '/'
2025-03-07T21:15:35.108-06:00 INFO 30835 --- [ main] smoketest.jetty.SampleJettyApplication : Started SampleJettyApplication in 1.046 seconds (process running for 1.306)

[wilkinsona]

Thanks for the PR but I cannot reproduce the behavior that you have described. Starting a plain Spring Boot application that uses Jetty with server.port set to -1 does not start the server. Specifically, output similar to the following is not logged:

2025-03-09T08:59:51.401Z  INFO 91406 --- [           main] o.e.jetty.server.AbstractConnector       : Started ServerConnector@24db6ce{HTTP/1.1, (http/1.1)}{0.0.0.0:52560}
2025-03-09T08:59:51.403Z  INFO 91406 --- [           main] o.s.b.web.server.jetty.JettyWebServer    : Jetty started on port 52560 (http/1.1) with context path '/'

The proposed changes look as if they will result in more of Jetty being started. Specifically, server.start() will now be called which doesn't seem right to me.

If you would like us to consider making changes in this area, please provide a minimal sample that reproduces the behavior that you have described.

[Dogacel]

Thanks for the PR but I cannot reproduce the behavior that you have described. Starting a plain Spring Boot application that uses Jetty with server.port set to -1 does not start the server. Specifically, output similar to the following is not logged:

2025-03-09T08:59:51.401Z  INFO 91406 --- [           main] o.e.jetty.server.AbstractConnector       : Started ServerConnector@24db6ce{HTTP/1.1, (http/1.1)}{0.0.0.0:52560}
2025-03-09T08:59:51.403Z  INFO 91406 --- [           main] o.s.b.web.server.jetty.JettyWebServer    : Jetty started on port 52560 (http/1.1) with context path '/'

The proposed changes look as if they will result in more of Jetty being started. Specifically, server.start() will now be called which doesn't seem right to me.

If you would like us to consider making changes in this area, please provide a minimal sample that reproduces the behavior that you have described.

Sorry I have skipped a minor detail. We are currently adding the following piece of code to all armeria/jetty based applications otherwise it doesn't work.

    @Configuration(proxyBeanMethods = false)
    @ConditionalOnClass({ Servlet.class, Server.class, Loader.class, WebAppContext.class })
    static class EmbeddedJetty {

        @Bean
        JettyServletWebServerFactory jettyServletWebServerFactory(
                ObjectProvider<JettyServerCustomizer> serverCustomizers) {
            final JettyServletWebServerFactory factory = new JettyServletWebServerFactory() {

                @Override
                protected JettyWebServer getJettyWebServer(Server server) {
                    return new JettyWebServer(server, true);
                }
            };
            factory.getServerCustomizers().addAll(serverCustomizers.orderedStream().toList());
            return factory;
        }
    }

So the fix is basically this line -> new JettyWebServer(server, true);

However this results in auto-configure to be set to true, which results in Jetty to start the server at a random port. However we don't see this behavior in tomcat and it works out of the box without any issues. My goal is to ensure jetty starts at port -1 if autoconfigure is set to false.

I am sending a commit that includes smoke tests for armeria-jetty and armeria-tomcat. I am trying to make the unit tests work at the moment however you can test it by running the Main methods and observe the difference between before/after the patch.

It also made me realize my fix currently doesn't work.

[Dogacel]

@wilkinsona Second update, I have fixed the previously mentioned bug and also completed the smoke tests for Armeria Jetty / Tomcat servlet supports. Please let me know if anything looks odd!

[Dogacel]

cc: @trustin @minwoox @jrhee17 @ikhoon for adding Armeria smoketests to the Spring repository.

[wilkinsona]

I’m afraid we won’t add Ameria-based smoke tests to Spring Boot as it creates a cycle between the two projects.

Unfortunately, I am still not convinced about the auto-start change either. Arguably Tomcat is wrong at the moment as it is partially started whereas Undertow and Jetty are correct as neither starts at all when autostart is false. Your change moves Jetty closer to tomcat which is in the wrong direction.

Can we take a step back please? What’s the need for Jetty to be in a partially started state?

[Dogacel]

I’m afraid we won’t add Ameria-based smoke tests to Spring Boot as it creates a cycle between the two projects.

Unfortunately, I am still not convinced about the auto-start change either. Arguably Tomcat is wrong at the moment as it is partially started whereas Undertow and Jetty are correct as neither starts at all when autostart is false. Your change moves Jetty closer to tomcat which is in the wrong direction.

Can we take a step back please? What’s the need for Jetty to be in a partially started state?

With the Armeria connector we would like to forward the incoming requests to the Jetty servlet and let it process it, but we don't want Jetty to bind to a random port. Because Armeria exposes itself to the network instead of Jetty, everything must go through Armeria first. Currently behavior of Tomcat when port is set to -1 is what we are trying to achieve with Jetty.

How can we make sure Jetty servlet can serve traffic over a custom connector but never expose itself to the external traffic?

[wilkinsona]

I would use a customizer to remove the server's connectors. Something like this:

@Bean
JettyServerCustomizer jettyCustomizer() {
    return (server) -> server.setConnectors(new Connector[0]);
}

This should result in Jetty starting up but not accepting any connections itself.

[Dogacel]

I would use a customizer to remove the server's connectors. Something like this:

@Bean
JettyServerCustomizer jettyCustomizer() {
    return (server) -> server.setConnectors(new Connector[0]);
}

This should result in Jetty starting up but not accepting any connections itself.

I have also tried it, however removing the connector still resulted in Jetty to start listening some random port and handle some of the requests. I have to double check if it was handling it properly or was just a dummy response I was seeking.

[ikhoon]

I’m one of the maintainers of Armeria. Armeria's Jetty integration needs to call handleDeferredInitialize() when autoStart = false (server.port = -1). Would it be possible for JettyWebServer to expose .isAutoStart() and .handleDeferredInitialize() as public APIs?

This would allow us to achieve our goal without modifying the internals of JettyWebServer. @wilkinsona What do you think?

[wilkinsona]

As this discussion has shown, the "auto-start" feature is a bit of a mess at the moment. I've opened #44656 to review things. Until that's been done, I don't think we should expose isAutoStart() as public API as it may be heading in the wrong direction and making things worse.

I'd really like to fully explore the approach of removing the server's connector(s). As far as I know, that should get things into the state that you want and it can already be achieved with the existing public API. @Dogacel said above that it didn't work and Jetty still listened on a random port. I'd like to see an example of that so that we can figure out what's going on. We can then take things from there and see what needs to be done to support Armeria's needs.