Skip to content

Commit 084606a

Browse files
ennuiteclaude
andcommitted
GH-1027: Move port validation to NettyClientBuilder.build()
Validate the port synchronously in NettyClientBuilder.build() before calling forAddress(String, int), restoring the fail-fast behaviour that was lost when switching away from forAddress(SocketAddress). Placing the check here protects all callers (not just JDBC), and the resulting IllegalArgumentException is already caught and wrapped as SQLException by ArrowFlightSqlClientHandler.Builder.build(). Reverts the validation that was previously added to ArrowFlightConnectionConfigImpl.getPort() and the associated throws-SQLException ripple across FlightServerTestExtension, OAuthIntegrationTest, and ArrowFlightConnectionConfigImplTest. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e895aa4 commit 084606a

5 files changed

Lines changed: 17 additions & 24 deletions

File tree

flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/NettyClientBuilder.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,12 @@ public NettyChannelBuilder build() {
139139
case LocationSchemes.GRPC_INSECURE:
140140
case LocationSchemes.GRPC_TLS:
141141
{
142-
builder =
143-
NettyChannelBuilder.forAddress(
144-
location.getUri().getHost(), location.getUri().getPort());
142+
final int port = location.getUri().getPort();
143+
if (port < 1 || port > 65535) {
144+
throw new IllegalArgumentException(
145+
"Invalid port " + port + ": must be between 1 and 65535.");
146+
}
147+
builder = NettyChannelBuilder.forAddress(location.getUri().getHost(), port);
145148
break;
146149
}
147150
case LocationSchemes.GRPC_DOMAIN_SOCKET:

flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,8 @@ public String getHost() {
5555
*
5656
* @return the port.
5757
*/
58-
public int getPort() throws SQLException {
59-
final int port = ArrowFlightConnectionProperty.PORT.getInteger(properties);
60-
if (port < 1 || port > 65535) {
61-
throw new SQLException("Invalid port " + port + ": must be between 1 and 65535.");
62-
}
63-
return port;
58+
public int getPort() {
59+
return ArrowFlightConnectionProperty.PORT.getInteger(properties);
6460
}
6561

6662
/**

flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/FlightServerTestExtension.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ private FlightServer getStartServer(
211211
*
212212
* @return the port value.
213213
*/
214-
public int getPort() throws SQLException {
214+
public int getPort() {
215215
return config.getPort();
216216
}
217217

flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/OAuthIntegrationTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ private void enqueueErrorResponse(String error, String description) {
161161
.build());
162162
}
163163

164-
private Properties createBaseProperties() throws SQLException {
164+
private Properties createBaseProperties() {
165165
Properties props = new Properties();
166166
props.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost");
167167
props.put(
@@ -170,7 +170,7 @@ private Properties createBaseProperties() throws SQLException {
170170
return props;
171171
}
172172

173-
private String getJdbcUrl() throws SQLException {
173+
private String getJdbcUrl() {
174174
return String.format(
175175
"jdbc:arrow-flight-sql://localhost:%d", FLIGHT_SERVER_TEST_EXTENSION.getPort());
176176
}
@@ -408,7 +408,7 @@ public void testTokenRefreshAfterExpiration() throws Exception {
408408
// ==================== Error Handling Tests ====================
409409

410410
@Test
411-
public void testMissingRequiredParametersClientCredentials() throws Exception {
411+
public void testMissingRequiredParametersClientCredentials() {
412412
Properties props = createBaseProperties();
413413
props.put(ArrowFlightConnectionProperty.OAUTH_FLOW.camelName(), "client_credentials");
414414
props.put(ArrowFlightConnectionProperty.OAUTH_TOKEN_URI.camelName(), tokenEndpoint.toString());
@@ -418,7 +418,7 @@ public void testMissingRequiredParametersClientCredentials() throws Exception {
418418
}
419419

420420
@Test
421-
public void testMissingRequiredParametersTokenExchange() throws Exception {
421+
public void testMissingRequiredParametersTokenExchange() {
422422
Properties props = createBaseProperties();
423423
props.put(ArrowFlightConnectionProperty.OAUTH_FLOW.camelName(), "token_exchange");
424424
props.put(ArrowFlightConnectionProperty.OAUTH_TOKEN_URI.camelName(), tokenEndpoint.toString());
@@ -428,7 +428,7 @@ public void testMissingRequiredParametersTokenExchange() throws Exception {
428428
}
429429

430430
@Test
431-
public void testInvalidOAuthFlow() throws Exception {
431+
public void testInvalidOAuthFlow() {
432432
Properties props = createBaseProperties();
433433
props.put(ArrowFlightConnectionProperty.OAUTH_FLOW.camelName(), "invalid_flow");
434434
props.put(ArrowFlightConnectionProperty.OAUTH_TOKEN_URI.camelName(), tokenEndpoint.toString());
@@ -437,7 +437,7 @@ public void testInvalidOAuthFlow() throws Exception {
437437
}
438438

439439
@Test
440-
public void testMalformedTokenEndpoint() throws Exception {
440+
public void testMalformedTokenEndpoint() {
441441
Properties props = createBaseProperties();
442442
props.put(ArrowFlightConnectionProperty.OAUTH_FLOW.camelName(), "client_credentials");
443443
props.put(ArrowFlightConnectionProperty.OAUTH_TOKEN_URI.camelName(), "not-a-valid-uri://");

flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImplTest.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public void testGetProperty(
7272
}
7373

7474
public static Stream<Arguments> provideParameters() {
75-
int port = RANDOM.nextInt(65535) + 1;
75+
int port = RANDOM.nextInt(Short.toUnsignedInt(Short.MAX_VALUE));
7676
boolean useEncryption = RANDOM.nextBoolean();
7777
int threadPoolSize = RANDOM.nextInt(getRuntime().availableProcessors());
7878
return Stream.of(
@@ -87,13 +87,7 @@ public static Stream<Arguments> provideParameters() {
8787
port,
8888
port,
8989
(Function<ArrowFlightConnectionConfigImpl, ?>)
90-
config -> {
91-
try {
92-
return config.getPort();
93-
} catch (java.sql.SQLException e) {
94-
throw new RuntimeException(e);
95-
}
96-
}),
90+
ArrowFlightConnectionConfigImpl::getPort),
9791
Arguments.of(
9892
USER,
9993
"user",

0 commit comments

Comments
 (0)