From e0bcf44c9764584348de8db5205bde18fb92c3e5 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 2 Oct 2024 07:27:36 +0900 Subject: [PATCH 1/6] Enable ErrorProne DefaultCharset --- .../main/java/io/trino/gateway/ha/handler/ProxyUtils.java | 5 +++-- .../test/java/io/trino/gateway/ha/HaGatewayTestUtils.java | 2 +- pom.xml | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java b/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java index af017542b..ed752201a 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java @@ -29,6 +29,7 @@ import static io.trino.gateway.ha.handler.QueryIdCachingProxyHandler.TRINO_UI_PATH; import static io.trino.gateway.ha.handler.QueryIdCachingProxyHandler.USER_HEADER; import static io.trino.gateway.ha.handler.QueryIdCachingProxyHandler.V1_QUERY_PATH; +import static java.nio.charset.StandardCharsets.UTF_8; public final class ProxyUtils { @@ -80,7 +81,7 @@ public static String getQueryUser(String userHeader, String authorization) return user; } - String info = new String(Base64.getDecoder().decode(headerInfo)); + String info = new String(Base64.getDecoder().decode(headerInfo), UTF_8); List parts = Splitter.on(':').limit(2).splitToList(info); if (parts.size() < 1) { log.error("No user inside the basic auth text"); @@ -94,7 +95,7 @@ public static String extractQueryIdIfPresent(HttpServletRequest request, List -XDcompilePolicy=simple -Xplugin:ErrorProne \ + -Xep:DefaultCharset:ERROR \ -Xep:UnnecessaryParentheses:ERROR \ -Xep:UnusedMethod:ERROR \ -Xep:UnusedVariable:ERROR From ee53978be5aafd942ecaec87d4c3aa3ff427ba0d Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 2 Oct 2024 07:30:32 +0900 Subject: [PATCH 2/6] Enable ErrorProne ClassCanBeStatic --- .../java/io/trino/gateway/ha/security/TestLbLdapClient.java | 2 +- .../src/test/java/io/trino/gateway/ha/security/TestOIDC.java | 2 +- pom.xml | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbLdapClient.java b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbLdapClient.java index e3cc441bb..78afeceeb 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbLdapClient.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbLdapClient.java @@ -145,7 +145,7 @@ void testMemberof() assertThat(lbLdapClient.getMemberOf(user)).isNotEqualTo("Admin,User"); } - class DummyPasswordWarning + static class DummyPasswordWarning implements org.apache.directory.ldap.client.template.PasswordWarning { @Override diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestOIDC.java b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestOIDC.java index ba1af43c7..2a42ad4fd 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestOIDC.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestOIDC.java @@ -233,7 +233,7 @@ public X509Certificate[] getAcceptedIssuers() clientBuilder.hostnameVerifier((hostname, session) -> true); } - public class BadCookieJar + public static class BadCookieJar implements CookieJar { private JavaNetCookieJar cookieJar; diff --git a/pom.xml b/pom.xml index 5e2dec2d8..bbcbb5377 100644 --- a/pom.xml +++ b/pom.xml @@ -100,6 +100,7 @@ -XDcompilePolicy=simple -Xplugin:ErrorProne \ + -Xep:ClassCanBeStatic:ERROR \ -Xep:DefaultCharset:ERROR \ -Xep:UnnecessaryParentheses:ERROR \ -Xep:UnusedMethod:ERROR \ From 858c3f08b79afd84586216d4c4d0d4da57c0744d Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 2 Oct 2024 07:32:45 +0900 Subject: [PATCH 3/6] Enable ErrorProne StringCaseLocaleUsage --- .../src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java | 3 ++- .../main/java/io/trino/gateway/ha/router/TrinoRequestUser.java | 3 ++- .../java/io/trino/gateway/ha/security/LbFormAuthManager.java | 3 ++- .../main/java/io/trino/gateway/ha/security/LbOAuthManager.java | 3 ++- pom.xml | 1 + 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java b/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java index ed752201a..448bd3e26 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/handler/ProxyUtils.java @@ -30,6 +30,7 @@ import static io.trino.gateway.ha.handler.QueryIdCachingProxyHandler.USER_HEADER; import static io.trino.gateway.ha.handler.QueryIdCachingProxyHandler.V1_QUERY_PATH; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Locale.ENGLISH; public final class ProxyUtils { @@ -97,7 +98,7 @@ public static String extractQueryIdIfPresent(HttpServletRequest request, List extractUserFromAuthorizationHeader(String header, Strin } } - if (header.toLowerCase().contains("bearer")) { + if (header.toLowerCase(ENGLISH).contains("bearer")) { return extractUserFromBearerAuth(header, userField); } return Optional.empty(); diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbFormAuthManager.java b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbFormAuthManager.java index e18e7ec50..a2f415c82 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbFormAuthManager.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbFormAuthManager.java @@ -33,6 +33,7 @@ import java.util.stream.Stream; import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static java.util.Locale.ENGLISH; public class LbFormAuthManager { @@ -52,7 +53,7 @@ public LbFormAuthManager(FormAuthConfiguration configuration, this.presetUsers = presetUsers; this.pagePermissions = pagePermissions.entrySet().stream() .filter(entry -> entry.getValue() != null) - .collect(toImmutableMap(entry -> entry.getKey().toUpperCase(), Map.Entry::getValue)); + .collect(toImmutableMap(entry -> entry.getKey().toUpperCase(ENGLISH), Map.Entry::getValue)); if (configuration != null) { this.lbKeyProvider = new LbKeyProvider(configuration diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java index a06a1042a..e9dacf694 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java @@ -55,6 +55,7 @@ import static jakarta.ws.rs.core.Response.Status.FOUND; import static jakarta.ws.rs.core.Response.Status.UNAUTHORIZED; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Locale.ENGLISH; public class LbOAuthManager { @@ -70,7 +71,7 @@ public LbOAuthManager(OAuthConfiguration configuration, Map page this.oauthConfig = configuration; this.pagePermissions = pagePermissions.entrySet().stream() .filter(entry -> entry.getValue() != null) - .collect(toImmutableMap(entry -> entry.getKey().toUpperCase(), Map.Entry::getValue)); + .collect(toImmutableMap(entry -> entry.getKey().toUpperCase(ENGLISH), Map.Entry::getValue)); } public String getUserIdField() diff --git a/pom.xml b/pom.xml index bbcbb5377..1c5f9bc6a 100644 --- a/pom.xml +++ b/pom.xml @@ -102,6 +102,7 @@ -Xplugin:ErrorProne \ -Xep:ClassCanBeStatic:ERROR \ -Xep:DefaultCharset:ERROR \ + -Xep:StringCaseLocaleUsage:ERROR \ -Xep:UnnecessaryParentheses:ERROR \ -Xep:UnusedMethod:ERROR \ -Xep:UnusedVariable:ERROR From cadb30c573f931337fd35f21214aba8aca7d82d7 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 2 Oct 2024 07:49:35 +0900 Subject: [PATCH 4/6] Enable ErrorProne MissingOverride --- .../src/main/java/io/trino/gateway/ha/security/LbFilter.java | 1 + pom.xml | 1 + 2 files changed, 2 insertions(+) diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbFilter.java b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbFilter.java index 1d7ead76d..2e82a7d4e 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbFilter.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbFilter.java @@ -52,6 +52,7 @@ public LbFilter(IdTokenAuthenticator idTokenAuthenticator, Authorizer lbAuthoriz * Filters requests by checking for the token cookie and authorization header, * and authenticates the user using the filter's authenticator. */ + @Override public void filter(final ContainerRequestContext requestContext) throws WebApplicationException { diff --git a/pom.xml b/pom.xml index 1c5f9bc6a..1bdf14fe8 100644 --- a/pom.xml +++ b/pom.xml @@ -102,6 +102,7 @@ -Xplugin:ErrorProne \ -Xep:ClassCanBeStatic:ERROR \ -Xep:DefaultCharset:ERROR \ + -Xep:MissingOverride:ERROR \ -Xep:StringCaseLocaleUsage:ERROR \ -Xep:UnnecessaryParentheses:ERROR \ -Xep:UnusedMethod:ERROR \ From 41fdc73676427e4c5a6683c22e12996fbf1fad94 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 2 Oct 2024 07:54:09 +0900 Subject: [PATCH 5/6] Disable ErrorProne EqualsGetClass We would rather want the opposite check. --- pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pom.xml b/pom.xml index 1bdf14fe8..8fe58b2c8 100644 --- a/pom.xml +++ b/pom.xml @@ -102,6 +102,7 @@ -Xplugin:ErrorProne \ -Xep:ClassCanBeStatic:ERROR \ -Xep:DefaultCharset:ERROR \ + -Xep:EqualsGetClass:OFF \ -Xep:MissingOverride:ERROR \ -Xep:StringCaseLocaleUsage:ERROR \ -Xep:UnnecessaryParentheses:ERROR \ From f88ea99ffd32ea49a5ce222a93c74e7f44a9042c Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 2 Oct 2024 07:48:16 +0900 Subject: [PATCH 6/6] Fix deprecated warning --- .../java/io/trino/gateway/ha/security/LbOAuthManager.java | 3 ++- .../test/java/io/trino/gateway/ha/HaGatewayTestUtils.java | 4 ++-- .../ha/TestGatewayHaWithRoutingRulesSingleBackend.java | 2 +- .../test/java/io/trino/gateway/ha/TestNoXForwarded.java | 2 +- .../test/java/io/trino/gateway/ha/TestTrinoResource.java | 8 ++++---- .../io/trino/gateway/ha/security/TestAuthorization.java | 2 +- .../java/io/trino/gateway/ha/security/TestLbFilter.java | 2 +- .../io/trino/gateway/ha/security/TestLbLdapClient.java | 2 +- 8 files changed, 13 insertions(+), 12 deletions(-) diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java index e9dacf694..0bf8a42c4 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbOAuthManager.java @@ -98,7 +98,8 @@ public Response exchangeCodeForToken(String code, String nonce, String redirectL TokenRequest tokenRequest = new TokenRequest( oauthConfig.getTokenEndpoint(), new ClientSecretBasic(new ClientID(oauthConfig.getClientId()), new Secret(oauthConfig.getClientSecret())), - new AuthorizationCodeGrant(new AuthorizationCode(code), oauthConfig.getRedirectUrl())); + new AuthorizationCodeGrant(new AuthorizationCode(code), oauthConfig.getRedirectUrl()), + null); TokenResponse tokenResponse; try { diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/HaGatewayTestUtils.java b/gateway-ha/src/test/java/io/trino/gateway/ha/HaGatewayTestUtils.java index d158770d8..ea390a68d 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/HaGatewayTestUtils.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/HaGatewayTestUtils.java @@ -140,7 +140,6 @@ public static void setUpBackend( { RequestBody requestBody = RequestBody.create( - MediaType.parse("application/json; charset=utf-8"), "{ \"name\": \"" + name + "\",\"proxyTo\": \"" @@ -151,7 +150,8 @@ public static void setUpBackend( + active + ",\"routingGroup\": \"" + routingGroup - + "\"}"); + + "\"}", + MediaType.parse("application/json; charset=utf-8")); Request request = new Request.Builder() .url("http://localhost:" + routerPort + "/entity?entityType=GATEWAY_BACKEND") diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/TestGatewayHaWithRoutingRulesSingleBackend.java b/gateway-ha/src/test/java/io/trino/gateway/ha/TestGatewayHaWithRoutingRulesSingleBackend.java index 830a08b99..ee97716f1 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/TestGatewayHaWithRoutingRulesSingleBackend.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/TestGatewayHaWithRoutingRulesSingleBackend.java @@ -62,7 +62,7 @@ void testRequestDelivery() throws Exception { RequestBody requestBody = - RequestBody.create(MediaType.parse("application/json; charset=utf-8"), "SELECT * from system.runtime.nodes"); + RequestBody.create("SELECT * from system.runtime.nodes", MediaType.parse("application/json; charset=utf-8")); Request request = new Request.Builder() .url("http://localhost:" + routerPort + "/v1/statement") diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/TestNoXForwarded.java b/gateway-ha/src/test/java/io/trino/gateway/ha/TestNoXForwarded.java index aec0e9e2a..e4d4cdb2c 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/TestNoXForwarded.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/TestNoXForwarded.java @@ -64,7 +64,7 @@ void testRequestDelivery() throws Exception { RequestBody requestBody = - RequestBody.create(MediaType.parse("application/json; charset=utf-8"), "SELECT 1"); + RequestBody.create("SELECT 1", MediaType.parse("application/json; charset=utf-8")); Request request = new Request.Builder() .url("http://localhost:" + routerPort + "/v1/statement") diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/TestTrinoResource.java b/gateway-ha/src/test/java/io/trino/gateway/ha/TestTrinoResource.java index ac9256766..856ebf2a9 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/TestTrinoResource.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/TestTrinoResource.java @@ -214,7 +214,7 @@ void testDeleteResourceGroupOk() throws Exception { RequestBody requestBody = - RequestBody.create(MediaType.parse("application/json; charset=utf-8"), ""); + RequestBody.create("", MediaType.parse("application/json; charset=utf-8")); Request request = new Request.Builder() .url("http://localhost:" + routerPort + "/trino/resourcegroup/delete/3") @@ -230,7 +230,7 @@ void testDeleteResourceGroupNoId() throws Exception { RequestBody requestBody = - RequestBody.create(MediaType.parse("application/json; charset=utf-8"), ""); + RequestBody.create("", MediaType.parse("application/json; charset=utf-8")); Request request = new Request.Builder() .url("http://localhost:" + routerPort + "/trino/resourcegroup/delete/") @@ -246,7 +246,7 @@ void testDeleteGlobalPropertyOk() throws Exception { RequestBody requestBody = - RequestBody.create(MediaType.parse("application/json; charset=utf-8"), ""); + RequestBody.create("", MediaType.parse("application/json; charset=utf-8")); Request request = new Request.Builder() .url("http://localhost:" + routerPort + "/trino/globalproperty/delete/cpu_quota_period") @@ -262,7 +262,7 @@ void testDeleteGlobalPropertyNoName() throws Exception { RequestBody requestBody = - RequestBody.create(MediaType.parse("application/json; charset=utf-8"), ""); + RequestBody.create("", MediaType.parse("application/json; charset=utf-8")); Request request = new Request.Builder() .url("http://localhost:" + routerPort + "/trino/globalproperty/delete/") diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestAuthorization.java b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestAuthorization.java index 178725536..73bca6633 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestAuthorization.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestAuthorization.java @@ -101,7 +101,7 @@ private List getRoles(String credentials) private Response makeRequest(Optional credentials) throws IOException { - RequestBody emptyRequestBody = RequestBody.create(MediaType.parse("application/json; charset=utf-8"), ""); + RequestBody emptyRequestBody = RequestBody.create("", MediaType.parse("application/json; charset=utf-8")); Request.Builder builder = new Request.Builder() .url("http://localhost:" + routerPort + "/userinfo") .post(emptyRequestBody); diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbFilter.java b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbFilter.java index bce9c62ef..20611a84f 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbFilter.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbFilter.java @@ -81,7 +81,7 @@ void testSuccessfulCookieAuthentication() .when(requestContext.getCookies()) .thenReturn( Map.of(SessionCookie.OAUTH_ID_TOKEN, - new Cookie(SessionCookie.OAUTH_ID_TOKEN, ID_TOKEN))); + new Cookie.Builder(SessionCookie.OAUTH_ID_TOKEN).value(ID_TOKEN).build())); Mockito .when(requestContext.getHeaders()) .thenReturn(new MultivaluedHashMap()); diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbLdapClient.java b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbLdapClient.java index 78afeceeb..320154290 100644 --- a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbLdapClient.java +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbLdapClient.java @@ -50,7 +50,7 @@ final class TestLbLdapClient public void initMocks() { log.info("initializing test"); - org.mockito.MockitoAnnotations.initMocks(this); + org.mockito.MockitoAnnotations.openMocks(this); } @AfterEach