From eb99dd053d748072e49e0dfccd1aa9a6411bd871 Mon Sep 17 00:00:00 2001 From: webarch Date: Tue, 9 Jun 2020 15:42:47 -0400 Subject: [PATCH] Adding bind address pre-check before starting the default proxy server, this avoids resource leaking --- .../browserup/bup/BrowserUpProxyServer.java | 2 +- .../bup/exception/AddressInUseException.java | 46 +++++++++++++++++++ .../com/browserup/bup/proxy/ProxyManager.java | 46 ++++++++++++++++++- .../bup/proxy/ProxyPortAssignmentTest.java | 29 +++++++++++- 4 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 browserup-proxy-rest/src/main/java/com/browserup/bup/exception/AddressInUseException.java diff --git a/browserup-proxy-core/src/main/java/com/browserup/bup/BrowserUpProxyServer.java b/browserup-proxy-core/src/main/java/com/browserup/bup/BrowserUpProxyServer.java index a467fccba..6f7ab62d6 100644 --- a/browserup-proxy-core/src/main/java/com/browserup/bup/BrowserUpProxyServer.java +++ b/browserup-proxy-core/src/main/java/com/browserup/bup/BrowserUpProxyServer.java @@ -530,7 +530,7 @@ public InetAddress getClientBindAddress() { @Override public int getPort() { if (started.get()) { - return proxyServer.getListenAddress().getPort(); + return (proxyServer != null ? proxyServer.getListenAddress().getPort() : 0); } else { return 0; } diff --git a/browserup-proxy-rest/src/main/java/com/browserup/bup/exception/AddressInUseException.java b/browserup-proxy-rest/src/main/java/com/browserup/bup/exception/AddressInUseException.java new file mode 100644 index 000000000..545b121a7 --- /dev/null +++ b/browserup-proxy-rest/src/main/java/com/browserup/bup/exception/AddressInUseException.java @@ -0,0 +1,46 @@ +/* + * Modifications Copyright (c) 2019 BrowserUp, Inc. + */ + +package com.browserup.bup.exception; + +import java.net.InetAddress; + +public class AddressInUseException extends RuntimeException { + + private static final long serialVersionUID = -2548760680049910435L; + private final int port; + private final InetAddress address; + + public AddressInUseException(int port, InetAddress bindAddress) { + this.port = port; + this.address = bindAddress; + } + + public AddressInUseException(String message, int port, InetAddress bindAddress) { + super(message); + this.port = port; + this.address = bindAddress; + } + + public AddressInUseException(String message, Throwable cause, int port, InetAddress bindAddress) { + super(message, cause); + this.port = port; + this.address = bindAddress; + } + + public AddressInUseException(Throwable cause, int port, InetAddress bindAddress) { + super(cause); + this.port = port; + this.address = bindAddress; + } + + public int getPort() { + return port; + } + + public InetAddress getAddress() { + return address; + } + +} diff --git a/browserup-proxy-rest/src/main/java/com/browserup/bup/proxy/ProxyManager.java b/browserup-proxy-rest/src/main/java/com/browserup/bup/proxy/ProxyManager.java index 007c37147..d49da0d7b 100644 --- a/browserup-proxy-rest/src/main/java/com/browserup/bup/proxy/ProxyManager.java +++ b/browserup-proxy-rest/src/main/java/com/browserup/bup/proxy/ProxyManager.java @@ -5,6 +5,7 @@ package com.browserup.bup.proxy; import com.browserup.bup.BrowserUpProxyServer; +import com.browserup.bup.exception.AddressInUseException; import com.browserup.bup.exception.ProxyExistsException; import com.browserup.bup.exception.ProxyPortsExhaustedException; import com.browserup.bup.proxy.auth.AuthType; @@ -19,9 +20,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.lang.ref.WeakReference; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.Socket; import java.net.URISyntaxException; import java.net.UnknownHostException; import java.util.Collection; @@ -217,8 +220,48 @@ public BrowserUpProxyServer create(String upstreamHttpProxy, boolean upstreamPro } throw new ProxyPortsExhaustedException(); } + + /** + * Check that the Port is Available to bind to. + * + * We need to verify that the port isn't already bound to on the client bind address. + * The application by default assumes that the min-max ports are reserved, however + * in a mixed environment there may be some stray ports that are being used by + * other services. + * + * This method will verify that the port isn't already bound by trying + * to bind to it and then closing it. + * + * If the port is already bound it will throw a run-time exception. + * + * @param clientBindAddress the bind address to check, can be null which is a wildcard + * @param port the port to attempt to bind to with the bind address + */ + private void checkPortAvailability(InetAddress clientBindAddress, Integer port) { - public BrowserUpProxyServer create(String upstreamHttpProxy, String proxyUsername, String proxyPassword, Integer port, String bindAddr, boolean useEcc, boolean trustAllServers) { + InetSocketAddress testClientBindSocket; + + // Use the wildcard address if the clientBindAddress isn't specified + if (clientBindAddress == null) { + testClientBindSocket = new InetSocketAddress(port); + } else { + testClientBindSocket = new InetSocketAddress(clientBindAddress, port); + } + + // Use a test socket to attempt binding with + try(Socket testSocket = new Socket()) { + testSocket.bind(testClientBindSocket); + } catch (IOException e) { + // Assume that the port cannot be bound to, log a warning and then return False + LOG.error("Bind address unavailable: " + testClientBindSocket.toString(), e); + throw new AddressInUseException(e, port, clientBindAddress); + } + + // Log that the bind address is available + LOG.debug("Bind address available {}", testClientBindSocket); + } + + public BrowserUpProxyServer create(String upstreamHttpProxy, String proxyUsername, String proxyPassword, Integer port, String bindAddr, boolean useEcc, boolean trustAllServers) { return create(upstreamHttpProxy, false, null, proxyUsername, proxyPassword, port, null, null, false, false); } @@ -249,6 +292,7 @@ private BrowserUpProxyServer startProxy(BrowserUpProxyServer proxy, int port, In LOG.info("Proxy already exists at port {}", port); throw new ProxyExistsException(port); } + checkPortAvailability(clientBindAddr, port); } try { diff --git a/browserup-proxy-rest/src/test/java/com/browserup/bup/proxy/ProxyPortAssignmentTest.java b/browserup-proxy-rest/src/test/java/com/browserup/bup/proxy/ProxyPortAssignmentTest.java index b974935a5..7c99ae30f 100644 --- a/browserup-proxy-rest/src/test/java/com/browserup/bup/proxy/ProxyPortAssignmentTest.java +++ b/browserup-proxy-rest/src/test/java/com/browserup/bup/proxy/ProxyPortAssignmentTest.java @@ -7,7 +7,12 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; + import com.browserup.bup.BrowserUpProxyServer; +import com.browserup.bup.exception.AddressInUseException; import com.browserup.bup.exception.ProxyExistsException; import com.browserup.bup.exception.ProxyPortsExhaustedException; import com.browserup.bup.proxy.test.util.ProxyManagerTest; @@ -57,4 +62,26 @@ public void testManualAssignment() { proxyManager.delete(9094); } } -} \ No newline at end of file + + @Test + public void testBindFailure() { + + int testPort = 9094; + + InetSocketAddress testClientBindSocket = new InetSocketAddress(testPort); + + // Use a test socket to attempt binding with + try(Socket testSocket = new Socket()) { + testSocket.bind(testClientBindSocket); + try{ + proxyManager.create(9094); + fail(); + }catch(AddressInUseException e){ + assertEquals(9094, e.getPort()); + } + } catch (IOException e1) { + e1.printStackTrace(); + fail(); + } + } +}