Improve analysis of tomcat bind exception

Using the throwOnFailure attribute on the tomcat connector, we can now
determine if the underlying exception was a BindException and throw
a PortInUseException instead of the generic WebServerException.

Closes gh-7130
pull/17199/head
Madhura Bhave 6 years ago
parent 587e647e1a
commit 4c7e457582

@ -115,6 +115,7 @@ public class TomcatReactiveWebServerFactory extends AbstractReactiveWebServerFac
File baseDir = (this.baseDirectory != null) ? this.baseDirectory : createTempDir("tomcat");
tomcat.setBaseDir(baseDir.getAbsolutePath());
Connector connector = new Connector(this.protocol);
connector.setThrowOnFailure(true);
tomcat.getService().addConnector(connector);
customizeConnector(connector);
tomcat.setConnector(connector);

@ -176,6 +176,7 @@ public class TomcatServletWebServerFactory extends AbstractServletWebServerFacto
File baseDir = (this.baseDirectory != null) ? this.baseDirectory : createTempDir("tomcat");
tomcat.setBaseDir(baseDir.getAbsolutePath());
Connector connector = new Connector(this.protocol);
connector.setThrowOnFailure(true);
tomcat.getService().addConnector(connector);
customizeConnector(connector);
tomcat.setConnector(connector);

@ -16,6 +16,7 @@
package org.springframework.boot.web.embedded.tomcat;
import java.net.BindException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
@ -37,6 +38,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.naming.ContextBindings;
import org.springframework.boot.web.server.PortInUseException;
import org.springframework.boot.web.server.WebServer;
import org.springframework.boot.web.server.WebServerException;
import org.springframework.util.Assert;
@ -207,6 +209,9 @@ public class TomcatWebServer implements WebServer {
throw ex;
}
catch (Exception ex) {
if (findBindException(ex) != null) {
throw new PortInUseException(this.tomcat.getConnector().getPort());
}
throw new WebServerException("Unable to start embedded Tomcat server", ex);
}
finally {
@ -229,6 +234,16 @@ 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();

@ -338,9 +338,14 @@ class JettyServletWebServerFactoryTests extends AbstractServletWebServerFactoryT
}
@Override
protected void handleExceptionCausedByBlockedPort(RuntimeException ex, int blockedPort) {
protected void handleExceptionCausedByBlockedPortOnPrimaryConnector(RuntimeException ex, int blockedPort) {
assertThat(ex).isInstanceOf(PortInUseException.class);
assertThat(((PortInUseException) ex).getPort()).isEqualTo(blockedPort);
}
@Override
protected void handleExceptionCausedByBlockedPortOnSecondaryConnector(RuntimeException ex, int blockedPort) {
this.handleExceptionCausedByBlockedPortOnPrimaryConnector(ex, blockedPort);
}
}

@ -16,6 +16,9 @@
package org.springframework.boot.web.embedded.tomcat;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.util.Arrays;
import org.apache.catalina.Context;
@ -32,10 +35,15 @@ import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.InOrder;
import org.springframework.boot.web.reactive.server.AbstractReactiveWebServerFactory;
import org.springframework.boot.web.reactive.server.AbstractReactiveWebServerFactoryTests;
import org.springframework.boot.web.server.PortInUseException;
import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactoryTests;
import org.springframework.http.server.reactive.HttpHandler;
import org.springframework.util.SocketUtils;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.inOrder;
@ -188,4 +196,41 @@ class TomcatReactiveWebServerFactoryTests extends AbstractReactiveWebServerFacto
assertThat(context.getClearReferencesThreadLocals()).isFalse();
}
@Test
protected void portClashOfPrimaryConnectorResultsInPortInUseException() throws IOException {
doWithBlockedPort((port) -> {
assertThatExceptionOfType(RuntimeException.class).isThrownBy(() -> {
AbstractReactiveWebServerFactory factory = getFactory();
factory.setPort(port);
this.webServer = factory.getWebServer(mock(HttpHandler.class));
this.webServer.start();
}).satisfies((ex) -> handleExceptionCausedByBlockedPortOnPrimaryConnector(ex, port));
});
}
protected final void doWithBlockedPort(AbstractServletWebServerFactoryTests.BlockedPortAction action)
throws IOException {
int port = SocketUtils.findAvailableTcpPort(40000);
ServerSocket serverSocket = new ServerSocket();
for (int i = 0; i < 10; i++) {
try {
serverSocket.bind(new InetSocketAddress(port));
break;
}
catch (Exception ex) {
}
}
try {
action.run(port);
}
finally {
serverSocket.close();
}
}
protected void handleExceptionCausedByBlockedPortOnPrimaryConnector(RuntimeException ex, int blockedPort) {
assertThat(ex).isInstanceOf(PortInUseException.class);
assertThat(((PortInUseException) ex).getPort()).isEqualTo(blockedPort);
}
}

