From 8efa2fc569307cdd7bbf71f6a61e52216fc2aaa5 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Sat, 23 Nov 2013 11:26:55 +0000 Subject: [PATCH] Use server.port=0 for scanning This leverages existing capabilities of teh JDK and the OS to grab a port at random and not have it stolen by another process. It's very hard to avoid that race condition in pure Java code, so why bother? User can set port<0 to disable autoStart of connectors (e.g. to start a web application context but not have it listen on any port). In that case the actual socket port will be set to 0 (and therefore if it ever starts up the local port will be random). --- .../autoconfigure/web/ServerProperties.java | 34 ------------------- .../web/ServerPropertiesTests.java | 21 ------------ spring-boot-javadoc/pom.xml | 2 +- ...stractEmbeddedServletContainerFactory.java | 9 ----- .../jetty/JettyEmbeddedServletContainer.java | 6 ++++ .../JettyEmbeddedServletContainerFactory.java | 7 ++-- .../TomcatEmbeddedServletContainer.java | 1 + ...TomcatEmbeddedServletContainerFactory.java | 10 ++---- 8 files changed, 15 insertions(+), 75 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java index 9132b4df5aa..7379bcb4c59 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java @@ -35,7 +35,6 @@ import org.springframework.boot.context.embedded.tomcat.TomcatConnectorCustomize import org.springframework.boot.context.embedded.tomcat.TomcatContextCustomizer; import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory; import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.util.SocketUtils; import org.springframework.util.StringUtils; /** @@ -54,8 +53,6 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer { private Integer sessionTimeout; - private boolean scan = false; - @NotNull private String contextPath = ""; @@ -81,14 +78,6 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer { this.port = port; } - public boolean getScan() { - return this.scan; - } - - public void setScan(boolean scan) { - this.scan = scan; - } - public InetAddress getAddress() { return this.address; } @@ -112,9 +101,6 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer { @Override public void customize(ConfigurableEmbeddedServletContainerFactory factory) { Integer port = getPort(); - if (this.scan) { - port = scanForPort(port); - } if (port != null) { factory.setPort(port); } @@ -132,26 +118,6 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer { } } - private Integer scanForPort(Integer port) { - boolean found = false; - int delta = 1; - port = port == null ? 8080 : port; - while (!found) { - try { - port = SocketUtils.findAvailableTcpPort(port, port + delta); - found = true; - } - catch (IllegalStateException e) { - if (delta > 65536) { - throw e; - } - delta = delta > 5 ? delta > 100 ? delta * 4 : delta * 3 : delta * 2; - } - port = port + delta; - } - return port; - } - public static class Tomcat { private String accessLogPattern; diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java index cc3df622210..96f66412756 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java @@ -24,12 +24,9 @@ import java.util.Map; import org.junit.Test; import org.springframework.beans.MutablePropertyValues; import org.springframework.boot.bind.RelaxedDataBinder; -import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletContainerFactory; -import org.springframework.boot.context.embedded.MockEmbeddedServletContainerFactory; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; /** * Tests for {@link ServerProperties}. @@ -71,24 +68,6 @@ public class ServerPropertiesTests { .getProtocolHeader()); } - @Test - public void testPortScan() throws Exception { - this.properties.setScan(true); - this.properties.setPort(1000); - ConfigurableEmbeddedServletContainerFactory factory = new MockEmbeddedServletContainerFactory(); - this.properties.customize(factory); - assertTrue(factory.getPort() > 1000); - } - - @Test - public void testPortScanFromHigher() throws Exception { - this.properties.setScan(true); - this.properties.setPort(5678); - ConfigurableEmbeddedServletContainerFactory factory = new MockEmbeddedServletContainerFactory(); - this.properties.customize(factory); - assertTrue(factory.getPort() < 6000); - } - // FIXME test customize } diff --git a/spring-boot-javadoc/pom.xml b/spring-boot-javadoc/pom.xml index c1d94f5cca0..669e28899cf 100644 --- a/spring-boot-javadoc/pom.xml +++ b/spring-boot-javadoc/pom.xml @@ -50,7 +50,7 @@ aggregate-javadocs - jar + aggregate-jar true diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java index 5748208458f..07fd74ce0e2 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactory.java @@ -82,7 +82,6 @@ public abstract class AbstractEmbeddedServletContainerFactory implements * @param port the port number for the embedded servlet container */ public AbstractEmbeddedServletContainerFactory(int port) { - checkPort(port); this.port = port; } @@ -94,7 +93,6 @@ public abstract class AbstractEmbeddedServletContainerFactory implements */ public AbstractEmbeddedServletContainerFactory(String contextPath, int port) { checkContextPath(contextPath); - checkPort(port); this.contextPath = contextPath; this.port = port; } @@ -129,16 +127,9 @@ public abstract class AbstractEmbeddedServletContainerFactory implements @Override public void setPort(int port) { - checkPort(port); this.port = port; } - private void checkPort(int port) { - if (port < 0 || port > 65535) { - throw new IllegalArgumentException("Port must be between 1 and 65535"); - } - } - /** * Returns the port that the embedded servlet container should listen on. */ diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java index de615ee44b6..cc6ff8b6ab4 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java @@ -16,6 +16,8 @@ package org.springframework.boot.context.embedded.jetty; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Server; import org.springframework.boot.context.embedded.EmbeddedServletContainer; @@ -28,10 +30,13 @@ import org.springframework.util.Assert; * {@link JettyEmbeddedServletContainerFactory} and not directly. * * @author Phillip Webb + * @author Dave Syer * @see JettyEmbeddedServletContainerFactory */ public class JettyEmbeddedServletContainer implements EmbeddedServletContainer { + private final Log logger = LogFactory.getLog(JettyEmbeddedServletContainer.class); + private final Server server; private boolean autoStart; @@ -81,6 +86,7 @@ public class JettyEmbeddedServletContainer implements EmbeddedServletContainer { Connector[] connectors = this.server.getConnectors(); for (Connector connector : connectors) { connector.start(); + this.logger.info("Jetty started on port: " + connector.getLocalPort()); } } catch (Exception ex) { diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java index 24ec2ee0b86..2df5413a261 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java @@ -97,7 +97,8 @@ public class JettyEmbeddedServletContainerFactory extends public EmbeddedServletContainer getEmbeddedServletContainer( ServletContextInitializer... initializers) { WebAppContext context = new WebAppContext(); - Server server = new Server(new InetSocketAddress(getAddress(), getPort())); + int port = getPort() >= 0 ? getPort() : 0; + Server server = new Server(new InetSocketAddress(getAddress(), port)); if (this.resourceLoader != null) { context.setClassLoader(this.resourceLoader.getClassLoader()); @@ -123,7 +124,7 @@ public class JettyEmbeddedServletContainerFactory extends postProcessWebAppContext(context); server.setHandler(context); - this.logger.info("Server initialized with port: " + getPort()); + this.logger.info("Server initialized with port: " + port); return getJettyEmbeddedServletContainer(server); } @@ -241,7 +242,7 @@ public class JettyEmbeddedServletContainerFactory extends * @return a new {@link JettyEmbeddedServletContainer} instance */ protected JettyEmbeddedServletContainer getJettyEmbeddedServletContainer(Server server) { - return new JettyEmbeddedServletContainer(server, getPort() > 0); + return new JettyEmbeddedServletContainer(server, getPort() >= 0); } @Override diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index 14de6387611..dd16e9c7dc8 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -104,6 +104,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer if (connector != null && this.autoStart) { try { connector.getProtocolHandler().start(); + this.logger.info("Tomcat started on port: " + connector.getLocalPort()); } catch (Exception ex) { this.logger.error("Cannot start connector: ", ex); diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactory.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactory.java index 9b578455b78..bd746e8f180 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactory.java @@ -193,12 +193,8 @@ public class TomcatEmbeddedServletContainerFactory extends // Needs to be protected so it can be used by subclasses protected void customizeConnector(Connector connector) { - if (getPort() > 0) { - connector.setPort(getPort()); - } - else { - connector.setPort(8080); - } + int port = getPort() >= 0 ? getPort() : 0; + connector.setPort(port); if (connector.getProtocolHandler() instanceof AbstractProtocol) { if (getAddress() != null) { ((AbstractProtocol) connector.getProtocolHandler()) @@ -259,7 +255,7 @@ public class TomcatEmbeddedServletContainerFactory extends */ protected TomcatEmbeddedServletContainer getTomcatEmbeddedServletContainer( Tomcat tomcat) { - return new TomcatEmbeddedServletContainer(tomcat, getPort() > 0); + return new TomcatEmbeddedServletContainer(tomcat, getPort() >= 0); } private File createTempDir(String prefix) {