Skip to content

Commit d40b81f

Browse files
committed
Add minor checks in jetty utils class
1 parent 092f3f2 commit d40b81f

File tree

3 files changed

+59
-3
lines changed

3 files changed

+59
-3
lines changed

server/src/main/java/org/apache/druid/server/JettyUtils.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,13 @@ public class JettyUtils
3333
* Concatenate URI parts, in a way that is useful for proxy servlets.
3434
*
3535
* @param base base part of the uri, like http://example.com (no trailing slash)
36-
* @param encodedPath encoded path, like you would get from HttpServletRequest's getRequestURI
36+
* @param encodedPath encoded path, like you would get from HttpServletRequest's getRequestURI. Must start with
37+
* a slash.
3738
* @param encodedQueryString encoded query string, like you would get from HttpServletRequest's getQueryString
39+
*
40+
* @return rewritten target URI, or null if the URI cannot be rewritten
3841
*/
42+
@Nullable
3943
public static String concatenateForRewrite(
4044
final String base,
4145
final String encodedPath,
@@ -44,6 +48,10 @@ public static String concatenateForRewrite(
4448
{
4549
// Query string and path are already encoded, no need for anything fancy beyond string concatenation.
4650

51+
if (!encodedPath.startsWith("/")) {
52+
return null;
53+
}
54+
4755
final StringBuilder url = new StringBuilder(base).append(encodedPath);
4856

4957
if (encodedQueryString != null) {

server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,12 +351,36 @@ public void testBadProxyDestination() throws Exception
351351
Assert.assertFalse("overlord called", OVERLORD_EXPECTED_REQUEST.called);
352352
}
353353

354+
@Test
355+
public void testCoordinatorNoPath() throws Exception
356+
{
357+
HttpURLConnection connection = ((HttpURLConnection)
358+
new URL(StringUtils.format("http://localhost:%d/proxy/coordinator", port)).openConnection());
359+
connection.setRequestMethod("GET");
360+
361+
Assert.assertEquals(403, connection.getResponseCode()); // proxy with no path is not allowed
362+
Assert.assertFalse("coordinator called", COORDINATOR_EXPECTED_REQUEST.called);
363+
Assert.assertFalse("overlord called", OVERLORD_EXPECTED_REQUEST.called);
364+
}
365+
366+
@Test
367+
public void testOverlordNoPath() throws Exception
368+
{
369+
HttpURLConnection connection = ((HttpURLConnection)
370+
new URL(StringUtils.format("http://localhost:%d/proxy/overlord", port)).openConnection());
371+
connection.setRequestMethod("GET");
372+
373+
Assert.assertEquals(403, connection.getResponseCode()); // proxy with no path is not allowed
374+
Assert.assertFalse("coordinator called", COORDINATOR_EXPECTED_REQUEST.called);
375+
Assert.assertFalse("overlord called", OVERLORD_EXPECTED_REQUEST.called);
376+
}
377+
354378
@Test
355379
public void testCoordinatorLeaderUnknown() throws Exception
356380
{
357381
isValidLeader = false;
358382
HttpURLConnection connection = ((HttpURLConnection)
359-
new URL(StringUtils.format("http://localhost:%d/druid/coordinator", port)).openConnection());
383+
new URL(StringUtils.format("http://localhost:%d/druid/coordinator/status", port)).openConnection());
360384
connection.setRequestMethod("GET");
361385

362386
Assert.assertEquals(503, connection.getResponseCode());
@@ -369,7 +393,7 @@ public void testOverlordLeaderUnknown() throws Exception
369393
{
370394
isValidLeader = false;
371395
HttpURLConnection connection = ((HttpURLConnection)
372-
new URL(StringUtils.format("http://localhost:%d/druid/indexer", port)).openConnection());
396+
new URL(StringUtils.format("http://localhost:%d/druid/indexer/status", port)).openConnection());
373397
connection.setRequestMethod("GET");
374398

375399
Assert.assertEquals(503, connection.getResponseCode());

server/src/test/java/org/apache/druid/server/JettyUtilsTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,28 @@ public void testConcatenateForRewrite()
3636
)
3737
);
3838
}
39+
40+
@Test
41+
public void testConcatenateForRewriteEmptyPath()
42+
{
43+
Assert.assertNull(
44+
JettyUtils.concatenateForRewrite(
45+
"http://example.com",
46+
"",
47+
"q=baz%20qux"
48+
)
49+
);
50+
}
51+
52+
@Test
53+
public void testConcatenateForRewriteInvalidPath()
54+
{
55+
Assert.assertNull(
56+
JettyUtils.concatenateForRewrite(
57+
"http://example.com",
58+
"foo%20bar", // path must start with '/'
59+
"q=baz%20qux"
60+
)
61+
);
62+
}
3963
}

0 commit comments

Comments
 (0)