@ -66,6 +66,7 @@ import org.mockito.ArgumentCaptor;
import org.mockito.InOrder;
import org.springframework.boot.testsupport.system.CapturedOutput;
import org.springframework.boot.web.server.PortInUseException;
import org.springframework.boot.web.server.WebServerException;
import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactory;
import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactoryTests;
@ -326,18 +327,6 @@ class TomcatServletWebServerFactoryTests extends AbstractServletWebServerFactory
assertThat(connector.getURIEncoding()).isEqualTo("UTF-8");
}
@Test
void primaryConnectorPortClashThrowsWebServerException() throws IOException {
doWithBlockedPort((port) -> {
TomcatServletWebServerFactory factory = getFactory();
factory.setPort(port);
assertThatExceptionOfType(WebServerException.class).isThrownBy(() -> {
this.webServer = factory.getWebServer();
this.webServer.start();
});
});
}
@Test
void startupFailureDoesNotResultInUnstoppedThreadsBeingReported(CapturedOutput capturedOutput) throws IOException {
super.portClashOfPrimaryConnectorResultsInPortInUseException();
@ -593,7 +582,13 @@ class TomcatServletWebServerFactoryTests extends AbstractServletWebServerFactory
}
@Override
protected void handleExceptionCausedByBlockedPort(RuntimeException ex, int blockedPort) {
protected void handleExceptionCausedByBlockedPortOnPrimaryConnector(RuntimeException ex, int blockedPort) {
assertThat(ex).isInstanceOf(PortInUseException.class);
assertThat(((PortInUseException) ex).getPort()).isEqualTo(blockedPort);
}
@Override
protected void handleExceptionCausedByBlockedPortOnSecondaryConnector(RuntimeException ex, int blockedPort) {
assertThat(ex).isInstanceOf(ConnectorStartFailedException.class);
assertThat(((ConnectorStartFailedException) ex).getPort()).isEqualTo(blockedPort);
}

@ -279,11 +279,16 @@ class UndertowServletWebServerFactoryTests extends AbstractServletWebServerFacto
}
@Override
protected void handleExceptionCausedByBlockedPort(RuntimeException ex, int blockedPort) {
protected void handleExceptionCausedByBlockedPortOnPrimaryConnector(RuntimeException ex, int blockedPort) {
assertThat(ex).isInstanceOf(PortInUseException.class);
assertThat(((PortInUseException) ex).getPort()).isEqualTo(blockedPort);
Undertow undertow = (Undertow) ReflectionTestUtils.getField(this.webServer, "undertow");
assertThat(undertow.getWorker()).isNull();
}
@Override
protected void handleExceptionCausedByBlockedPortOnSecondaryConnector(RuntimeException ex, int blockedPort) {
this.handleExceptionCausedByBlockedPortOnPrimaryConnector(ex, blockedPort);
}
}

@ -831,7 +831,7 @@ public abstract class AbstractServletWebServerFactoryTests {
factory.setPort(port);
AbstractServletWebServerFactoryTests.this.webServer = factory.getWebServer();
AbstractServletWebServerFactoryTests.this.webServer.start();
}).satisfies((ex) -> handleExceptionCausedByBlockedPort(ex, port));
}).satisfies((ex) -> handleExceptionCausedByBlockedPortOnPrimaryConnector(ex, port));
});
}
@ -843,7 +843,7 @@ public abstract class AbstractServletWebServerFactoryTests {
addConnector(port, factory);
AbstractServletWebServerFactoryTests.this.webServer = factory.getWebServer();
AbstractServletWebServerFactoryTests.this.webServer.start();
}).satisfies((ex) -> handleExceptionCausedByBlockedPort(ex, port));
}).satisfies((ex) -> handleExceptionCausedByBlockedPortOnSecondaryConnector(ex, port));
});
}
@ -962,7 +962,10 @@ public abstract class AbstractServletWebServerFactoryTests {
protected abstract void addConnector(int port, AbstractServletWebServerFactory factory);
protected abstract void handleExceptionCausedByBlockedPort(RuntimeException ex, int blockedPort);
protected abstract void handleExceptionCausedByBlockedPortOnPrimaryConnector(RuntimeException ex, int blockedPort);
protected abstract void handleExceptionCausedByBlockedPortOnSecondaryConnector(RuntimeException ex,
int blockedPort);
private boolean doTestCompression(int contentSize, String[] mimeTypes, String[] excludedUserAgents)
throws Exception {

Loading…
Cancel
Save