Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ public CompletableFuture<?> start() {

final CompletableFuture<?> resultFuture = new CompletableFuture<>();
try {
NetworkUtility.checkPortsAvailable(config.getPort());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to add the extra empty line?

// Create the HTTP server and a router object.
httpServer = vertx.createHttpServer(getHttpServerOptions());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ public CompletableFuture<Void> start() {

final CompletableFuture<Void> resultFuture = new CompletableFuture<>();
try {
NetworkUtility.checkPortsAvailable(config.getPort());
// Create the HTTP server and a router object.
httpServer = vertx.createHttpServer(getHttpServerOptions());
httpServer.webSocketHandler(webSocketHandler());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.subscription.SubscriptionManager;
import org.hyperledger.besu.metrics.BesuMetricCategory;
import org.hyperledger.besu.plugin.services.MetricsSystem;
import org.hyperledger.besu.util.NetworkUtility;

import java.net.InetSocketAddress;
import java.util.Optional;
Expand Down Expand Up @@ -104,7 +105,7 @@ public CompletableFuture<?> start() {
"Starting Websocket service on {}:{}", configuration.getHost(), configuration.getPort());

final CompletableFuture<?> resultFuture = new CompletableFuture<>();

NetworkUtility.checkPortsAvailable(configuration.getPort());
httpServer =
vertx
.createHttpServer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.api.jsonrpc;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
Expand Down Expand Up @@ -2083,4 +2084,19 @@ public void handleResponseWithOptionalExistingValue() throws Exception {
rpcMethods.remove(method.getName());
}
}

@Test
public void failWhenPortIsAlreadyInUse() throws Exception {
final JsonRpcConfiguration config = createJsonRpcConfig();
config.setHost("0.0.0.0");
config.setPort(8545);
final JsonRpcHttpService firstServiceToAllocatePort = createJsonRpcHttpService(config);
final JsonRpcHttpService secondServiceToAllocatePort = createJsonRpcHttpService(config);
firstServiceToAllocatePort.start().join();

assertThatIllegalArgumentException()
.isThrownBy(() -> secondServiceToAllocatePort.start())
.withMessageContaining(
"Port 8545 is already in use. Check for other processes using this port.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.api.jsonrpc.websocket;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.spy;
Expand Down Expand Up @@ -385,4 +386,25 @@ public void handleResponseWithOptionalExistingValue(final TestContext context) {
async.awaitSuccess(VERTX_AWAIT_TIMEOUT_MILLIS);
async.handler((result) -> websocketMethods.remove(method.getName()));
}

@Test
public void failWhenPortIsAlreadyInUse() {
websocketConfiguration = WebSocketConfiguration.createDefault();
websocketConfiguration.setHost("0.0.0.0");
websocketConfiguration.setPort(8546);
final WebSocketService firstWebSocketService =
new WebSocketService(
vertx, websocketConfiguration, webSocketMessageHandlerSpy, new NoOpMetricsSystem());

firstWebSocketService.start().join();

final WebSocketService secondWebSocketService =
new WebSocketService(
vertx, websocketConfiguration, webSocketMessageHandlerSpy, new NoOpMetricsSystem());

assertThatIllegalArgumentException()
.isThrownBy(() -> secondWebSocketService.start())
.withMessageContaining(
"Port 8546 is already in use. Check for other processes using this port.");
}
}
25 changes: 25 additions & 0 deletions util/src/main/java/org/hyperledger/besu/util/NetworkUtility.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,26 @@
*/
package org.hyperledger.besu.util;

import java.io.IOException;
import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.NetworkInterface;
import java.net.ServerSocket;
import java.net.SocketException;
import java.net.UnknownHostException;
import java.util.function.Supplier;

import com.google.common.base.Suppliers;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class NetworkUtility {
public static final String INADDR_ANY = "0.0.0.0";
public static final String INADDR6_ANY = "0:0:0:0:0:0:0:0";

private static final Logger LOG = LoggerFactory.getLogger(NetworkUtility.class);

private NetworkUtility() {}

private static final Supplier<Boolean> ipv6Available =
Expand Down Expand Up @@ -98,4 +104,23 @@ public static void checkPort(final int port, final String portTypeName) {
"%s port requires a value between 1 and 65535. Got %d.", portTypeName, port));
}
}

public static boolean isPortAvailableForTcp(final int port) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be private?

try (final ServerSocket serverSocket = new ServerSocket()) {
serverSocket.setReuseAddress(true);
serverSocket.bind(new InetSocketAddress(port));
return true;
} catch (IOException ex) {
LOG.trace(String.format("Failed to open port %d for TCP", port), ex);
}
return false;
}

public static void checkPortsAvailable(final int tcpPort) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then method name says Ports so suggests that more than one port as parameter like a var arg, otherwise use the singular

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the name of the method, thanks for raising that @fab-10

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the approach to cover the ports we will require to initialise all the services defined in the config. So we can have a more general solution that loops through the ports and check if they are available.

if (!isPortAvailableForTcp(tcpPort)) {
throw new InvalidConfigurationException(
String.format(
"Port %d is already in use. Check for other processes using this port.", tcpPort));
}
}
}