From 5c61df3131f095937c057014a3fedc8e5071e1a3 Mon Sep 17 00:00:00 2001 From: Stefan Partusch Date: Wed, 23 Dec 2020 15:24:51 +0100 Subject: [PATCH 1/2] Return -1 port for non-listening WebServers Update `WebServer` implementations to return -1 from `getPort()` if the server isn't listening on a port. This aligns the implementations with the interface Javadoc. See gh-24606 --- .../boot/web/embedded/jetty/JettyWebServer.java | 6 ++++-- .../boot/web/embedded/netty/NettyWebServer.java | 2 +- .../boot/web/embedded/tomcat/TomcatWebServer.java | 2 +- .../web/embedded/undertow/UndertowWebServer.java | 2 +- .../AbstractReactiveWebServerFactoryTests.java | 10 ++++++++++ .../server/AbstractServletWebServerFactoryTests.java | 12 +++++++++++- 6 files changed, 28 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java index 1d6ba620d5..c7a0d59ebe 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java @@ -275,11 +275,13 @@ public class JettyWebServer implements WebServer { @Override public int getPort() { Connector[] connectors = this.server.getConnectors(); + Integer localPort = -1; for (Connector connector : connectors) { // Probably only one... - return getLocalPort(connector); + localPort = getLocalPort(connector); + break; } - return 0; + return (localPort > 0) ? localPort : -1; } @Override diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java index 7f4bbf44ac..193153d9b6 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java @@ -192,7 +192,7 @@ public class NettyWebServer implements WebServer { if (this.disposableServer != null) { return this.disposableServer.port(); } - return 0; + return -1; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java index 98e281f209..5eae980b80 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java @@ -364,7 +364,7 @@ public class TomcatWebServer implements WebServer { if (connector != null) { return connector.getLocalPort(); } - return 0; + return -1; } private String getContextPath() { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java index 5715f1ea3c..b09e462ebb 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java @@ -295,7 +295,7 @@ public class UndertowWebServer implements WebServer { public int getPort() { List ports = getActualPorts(); if (ports.isEmpty()) { - return 0; + return -1; } return ports.get(0).getNumber(); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java index 42b3595ff9..a41b922bfe 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java @@ -115,6 +115,16 @@ public abstract class AbstractReactiveWebServerFactoryTests { assertThat(this.webServer.getPort()).isEqualTo(specificPort); } + @Test + void portIsMinusOneWhenConnectionIsClosed() { + AbstractReactiveWebServerFactory factory = getFactory(); + this.webServer = factory.getWebServer(new EchoHandler()); + this.webServer.start(); + assertThat(this.webServer.getPort()).isGreaterThan(0); + this.webServer.stop(); + assertThat(this.webServer.getPort()).isEqualTo(-1); + } + @Test void basicSslFromClassPath() { testBasicSslWithKeyStore("classpath:test.jks", "password"); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java index c51f1f0f19..94df46ce7a 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java @@ -250,7 +250,7 @@ public abstract class AbstractServletWebServerFactoryTests { factory.setPort(-1); this.webServer = factory.getWebServer(exampleServletRegistration()); this.webServer.start(); - assertThat(this.webServer.getPort()).isLessThan(0); // Jetty is -2 + assertThat(this.webServer.getPort()).isEqualTo(-1); } @Test @@ -300,6 +300,16 @@ public abstract class AbstractServletWebServerFactoryTests { assertThat(servlet.getInitCount()).isEqualTo(1); } + @Test + void portIsMinusOneWhenConnectionIsClosed() { + AbstractServletWebServerFactory factory = getFactory(); + this.webServer = factory.getWebServer(); + this.webServer.start(); + assertThat(this.webServer.getPort()).isGreaterThan(0); + this.webServer.stop(); + assertThat(this.webServer.getPort()).isEqualTo(-1); + } + @Test void specificPort() throws Exception { AbstractServletWebServerFactory factory = getFactory(); From 9b9c3edfcd0f5cef7531a47bf7cf0ea96a142531 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 5 Jan 2021 15:54:46 -0800 Subject: [PATCH 2/2] Polish "Return -1 port for non-listening WebServers" See gh-24606 --- .../web/embedded/jetty/JettyWebServer.java | 43 +++++++++++-------- .../web/embedded/netty/NettyWebServer.java | 2 +- .../web/embedded/tomcat/TomcatWebServer.java | 2 +- .../embedded/undertow/UndertowWebServer.java | 2 +- ...AbstractReactiveWebServerFactoryTests.java | 2 +- .../AbstractServletWebServerFactoryTests.java | 2 +- 6 files changed, 30 insertions(+), 23 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java index c7a0d59ebe..aebc93149d 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -207,18 +207,6 @@ public class JettyWebServer implements WebServer { return ports.toString(); } - private Integer getLocalPort(Connector connector) { - try { - // Jetty 9 internals are different, but the method name is the same - return (Integer) ReflectionUtils - .invokeMethod(ReflectionUtils.findMethod(connector.getClass(), "getLocalPort"), connector); - } - catch (Exception ex) { - logger.info("could not determine port ( " + ex.getMessage() + ")"); - return 0; - } - } - private String getProtocols(Connector connector) { List protocols = connector.getProtocols(); return " (" + StringUtils.collectionToDelimitedString(protocols, ", ") + ")"; @@ -275,13 +263,32 @@ public class JettyWebServer implements WebServer { @Override public int getPort() { Connector[] connectors = this.server.getConnectors(); - Integer localPort = -1; for (Connector connector : connectors) { - // Probably only one... - localPort = getLocalPort(connector); - break; + Integer localPort = getLocalPort(connector); + if (localPort != null && localPort > 0) { + return localPort; + } + } + return -1; + } + + private Integer getLocalPort(Connector connector) { + try { + if (connector instanceof NetworkConnector) { + return ((NetworkConnector) connector).getLocalPort(); + } + } + catch (Exception ex) { + } + try { + // Jetty 9 internals are different, but the method name is the same + return (Integer) ReflectionUtils + .invokeMethod(ReflectionUtils.findMethod(connector.getClass(), "getLocalPort"), connector); + } + catch (Exception ex) { + logger.info("could not determine port ( " + ex.getMessage() + ")"); } - return (localPort > 0) ? localPort : -1; + return 0; } @Override diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java index 193153d9b6..da00e5fac0 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java index 5eae980b80..b0e243aefe 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java index b09e462ebb..5278bf6920 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java index a41b922bfe..88b7f78595 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java index 94df46ce7a..9c76d168a9 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.