diff --git a/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightClient.java b/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightClient.java index fd6e498d13..c8964b93a6 100644 --- a/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightClient.java +++ b/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightClient.java @@ -822,7 +822,13 @@ public Builder verifyServer(boolean verifyServer) { return this; } - /** Create the client from this builder. */ + /** + * Create the client from this builder. + * + * @throws IllegalArgumentException if the location uses a TCP-based scheme ({@code grpc}, + * {@code grpc+tcp}, {@code grpc+tls}) and the location contains no port, or a port outside + * the range [1, 65535]. + */ public FlightClient build() { final NettyChannelBuilder channelBuilder = builder.build(); return new FlightClient(builder.allocator(), channelBuilder.build(), builder.middleware()); diff --git a/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/NettyClientBuilder.java b/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/NettyClientBuilder.java index 7df1a0a2a2..d7d8abe2e8 100644 --- a/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/NettyClientBuilder.java +++ b/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/NettyClientBuilder.java @@ -130,7 +130,13 @@ public NettyClientBuilder verifyServer(boolean verifyServer) { return this; } - /** Create the client from this builder. */ + /** + * Create the client from this builder. + * + * @throws IllegalArgumentException if the URI uses a TCP-based scheme ({@code grpc}, {@code + * grpc+tcp}, {@code grpc+tls}) and the URI contains no port, or a port outside the range [1, + * 65535]. + */ public NettyChannelBuilder build() { final NettyChannelBuilder builder; @@ -140,9 +146,13 @@ public NettyChannelBuilder build() { case LocationSchemes.GRPC_TLS: { final int port = location.getUri().getPort(); - if (port < 0 || port > 65535) { + if (port == -1) { + throw new IllegalArgumentException( + "No port specified in location URI: " + location.getUri()); + } + if (port < 1 || port > 65535) { throw new IllegalArgumentException( - "Invalid port " + port + ": must be between 0 and 65535."); + "Invalid port " + port + ": must be between 1 and 65535."); } builder = NettyChannelBuilder.forAddress(location.getUri().getHost(), port); break; diff --git a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java index cc95115b93..1a39671711 100644 --- a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java +++ b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java @@ -184,6 +184,7 @@ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception { try (ArrowFlightSqlClientHandler client = new ArrowFlightSqlClientHandler.Builder() .withHost(FLIGHT_SERVER_TEST_EXTENSION.getHost()) + .withPort(FLIGHT_SERVER_TEST_EXTENSION.getPort()) .withTlsRootCertificates(tlsRootCertsPath) .withClientCertificate(clientMTlsCertPath) .withClientKey(clientMTlsKeyPath) diff --git a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java index d9122d1015..1fb0eba655 100644 --- a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java +++ b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java @@ -63,6 +63,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** Tests for {@link Connection}. */ public class ConnectionTest { @@ -204,13 +206,12 @@ public void testGetBasicClientAuthenticatedShouldOpenConnection() throws Excepti } /** - * Checks if the exception IllegalArgumentException is thrown when trying to establish an - * unencrypted connection providing with an invalid port. - * - * @throws SQLException on error. + * Checks if a SQLException is thrown when trying to establish an unencrypted connection with an + * invalid port. */ - @Test - public void testUnencryptedConnectionProvidingInvalidPort() throws Exception { + @ParameterizedTest + @ValueSource(ints = {0, -1, 65536, 65537}) + public void testUnencryptedConnectionProvidingInvalidPort(int invalidPort) { final Properties properties = new Properties(); properties.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost"); @@ -218,7 +219,7 @@ public void testUnencryptedConnectionProvidingInvalidPort() throws Exception { properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(), passTest); properties.put(ArrowFlightConnectionProperty.USE_ENCRYPTION.camelName(), false); final String invalidUrl = - "jdbc:arrow-flight-sql://" + FLIGHT_SERVER_TEST_EXTENSION.getHost() + ":" + 65537; + "jdbc:arrow-flight-sql://" + FLIGHT_SERVER_TEST_EXTENSION.getHost() + ":" + invalidPort; assertThrows( SQLException.class, @@ -240,6 +241,7 @@ public void testGetBasicClientNoAuthShouldOpenConnection() throws Exception { try (ArrowFlightSqlClientHandler client = new ArrowFlightSqlClientHandler.Builder() .withHost(FLIGHT_SERVER_TEST_EXTENSION.getHost()) + .withPort(FLIGHT_SERVER_TEST_EXTENSION.getPort()) .withBufferAllocator(allocator) .withEncryption(false) .build()) { diff --git a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsRootCertsTest.java b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsRootCertsTest.java index f46ab1fa1c..76ed7e92bc 100644 --- a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsRootCertsTest.java +++ b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsRootCertsTest.java @@ -135,6 +135,7 @@ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception { try (ArrowFlightSqlClientHandler client = new ArrowFlightSqlClientHandler.Builder() .withHost(FLIGHT_SERVER_TEST_EXTENSION.getHost()) + .withPort(FLIGHT_SERVER_TEST_EXTENSION.getPort()) .withTlsRootCertificates(tlsRootCertsPath) .withBufferAllocator(allocator) .withEncryption(true) diff --git a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java index 387436afe9..ddea9b24af 100644 --- a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java +++ b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java @@ -172,6 +172,7 @@ public void testGetNonAuthenticatedEncryptedClientNoAuth() throws Exception { try (ArrowFlightSqlClientHandler client = new ArrowFlightSqlClientHandler.Builder() .withHost(FLIGHT_SERVER_TEST_EXTENSION.getHost()) + .withPort(FLIGHT_SERVER_TEST_EXTENSION.getPort()) .withSystemTrustStore(false) .withTrustStorePath(trustStorePath) .withTrustStorePassword(trustStorePass)