Limit when PortInUseException is thrown

Refactor `PortInUseException` logic to a single place and refine when the exception is thrown. Prior to this commit, we assumed that a `BindException` was only thrown when the port was in use. In fact, it's possible that the exception could be thrown because the requested address "could not be assigned". We now only throw a `PortInUserException` if the `BindException` message includes the phrase "in use". Fixes gh-21101
spring-projects · Apr 23, 2020 · 93f7e2b · 93f7e2b
1 parent 9bb53a4
commit 93f7e2b
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 78 deletions.
diff --git a/...spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java b/...spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java
@@ -17,7 +17,6 @@
 package org.springframework.boot.web.embedded.jetty;
 
 import java.io.IOException;
-import java.net.BindException;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
@@ -147,8 +146,9 @@ public void start() throws WebServerException {
 						connector.start();
 					}
 					catch (IOException ex) {
-						if (connector instanceof NetworkConnector && findBindException(ex) != null) {
-							throw new PortInUseException(((NetworkConnector) connector).getPort(), ex);
+						if (connector instanceof NetworkConnector) {
+							PortInUseException.throwIfPortBindingException(ex,
+									() -> ((NetworkConnector) connector).getPort());
 						}
 						throw ex;
 					}
@@ -168,16 +168,6 @@ public void start() throws WebServerException {
 		}
 	}
 
