From 4bbae8559c1e63cbcc6da967bee4792618bf4936 Mon Sep 17 00:00:00 2001 From: Adarsh Sanjeev Date: Wed, 19 Mar 2025 15:09:30 +0530 Subject: [PATCH] Fix utils --- .../org/apache/druid/server/JettyUtils.java | 10 ++++++- .../AsyncManagementForwardingServletTest.java | 28 +++++++++++++++++-- .../apache/druid/server/JettyUtilsTest.java | 24 ++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/JettyUtils.java b/server/src/main/java/org/apache/druid/server/JettyUtils.java index 717eb136b43a..0f503ef3b2fc 100644 --- a/server/src/main/java/org/apache/druid/server/JettyUtils.java +++ b/server/src/main/java/org/apache/druid/server/JettyUtils.java @@ -33,9 +33,13 @@ public class JettyUtils * Concatenate URI parts, in a way that is useful for proxy servlets. * * @param base base part of the uri, like http://example.com (no trailing slash) - * @param encodedPath encoded path, like you would get from HttpServletRequest's getRequestURI + * @param encodedPath encoded path, like you would get from HttpServletRequest's getRequestURI. Must start with + * a slash. * @param encodedQueryString encoded query string, like you would get from HttpServletRequest's getQueryString + * + * @return rewritten target URI, or null if the URI cannot be rewritten */ + @Nullable public static String concatenateForRewrite( final String base, final String encodedPath, @@ -44,6 +48,10 @@ public static String concatenateForRewrite( { // Query string and path are already encoded, no need for anything fancy beyond string concatenation. + if (!encodedPath.startsWith("/")) { + return null; + } + final StringBuilder url = new StringBuilder(base).append(encodedPath); if (encodedQueryString != null) { diff --git a/server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java b/server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java index ba7c78b99b0a..b49520021441 100644 --- a/server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java +++ b/server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java @@ -351,12 +351,36 @@ public void testBadProxyDestination() throws Exception Assert.assertFalse("overlord called", OVERLORD_EXPECTED_REQUEST.called); } + @Test + public void testCoordinatorNoPath() throws Exception + { + HttpURLConnection connection = ((HttpURLConnection) + new URL(StringUtils.format("http://localhost:%d/proxy/coordinator", port)).openConnection()); + connection.setRequestMethod("GET"); + + Assert.assertEquals(403, connection.getResponseCode()); // proxy with no path is not allowed + Assert.assertFalse("coordinator called", COORDINATOR_EXPECTED_REQUEST.called); + Assert.assertFalse("overlord called", OVERLORD_EXPECTED_REQUEST.called); + } + + @Test + public void testOverlordNoPath() throws Exception + { + HttpURLConnection connection = ((HttpURLConnection) + new URL(StringUtils.format("http://localhost:%d/proxy/overlord", port)).openConnection()); + connection.setRequestMethod("GET"); + + Assert.assertEquals(403, connection.getResponseCode()); // proxy with no path is not allowed + Assert.assertFalse("coordinator called", COORDINATOR_EXPECTED_REQUEST.called); + Assert.assertFalse("overlord called", OVERLORD_EXPECTED_REQUEST.called); + } + @Test public void testCoordinatorLeaderUnknown() throws Exception { isValidLeader = false; HttpURLConnection connection = ((HttpURLConnection) - new URL(StringUtils.format("http://localhost:%d/druid/coordinator", port)).openConnection()); + new URL(StringUtils.format("http://localhost:%d/druid/coordinator/status", port)).openConnection()); connection.setRequestMethod("GET"); Assert.assertEquals(503, connection.getResponseCode()); @@ -369,7 +393,7 @@ public void testOverlordLeaderUnknown() throws Exception { isValidLeader = false; HttpURLConnection connection = ((HttpURLConnection) - new URL(StringUtils.format("http://localhost:%d/druid/indexer", port)).openConnection()); + new URL(StringUtils.format("http://localhost:%d/druid/indexer/status", port)).openConnection()); connection.setRequestMethod("GET"); Assert.assertEquals(503, connection.getResponseCode()); diff --git a/server/src/test/java/org/apache/druid/server/JettyUtilsTest.java b/server/src/test/java/org/apache/druid/server/JettyUtilsTest.java index bd6d86f36bc7..de1cb2bf0b3f 100644 --- a/server/src/test/java/org/apache/druid/server/JettyUtilsTest.java +++ b/server/src/test/java/org/apache/druid/server/JettyUtilsTest.java @@ -36,4 +36,28 @@ public void testConcatenateForRewrite() ) ); } + + @Test + public void testConcatenateForRewriteEmptyPath() + { + Assert.assertNull( + JettyUtils.concatenateForRewrite( + "http://example.com", + "", + "q=baz%20qux" + ) + ); + } + + @Test + public void testConcatenateForRewriteInvalidPath() + { + Assert.assertNull( + JettyUtils.concatenateForRewrite( + "http://example.com", + "foo%20bar", // path must start with '/' + "q=baz%20qux" + ) + ); + } }