From 93f7e2b6baae606f40cce154f3ad130b098dd148 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 23 Apr 2020 14:35:33 -0700 Subject: [PATCH] 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 --- .../web/embedded/jetty/JettyWebServer.java | 16 ++---- .../web/embedded/netty/NettyWebServer.java | 20 ++----- .../web/embedded/tomcat/TomcatWebServer.java | 17 +----- .../undertow/UndertowServletWebServer.java | 21 ++------ .../embedded/undertow/UndertowWebServer.java | 23 ++------ .../boot/web/server/PortInUseException.java | 54 +++++++++++++++++++ .../AbstractServletWebServerFactoryTests.java | 12 +++++ 7 files changed, 85 insertions(+), 78 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 ad98984b7e..6f0ae81701 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 @@ -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 class JettyWebServer implements WebServer { 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 class JettyWebServer implements WebServer { } } - 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-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 758e1a6d3e..3ddc9d67b9 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 @@ -86,10 +86,11 @@ public class NettyWebServer implements WebServer { 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 @@ public class NettyWebServer implements WebServer { 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/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 9aabbadb3c..c291c21a0e 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-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 class TomcatWebServer implements WebServer { 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 @@ public class TomcatWebServer implements WebServer { } } - 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/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServer.java index fda497d462..e74dd2a678 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServer.java +++ b/spring-boot-project/spring-boot/src/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 class UndertowServletWebServer implements WebServer { } catch (Exception ex) { try { - if (findBindException(ex) != null) { + PortInUseException.ifPortBindingException(ex, (bindException) -> { List failedPorts = getConfiguredPorts(); - List 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 @@ public class UndertowServletWebServer implements WebServer { } } - 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/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 92c25f32ec..d4b3dfbce6 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 @@ -18,7 +18,6 @@ package org.springframework.boot.web.embedded.undertow; 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 class UndertowWebServer implements WebServer { } catch (Exception ex) { try { - if (findBindException(ex) != null) { - List failedPorts = getConfiguredPorts(); - List actualPorts = getActualPorts(); - failedPorts.removeAll(actualPorts); + PortInUseException.ifPortBindingException(ex, (bindException) -> { + List 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 @@ public class UndertowWebServer implements WebServer { } } - 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 ports = getActualPorts(); if (!ports.isEmpty()) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/PortInUseException.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/PortInUseException.java index edfb21c2a5..360c28ce71 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/PortInUseException.java +++ b/spring-boot-project/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 class PortInUseException extends WebServerException { 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 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 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 void ifCausedBy(Exception ex, Class causedBy, Consumer action) { + Throwable candidate = ex; + while (candidate != null) { + if (causedBy.isInstance(candidate)) { + action.accept((E) candidate); + return; + } + candidate = candidate.getCause(); + } + } + } 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 4f4c9c2ae4..3033fc7718 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 @@ -22,6 +22,7 @@ import java.io.FileWriter; 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.testsupport.web.servlet.ExampleServlet; 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 @@ public abstract class AbstractServletWebServerFactoryTests { }); } + @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();