-	private BindException findBindException(Throwable ex) {
-		if (ex == null) {
-			return null;
-		}
-		if (ex instanceof BindException) {
-			return (BindException) ex;
-		}
-		return findBindException(ex.getCause());
-	}
-
 	private String getActualPortsDescription() {
 		StringBuilder ports = new StringBuilder();
 		for (Connector connector : this.server.getConnectors()) {

diff --git a/...spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java b/...spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java
@@ -86,10 +86,11 @@ public void start() throws WebServerException {
 				this.disposableServer = startHttpServer();
 			}
 			catch (Exception ex) {
-				ChannelBindException bindException = findBindException(ex);
-				if (bindException != null && !isPermissionDenied(bindException.getCause())) {
-					throw new PortInUseException(bindException.localPort(), ex);
-				}
+				PortInUseException.ifCausedBy(ex, ChannelBindException.class, (bindException) -> {
+					if (!isPermissionDenied(bindException.getCause())) {
+						throw new PortInUseException(bindException.localPort(), ex);
+					}
+				});
 				throw new WebServerException("Unable to start Netty", ex);
 			}
 			logger.info("Netty started on port(s): " + getPort());
@@ -129,17 +130,6 @@ private void applyRouteProviders(HttpServerRoutes routes) {
 		routes.route(ALWAYS, this.handlerAdapter);
 	}
 
-	private ChannelBindException findBindException(Exception ex) {
-		Throwable candidate = ex;
-		while (candidate != null) {
-			if (candidate instanceof ChannelBindException) {
-				return (ChannelBindException) candidate;
-			}
-			candidate = candidate.getCause();
-		}
-		return null;
-	}
-
 	private void startDaemonAwaitThread(DisposableServer disposableServer) {
 		Thread awaitThread = new Thread("server") {
 

diff --git a/...ring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java b/...ring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * Copyright 2012-2020 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.
@@ -16,7 +16,6 @@
 
 package org.springframework.boot.web.embedded.tomcat;
 
-import java.net.BindException;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -209,9 +208,7 @@ public void start() throws WebServerException {
 				throw ex;
 			}
 			catch (Exception ex) {
-				if (findBindException(ex) != null) {
-					throw new PortInUseException(this.tomcat.getConnector().getPort());
-				}
+				PortInUseException.throwIfPortBindingException(ex, () -> this.tomcat.getConnector().getPort());
 				throw new WebServerException("Unable to start embedded Tomcat server", ex);
 			}
 			finally {
@@ -234,16 +231,6 @@ private void checkConnectorHasStarted(Connector connector) {
 		}
 	}
 
-	private BindException findBindException(Throwable ex) {
-		if (ex == null) {
-			return null;
-		}
-		if (ex instanceof BindException) {
-			return (BindException) ex;
-		}
-		return findBindException(ex.getCause());
-	}
-
 	private void stopSilently() {
 		try {
 			stopTomcat();

diff --git a/...rc/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServer.java b/...rc/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServer.java
@@ -17,7 +17,6 @@
 package org.springframework.boot.web.embedded.undertow;
 
 import java.lang.reflect.Field;
-import java.net.BindException;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.ArrayList;
@@ -146,14 +145,13 @@ public void start() throws WebServerException {
 			}
 			catch (Exception ex) {
 				try {
-					if (findBindException(ex) != null) {
+					PortInUseException.ifPortBindingException(ex, (bindException) -> {
 						List<Port> failedPorts = getConfiguredPorts();
-						List<Port> actualPorts = getActualPorts();
-						failedPorts.removeAll(actualPorts);
+						failedPorts.removeAll(getActualPorts());
 						if (failedPorts.size() == 1) {
-							throw new PortInUseException(failedPorts.iterator().next().getNumber(), ex);
+							throw new PortInUseException(failedPorts.get(0).getNumber());
 						}
-					}
+					});
 					throw new WebServerException("Unable to start embedded Undertow", ex);
 				}
 				finally {
@@ -180,17 +178,6 @@ private void stopSilently() {
 		}
 	}
 
-	private BindException findBindException(Exception ex) {
-		Throwable candidate = ex;
-		while (candidate != null) {
-			if (candidate instanceof BindException) {
-				return (BindException) candidate;
-			}
-			candidate = candidate.getCause();
-		}
-		return null;
-	}
-
 	private Undertow createUndertowServer() throws ServletException {
 		HttpHandler httpHandler = this.manager.start();
 		httpHandler = getContextHandler(httpHandler);

diff --git a/...-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java b/...-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java
@@ -18,7 +18,6 @@
 
 import java.io.Closeable;
 import java.lang.reflect.Field;
-import java.net.BindException;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.ArrayList;
@@ -104,14 +103,13 @@ public void start() throws WebServerException {
 			}
 			catch (Exception ex) {
 				try {
-					if (findBindException(ex) != null) {
-						List<UndertowWebServer.Port> failedPorts = getConfiguredPorts();
-						List<UndertowWebServer.Port> actualPorts = getActualPorts();
-						failedPorts.removeAll(actualPorts);
+					PortInUseException.ifPortBindingException(ex, (bindException) -> {
+						List<Port> failedPorts = getConfiguredPorts();
+						failedPorts.removeAll(getActualPorts());
 						if (failedPorts.size() == 1) {
-							throw new PortInUseException(failedPorts.iterator().next().getNumber(), ex);
+							throw new PortInUseException(failedPorts.get(0).getNumber());
 						}
-					}
+					});
 					throw new WebServerException("Unable to start embedded Undertow", ex);
 				}
 				finally {
@@ -133,17 +131,6 @@ private void stopSilently() {
 		}
 	}
 
-	private BindException findBindException(Exception ex) {
-		Throwable candidate = ex;
-		while (candidate != null) {
-			if (candidate instanceof BindException) {
-				return (BindException) candidate;
-			}
-			candidate = candidate.getCause();
-		}
-		return null;
-	}
-
 	private String getPortsDescription() {
 		List<UndertowWebServer.Port> ports = getActualPorts();
 		if (!ports.isEmpty()) {

diff --git a/...ect/spring-boot/src/main/java/org/springframework/boot/web/server/PortInUseException.java b/...ect/spring-boot/src/main/java/org/springframework/boot/web/server/PortInUseException.java
@@ -16,11 +16,16 @@
 
 package org.springframework.boot.web.server;
 
+import java.net.BindException;
+import java.util.function.Consumer;
+import java.util.function.IntSupplier;
+
 /**
  * A {@code PortInUseException} is thrown when a web server fails to start due to a port
  * already being in use.
  *
  * @author Andy Wilkinson
+ * @author Phillip Webb
  * @since 2.0.0
  */
 public class PortInUseException extends WebServerException {
@@ -53,4 +58,53 @@ public int getPort() {
 		return this.port;
 	}
 
+	/**
+	 * Throw a {@link PortInUseException} if the given exception was caused by a "port in
+	 * use" {@link BindException}.
+	 * @param ex the source exception
+	 * @param port a suppler used to provide the port
+	 * @since 2.2.7
+	 */
+	public static void throwIfPortBindingException(Exception ex, IntSupplier port) {
+		ifPortBindingException(ex, (bindException) -> {
+			throw new PortInUseException(port.getAsInt(), ex);
+		});
+	}
+
+	/**
+	 * Perform an action if the given exception was caused by a "port in use"
+	 * {@link BindException}.
+	 * @param ex the source exception
+	 * @param action the action to perform
+	 * @since 2.2.7
+	 */
+	public static void ifPortBindingException(Exception ex, Consumer<BindException> action) {
+		ifCausedBy(ex, BindException.class, (bindException) -> {
+			// bind exception can be also thrown because an address can't be assigned
+			if (bindException.getMessage().toLowerCase().contains("in use")) {
+				action.accept(bindException);
+			}
+		});
+	}
+
+	/**
+	 * Perform an action if the given exception was caused by a specific exception type.
+	 * @param <E> the cause exception type
+	 * @param ex the source exception
+	 * @param causedBy the required cause type
+	 * @param action the action to perform
+	 * @since 2.2.7
+	 */
+	@SuppressWarnings("unchecked")
+	public static <E extends Exception> void ifCausedBy(Exception ex, Class<E> causedBy, Consumer<E> action) {
+		Throwable candidate = ex;
+		while (candidate != null) {
+			if (causedBy.isInstance(candidate)) {
+				action.accept((E) candidate);
+				return;
+			}
+			candidate = candidate.getCause();
+		}
+	}
+
 }
diff --git a/...ava/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/...ava/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java
@@ -22,6 +22,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintWriter;
+import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.MalformedURLException;
 import java.net.ServerSocket;
@@ -96,6 +97,7 @@
 import org.springframework.boot.web.server.Compression;
 import org.springframework.boot.web.server.ErrorPage;
 import org.springframework.boot.web.server.MimeMappings;
+import org.springframework.boot.web.server.PortInUseException;
 import org.springframework.boot.web.server.Ssl;
 import org.springframework.boot.web.server.Ssl.ClientAuth;
 import org.springframework.boot.web.server.SslStoreProvider;
@@ -873,6 +875,16 @@ void portClashOfSecondaryConnectorResultsInPortInUseException() throws Exception
 		});
 	}
 
+	@Test
+	void malformedAddress() throws Exception {
+		AbstractServletWebServerFactory factory = getFactory();
+		factory.setAddress(InetAddress.getByName("123456"));
+		assertThatExceptionOfType(RuntimeException.class).isThrownBy(() -> {
+			this.webServer = factory.getWebServer();
+			this.webServer.start();
+		}).isNotInstanceOf(PortInUseException.class);
+	}
+
 	@Test
 	void localeCharsetMappingsAreConfigured() {
 		AbstractServletWebServerFactory factory = getFactory();