From f1d0735f5951a3dde80c3c7a4f4930779bff70b0 Mon Sep 17 00:00:00 2001 From: Pedro Matias Date: Sun, 30 Aug 2026 04:13:07 +0100 Subject: [PATCH 1/3] GH-1027: Reject port 0 in NettyClientBuilder and fix tests that omitted port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tighten the port validation to 1–65535 (port 0 is reserved and invalid for client connections). Four builder-based tests were not calling withPort() at all, so the port field defaulted to 0 and slipped through unnoticed before the validation was added. Each test now passes the actual test-server port via FLIGHT_SERVER_TEST_EXTENSION.getPort(). Co-Authored-By: Claude Sonnet 4.6 --- .../java/org/apache/arrow/flight/grpc/NettyClientBuilder.java | 4 ++-- .../org/apache/arrow/driver/jdbc/ConnectionMutualTlsTest.java | 1 + .../java/org/apache/arrow/driver/jdbc/ConnectionTest.java | 1 + .../apache/arrow/driver/jdbc/ConnectionTlsRootCertsTest.java | 1 + .../java/org/apache/arrow/driver/jdbc/ConnectionTlsTest.java | 1 + 5 files changed, 6 insertions(+), 2 deletions(-) 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..e32a446971 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 @@ -140,9 +140,9 @@ public NettyChannelBuilder build() { case LocationSchemes.GRPC_TLS: { final int port = location.getUri().getPort(); - if (port < 0 || port > 65535) { + 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..cd4862975e 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 @@ -240,6 +240,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) From c28a347f6553c8ca22eb90b4e8d2cb29cf595c64 Mon Sep 17 00:00:00 2001 From: Pedro Matias Date: Mon, 31 Aug 2026 02:56:48 +0100 Subject: [PATCH 2/3] GH-1027: Parameterize invalid port test to cover port 0 Converts testUnencryptedConnectionProvidingInvalidPort to a @ParameterizedTest covering 0, -1, 65536, and 65537, explicitly exercising the port-0 rejection added to NettyClientBuilder.build(). Co-Authored-By: Claude Sonnet 4.6 --- .../apache/arrow/driver/jdbc/ConnectionTest.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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 cd4862975e..9a644a9865 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 (out-of-range or reserved port 0). */ - @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, From 97ab8b27b0e7ddd5cbabdb82d1c4b0fc6ad83f32 Mon Sep 17 00:00:00 2001 From: Pedro Matias Date: Mon, 31 Aug 2026 02:59:01 +0100 Subject: [PATCH 3/3] Improve comment --- .../test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9a644a9865..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 @@ -207,7 +207,7 @@ public void testGetBasicClientAuthenticatedShouldOpenConnection() throws Excepti /** * Checks if a SQLException is thrown when trying to establish an unencrypted connection with an - * invalid port (out-of-range or reserved port 0). + * invalid port. */ @ParameterizedTest @ValueSource(ints = {0, -1, 65536, 65537})