diff --git a/presto-cli/src/main/java/com/facebook/presto/cli/ClientOptions.java b/presto-cli/src/main/java/com/facebook/presto/cli/ClientOptions.java index dd2f5e2df9cec..30ac745f3ea2e 100644 --- a/presto-cli/src/main/java/com/facebook/presto/cli/ClientOptions.java +++ b/presto-cli/src/main/java/com/facebook/presto/cli/ClientOptions.java @@ -146,6 +146,9 @@ public class ClientOptions @Option(name = "--validate-nexturi-source", title = "validate nextUri source", description = "Validate nextUri server host and port does not change during query execution") public boolean validateNextUriSource; + @Option(name = "--disable-redirects", title = "disable redirects", description = "Disable client following redirects from server") + public boolean disableRedirects; + public enum OutputFormat { ALIGNED, diff --git a/presto-cli/src/main/java/com/facebook/presto/cli/Console.java b/presto-cli/src/main/java/com/facebook/presto/cli/Console.java index c4de47f94e738..0669deab1ac8a 100644 --- a/presto-cli/src/main/java/com/facebook/presto/cli/Console.java +++ b/presto-cli/src/main/java/com/facebook/presto/cli/Console.java @@ -138,7 +138,8 @@ public boolean run() Optional.ofNullable(clientOptions.krb5ConfigPath), Optional.ofNullable(clientOptions.krb5KeytabPath), Optional.ofNullable(clientOptions.krb5CredentialCachePath), - !clientOptions.krb5DisableRemoteServiceHostnameCanonicalization)) { + !clientOptions.krb5DisableRemoteServiceHostnameCanonicalization, + !clientOptions.disableRedirects)) { if (hasQuery) { return executeCommand(queryRunner, query, clientOptions.outputFormat, clientOptions.ignoreErrors); } diff --git a/presto-cli/src/main/java/com/facebook/presto/cli/QueryRunner.java b/presto-cli/src/main/java/com/facebook/presto/cli/QueryRunner.java index 81067f598ec40..4921f127f98fe 100644 --- a/presto-cli/src/main/java/com/facebook/presto/cli/QueryRunner.java +++ b/presto-cli/src/main/java/com/facebook/presto/cli/QueryRunner.java @@ -66,7 +66,8 @@ public QueryRunner( Optional kerberosConfigPath, Optional kerberosKeytabPath, Optional kerberosCredentialCachePath, - boolean kerberosUseCanonicalHostname) + boolean kerberosUseCanonicalHostname, + boolean followRedirects) { this.session = new AtomicReference<>(requireNonNull(session, "session is null")); this.debug = debug; @@ -98,6 +99,7 @@ public QueryRunner( Optional.ofNullable(session.getExtraCredentials().get(GCS_CREDENTIALS_PATH_KEY)) .ifPresent(credentialPath -> setupGCSOauth(builder, credentialPath, Optional.ofNullable(session.getExtraCredentials().get(GCS_OAUTH_SCOPES_KEY)))); + builder.followRedirects(followRedirects); this.httpClient = builder.build(); } diff --git a/presto-cli/src/test/java/com/facebook/presto/cli/AbstractCliTest.java b/presto-cli/src/test/java/com/facebook/presto/cli/AbstractCliTest.java index f3397e6338851..25f2a817c5b4f 100644 --- a/presto-cli/src/test/java/com/facebook/presto/cli/AbstractCliTest.java +++ b/presto-cli/src/test/java/com/facebook/presto/cli/AbstractCliTest.java @@ -140,7 +140,8 @@ protected static QueryRunner createQueryRunner(ClientSession clientSession) Optional.empty(), Optional.empty(), Optional.empty(), - false); + false, + true); } protected static void assertHeaders(String headerName, Headers headers, Set expectedSessionHeaderValues) diff --git a/presto-cli/src/test/java/com/facebook/presto/cli/TestClientOptions.java b/presto-cli/src/test/java/com/facebook/presto/cli/TestClientOptions.java index 3600142664746..97c6aa882d233 100644 --- a/presto-cli/src/test/java/com/facebook/presto/cli/TestClientOptions.java +++ b/presto-cli/src/test/java/com/facebook/presto/cli/TestClientOptions.java @@ -134,6 +134,13 @@ public void testDisableCompression() assertTrue(console.clientOptions.toClientSession().isCompressionDisabled()); } + @Test + public void testDisableFollowingRedirects() + { + Console console = singleCommand(Console.class).parse("--disable-redirects"); + assertTrue(console.clientOptions.disableRedirects); + } + @Test(expectedExceptions = IllegalArgumentException.class) public void testThreePartPropertyName() { diff --git a/presto-client/src/main/java/com/facebook/presto/client/JsonResponse.java b/presto-client/src/main/java/com/facebook/presto/client/JsonResponse.java index 18d4c0642b473..8823fad06d74c 100644 --- a/presto-client/src/main/java/com/facebook/presto/client/JsonResponse.java +++ b/presto-client/src/main/java/com/facebook/presto/client/JsonResponse.java @@ -130,7 +130,7 @@ public static JsonResponse execute(JsonCodec codec, OkHttpClient clien { try (Response response = client.newCall(request).execute()) { // TODO: fix in OkHttp: https://github.com/square/okhttp/issues/3111 - if ((response.code() == 307) || (response.code() == 308)) { + if (((response.code() == 307) || (response.code() == 308)) && client.followRedirects()) { String location = response.header(LOCATION); if (location != null) { request = request.newBuilder().url(location).build(); diff --git a/presto-docs/src/main/sphinx/installation/jdbc.rst b/presto-docs/src/main/sphinx/installation/jdbc.rst index f46091ef99901..a1393a264799f 100644 --- a/presto-docs/src/main/sphinx/installation/jdbc.rst +++ b/presto-docs/src/main/sphinx/installation/jdbc.rst @@ -120,4 +120,5 @@ Name Description ``testHeaderKey:testHeaderValue`` will inject the header ``testHeaderKey`` with value ``testHeaderValue``. Values should be percent encoded. ``validateNextUriSource`` Validates that host and port in next URI does not change during query execution. +``followRedirects`` Disable Presto client to follow a redirect as a security measure. ================================= ======================================================================= diff --git a/presto-jdbc/src/main/java/com/facebook/presto/jdbc/ConnectionProperties.java b/presto-jdbc/src/main/java/com/facebook/presto/jdbc/ConnectionProperties.java index bebddb6fbe36e..6111034c24aa6 100644 --- a/presto-jdbc/src/main/java/com/facebook/presto/jdbc/ConnectionProperties.java +++ b/presto-jdbc/src/main/java/com/facebook/presto/jdbc/ConnectionProperties.java @@ -63,6 +63,7 @@ final class ConnectionProperties public static final ConnectionProperty> HTTP_PROTOCOLS = new HttpProtocols(); public static final ConnectionProperty> QUERY_INTERCEPTORS = new QueryInterceptors(); public static final ConnectionProperty VALIDATE_NEXTURI_SOURCE = new ValidateNextUriSource(); + public static final ConnectionProperty FOLLOW_REDIRECTS = new FollowRedirects(); private static final Set> ALL_PROPERTIES = ImmutableSet.>builder() .add(USER) @@ -91,6 +92,7 @@ final class ConnectionProperties .add(HTTP_PROTOCOLS) .add(QUERY_INTERCEPTORS) .add(VALIDATE_NEXTURI_SOURCE) + .add(FOLLOW_REDIRECTS) .build(); private static final Map> KEY_LOOKUP = unmodifiableMap(ALL_PROPERTIES.stream() @@ -378,4 +380,13 @@ public ValidateNextUriSource() super("validateNextUriSource", Optional.of("false"), NOT_REQUIRED, ALLOWED, BOOLEAN_CONVERTER); } } + + private static class FollowRedirects + extends AbstractConnectionProperty + { + public FollowRedirects() + { + super("followRedirects", Optional.of("true"), NOT_REQUIRED, ALLOWED, BOOLEAN_CONVERTER); + } + } } diff --git a/presto-jdbc/src/main/java/com/facebook/presto/jdbc/PrestoDriver.java b/presto-jdbc/src/main/java/com/facebook/presto/jdbc/PrestoDriver.java index ed7e5f2485f58..0d0c9bb218962 100644 --- a/presto-jdbc/src/main/java/com/facebook/presto/jdbc/PrestoDriver.java +++ b/presto-jdbc/src/main/java/com/facebook/presto/jdbc/PrestoDriver.java @@ -85,6 +85,7 @@ public Connection connect(String url, Properties info) PrestoDriverUri uri = new PrestoDriverUri(url, info); OkHttpClient.Builder builder = httpClient.newBuilder(); + builder.followRedirects(uri.followRedirects()); uri.setupClient(builder); QueryExecutor executor = new QueryExecutor(builder.build()); diff --git a/presto-jdbc/src/main/java/com/facebook/presto/jdbc/PrestoDriverUri.java b/presto-jdbc/src/main/java/com/facebook/presto/jdbc/PrestoDriverUri.java index 0b1f29d418680..cf5bf7a7ae175 100644 --- a/presto-jdbc/src/main/java/com/facebook/presto/jdbc/PrestoDriverUri.java +++ b/presto-jdbc/src/main/java/com/facebook/presto/jdbc/PrestoDriverUri.java @@ -52,6 +52,7 @@ import static com.facebook.presto.jdbc.ConnectionProperties.CUSTOM_HEADERS; import static com.facebook.presto.jdbc.ConnectionProperties.DISABLE_COMPRESSION; import static com.facebook.presto.jdbc.ConnectionProperties.EXTRA_CREDENTIALS; +import static com.facebook.presto.jdbc.ConnectionProperties.FOLLOW_REDIRECTS; import static com.facebook.presto.jdbc.ConnectionProperties.HTTP_PROTOCOLS; import static com.facebook.presto.jdbc.ConnectionProperties.HTTP_PROXY; import static com.facebook.presto.jdbc.ConnectionProperties.KERBEROS_CONFIG_PATH; @@ -217,6 +218,12 @@ public boolean validateNextUriSource() return VALIDATE_NEXTURI_SOURCE.getValue(properties).orElse(false); } + public boolean followRedirects() + throws SQLException + { + return FOLLOW_REDIRECTS.getValue(properties).orElse(true); + } + public void setupClient(OkHttpClient.Builder builder) throws SQLException { diff --git a/presto-jdbc/src/test/java/com/facebook/presto/jdbc/TestPrestoDriverUri.java b/presto-jdbc/src/test/java/com/facebook/presto/jdbc/TestPrestoDriverUri.java index 932ce0689ee57..282556b0252ad 100644 --- a/presto-jdbc/src/test/java/com/facebook/presto/jdbc/TestPrestoDriverUri.java +++ b/presto-jdbc/src/test/java/com/facebook/presto/jdbc/TestPrestoDriverUri.java @@ -26,6 +26,7 @@ import static com.facebook.presto.jdbc.ConnectionProperties.CUSTOM_HEADERS; import static com.facebook.presto.jdbc.ConnectionProperties.DISABLE_COMPRESSION; import static com.facebook.presto.jdbc.ConnectionProperties.EXTRA_CREDENTIALS; +import static com.facebook.presto.jdbc.ConnectionProperties.FOLLOW_REDIRECTS; import static com.facebook.presto.jdbc.ConnectionProperties.HTTP_PROTOCOLS; import static com.facebook.presto.jdbc.ConnectionProperties.HTTP_PROXY; import static com.facebook.presto.jdbc.ConnectionProperties.QUERY_INTERCEPTORS; @@ -181,6 +182,17 @@ public void testUriWithoutCompression() assertEquals(parameters.getProperties().getProperty(DISABLE_COMPRESSION.getKey()), "true"); } + @Test + public void testUriWithoutFollowingRedirects() + throws SQLException + { + PrestoDriverUri parameters = createDriverUri("presto://localhost:8080/blackhole?followRedirects=false"); + assertFalse(parameters.followRedirects()); + assertEquals(parameters.getProperties().getProperty(FOLLOW_REDIRECTS.getKey()), "false"); + + assertInvalid("presto://localhost:8080/blackhole?followRedirects=ANOTHERVALUE", "Connection property 'followRedirects' value is invalid: ANOTHERVALUE"); + } + @Test public void testUriWithoutSsl() throws SQLException