From d4637614abca44a8b5fdc088d4c8ab803de70d40 Mon Sep 17 00:00:00 2001 From: Yizhou Feng Date: Wed, 2 Oct 2024 09:02:59 +0000 Subject: [PATCH] add rawPath to RequestTarget --- .../common/AbstractRequestContextBuilder.java | 3 +- .../armeria/common/RequestTarget.java | 6 + .../internal/common/DefaultRequestTarget.java | 20 +++- .../armeria/server/RoutingContext.java | 1 + .../common/DefaultRequestTargetTest.java | 110 +++++++++++------- 5 files changed, 90 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java index fa49b3101be..7136b41d41a 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java @@ -155,7 +155,8 @@ protected AbstractRequestContextBuilder(boolean server, RpcRequest rpcReq, URI u } else { reqTarget = DefaultRequestTarget.createWithoutValidation( RequestTargetForm.ORIGIN, null, null, null, -1, - uri.getRawPath(), uri.getRawPath(), uri.getRawQuery(), uri.getRawFragment()); + uri.getRawPath(), uri.getRawPath(), uri.toString(), + uri.getRawQuery(), uri.getRawFragment()); } } diff --git a/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java b/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java index 5b7241a52f8..b2a869bc7a9 100644 --- a/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java +++ b/core/src/main/java/com/linecorp/armeria/common/RequestTarget.java @@ -131,6 +131,12 @@ static RequestTarget forClient(String reqTarget, @Nullable String prefix) { */ String maybePathWithMatrixVariables(); + /** + * Returns the raw path of this {@link RequestTarget}, which always starts with {@code '/'}. + * Unlike {@link #path()}, the returned string is the original request path without any normalization. + */ + String rawPath(); + /** * Returns the query of this {@link RequestTarget}. */ diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java index d5006ce9157..1eb65b38724 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java @@ -171,6 +171,7 @@ boolean mustPreserveEncoding(int cp) { -1, "*", "*", + "*", null, null); @@ -232,10 +233,10 @@ public static RequestTarget forClient(String reqTarget, @Nullable String prefix) */ public static RequestTarget createWithoutValidation( RequestTargetForm form, @Nullable String scheme, @Nullable String authority, - @Nullable String host, int port, String path, String pathWithMatrixVariables, + @Nullable String host, int port, String path, String pathWithMatrixVariables, String rawPath, @Nullable String query, @Nullable String fragment) { return new DefaultRequestTarget( - form, scheme, authority, host, port, path, pathWithMatrixVariables, query, fragment); + form, scheme, authority, host, port, path, pathWithMatrixVariables, rawPath, query, fragment); } private final RequestTargetForm form; @@ -248,6 +249,7 @@ public static RequestTarget createWithoutValidation( private final int port; private final String path; private final String maybePathWithMatrixVariables; + private final String rawPath; @Nullable private final String query; @Nullable @@ -256,7 +258,7 @@ public static RequestTarget createWithoutValidation( private DefaultRequestTarget(RequestTargetForm form, @Nullable String scheme, @Nullable String authority, @Nullable String host, int port, - String path, String maybePathWithMatrixVariables, + String path,String maybePathWithMatrixVariables, String rawPath, @Nullable String query, @Nullable String fragment) { assert (scheme != null && authority != null && host != null) || @@ -270,6 +272,7 @@ private DefaultRequestTarget(RequestTargetForm form, @Nullable String scheme, this.port = port; this.path = path; this.maybePathWithMatrixVariables = maybePathWithMatrixVariables; + this.rawPath = rawPath; this.query = query; this.fragment = fragment; } @@ -312,6 +315,11 @@ public String maybePathWithMatrixVariables() { return maybePathWithMatrixVariables; } + @Override + public String rawPath() { + return rawPath; + } + @Nullable @Override public String query() { @@ -443,6 +451,7 @@ private static RequestTarget slowForServer(String reqTarget, boolean allowSemico -1, matrixVariablesRemovedPath, encodedPath, + reqTarget, encodeQueryToPercents(query), null); } @@ -645,7 +654,9 @@ private static RequestTarget slowForClient(String reqTarget, null, -1, encodedPath, - encodedPath, encodedQuery, + encodedPath, + encodedPath, + encodedQuery, encodedFragment); } } @@ -692,6 +703,7 @@ private static DefaultRequestTarget newAbsoluteTarget( port, encodedPath, encodedPath, + encodedPath, encodedQuery, encodedFragment); } diff --git a/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java b/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java index af242602af0..df43b8803e2 100644 --- a/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java +++ b/core/src/main/java/com/linecorp/armeria/server/RoutingContext.java @@ -169,6 +169,7 @@ default RoutingContext withPath(String path) { oldReqTarget.port(), pathWithoutMatrixVariables, path, + path, oldReqTarget.query(), oldReqTarget.fragment()); diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java index f959f039552..7df6eff13a7 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java @@ -143,9 +143,9 @@ void serverShouldAcceptGoodDoubleDotPatterns(String pattern) { void dotsAndEqualsInNameValueQuery() { QUERY_SEPARATORS.forEach(qs -> { assertThat(forServer("/?a=..=" + qs + "b=..=")).satisfies(res -> { - assertThat(res).isNotNull(); - assertThat(res.query()).isEqualTo("a=..=" + qs + "b=..="); - assertThat(QueryParams.fromQueryString(res.query(), true)).containsExactly( + assertThat(res.requestTarget).isNotNull(); + assertThat(res.requestTarget.query()).isEqualTo("a=..=" + qs + "b=..="); + assertThat(QueryParams.fromQueryString(res.requestTarget.query(), true)).containsExactly( Maps.immutableEntry("a", "..="), Maps.immutableEntry("b", "..=") ); @@ -153,8 +153,8 @@ void dotsAndEqualsInNameValueQuery() { assertThat(forServer("/?a==.." + qs + "b==..")).satisfies(res -> { assertThat(res).isNotNull(); - assertThat(res.query()).isEqualTo("a==.." + qs + "b==.."); - assertThat(QueryParams.fromQueryString(res.query(), true)).containsExactly( + assertThat(res.requestTarget.query()).isEqualTo("a==.." + qs + "b==.."); + assertThat(QueryParams.fromQueryString(res.requestTarget.query(), true)).containsExactly( Maps.immutableEntry("a", "=.."), Maps.immutableEntry("b", "=..") ); @@ -162,8 +162,8 @@ void dotsAndEqualsInNameValueQuery() { assertThat(forServer("/?a==..=" + qs + "b==..=")).satisfies(res -> { assertThat(res).isNotNull(); - assertThat(res.query()).isEqualTo("a==..=" + qs + "b==..="); - assertThat(QueryParams.fromQueryString(res.query(), true)).containsExactly( + assertThat(res.requestTarget.query()).isEqualTo("a==..=" + qs + "b==..="); + assertThat(QueryParams.fromQueryString(res.requestTarget.query(), true)).containsExactly( Maps.immutableEntry("a", "=..="), Maps.immutableEntry("b", "=..=") ); @@ -500,9 +500,9 @@ void clientShouldAcceptAbsoluteUri(String uri, String expectedScheme, String expectedAuthority, String expectedPath, @Nullable String expectedQuery, @Nullable String expectedFragment) { - final RequestTarget res = forClient(uri); - assertThat(res.scheme()).isEqualTo(expectedScheme); - assertThat(res.authority()).isEqualTo(expectedAuthority); + final RequestTargetWithRawPath res = forClient(uri); + assertThat(res.requestTarget.scheme()).isEqualTo(expectedScheme); + assertThat(res.requestTarget.authority()).isEqualTo(expectedAuthority); assertAccepted(res, expectedPath, emptyToNull(expectedQuery), emptyToNull(expectedFragment)); } @@ -531,15 +531,15 @@ void shouldYieldEmptyStringForEmptyQueryAndFragment(Mode mode) { @ParameterizedTest @EnumSource(Mode.class) void testToString(Mode mode) { - assertThat(parse(mode, "/")).asString().isEqualTo("/"); - assertThat(parse(mode, "/?")).asString().isEqualTo("/?"); - assertThat(parse(mode, "/?a=b")).asString().isEqualTo("/?a=b"); + assertThat(parse(mode, "/").requestTarget).asString().isEqualTo("/"); + assertThat(parse(mode, "/?").requestTarget).asString().isEqualTo("/?"); + assertThat(parse(mode, "/?a=b").requestTarget).asString().isEqualTo("/?a=b"); if (mode == Mode.CLIENT) { - assertThat(forClient("/#")).asString().isEqualTo("/#"); - assertThat(forClient("/?#")).asString().isEqualTo("/?#"); - assertThat(forClient("/?a=b#c=d")).asString().isEqualTo("/?a=b#c=d"); - assertThat(forClient("http://foo/bar?a=b#c=d")).asString().isEqualTo("http://foo/bar?a=b#c=d"); + assertThat(forClient("/#").requestTarget).asString().isEqualTo("/#"); + assertThat(forClient("/?#").requestTarget).asString().isEqualTo("/?#"); + assertThat(forClient("/?a=b#c=d").requestTarget).asString().isEqualTo("/?a=b#c=d"); + assertThat(forClient("http://foo/bar?a=b#c=d").requestTarget).asString().isEqualTo("http://foo/bar?a=b#c=d"); } } @@ -572,32 +572,32 @@ void testRemoveMatrixVariables() { assertThat(removeMatrixVariables("/prefix/;a=b")).isNull(); } - private static void assertAccepted(@Nullable RequestTarget res, String expectedPath) { + private static void assertAccepted(RequestTargetWithRawPath res, String expectedPath) { assertAccepted(res, expectedPath, null, null); } - private static void assertAccepted(@Nullable RequestTarget res, + private static void assertAccepted(RequestTargetWithRawPath res, String expectedPath, @Nullable String expectedQuery) { assertAccepted(res, expectedPath, expectedQuery, null); } - private static void assertAccepted(@Nullable RequestTarget res, + private static void assertAccepted(RequestTargetWithRawPath res, String expectedPath, @Nullable String expectedQuery, @Nullable String expectedFragment) { - assertThat(res).isNotNull(); - assertThat(res.path()).isEqualTo(expectedPath); - assertThat(res.query()).isEqualTo(expectedQuery); - assertThat(res.fragment()).isEqualTo(expectedFragment); + assertThat(res.requestTarget).isNotNull(); + assertThat(res.requestTarget.path()).isEqualTo(expectedPath); + assertThat(res.requestTarget.query()).isEqualTo(expectedQuery); + assertThat(res.requestTarget.fragment()).isEqualTo(expectedFragment); + assertThat(res.requestTarget.rawPath()).isEqualTo(res.rawPath); } - private static void assertRejected(@Nullable RequestTarget res) { - assertThat(res).isNull(); + private static void assertRejected(RequestTargetWithRawPath res) { + assertThat(res.requestTarget).isNull(); } - @Nullable - private static RequestTarget parse(Mode mode, String rawPath) { + private static RequestTargetWithRawPath parse(Mode mode, String rawPath) { switch (mode) { case SERVER: return forServer(rawPath); @@ -608,37 +608,57 @@ private static RequestTarget parse(Mode mode, String rawPath) { } } - @Nullable - private static RequestTarget forServer(String rawPath) { + private static class RequestTargetWithRawPath { + final String rawPath; + @Nullable + final RequestTarget requestTarget; + + RequestTargetWithRawPath(String rawPath, @Nullable RequestTarget requestTarget) { + this.rawPath = rawPath; + this.requestTarget = requestTarget; + } + + @Override + public String toString() { + return "RequestTargetWithRawPath{" + + "rawPath='" + rawPath + '\'' + + ", requestTarget=" + requestTarget + + '}'; + } + } + + private static RequestTargetWithRawPath forServer(String rawPath) { return forServer(rawPath, false); } - @Nullable - private static RequestTarget forServer(String rawPath, boolean allowSemicolonInPathComponent) { - final RequestTarget res = DefaultRequestTarget.forServer(rawPath, allowSemicolonInPathComponent, false); - if (res != null) { - logger.info("forServer({}) => path: {}, query: {}", rawPath, res.path(), res.query()); + private static RequestTargetWithRawPath forServer(String rawPath, boolean allowSemicolonInPathComponent) { + final RequestTarget target = DefaultRequestTarget.forServer( + rawPath, + allowSemicolonInPathComponent, + false); + if (target != null) { + logger.info("forServer({}) => path: {}, query: {}", rawPath, target.path(), target.query()); } else { logger.info("forServer({}) => null", rawPath); } - return res; + return new RequestTargetWithRawPath(rawPath, target); } - @Nullable - private static RequestTarget forClient(String rawPath) { + private static RequestTargetWithRawPath forClient(String rawPath) { return forClient(rawPath, null); } - @Nullable - private static RequestTarget forClient(String rawPath, @Nullable String prefix) { - final RequestTarget res = DefaultRequestTarget.forClient(rawPath, prefix); - if (res != null) { - logger.info("forClient({}, {}) => path: {}, query: {}, fragment: {}", rawPath, prefix, res.path(), - res.query(), res.fragment()); + private static RequestTargetWithRawPath forClient(String rawPath, @Nullable String prefix) { + final RequestTarget target = DefaultRequestTarget.forClient(rawPath, prefix); + if (target != null) { + logger.info("forClient({}, {}) => path: {}, query: {}, fragment: {}", + rawPath, prefix, target.path(), + target.query(), target.fragment()); } else { logger.info("forClient({}, {}) => null", rawPath, prefix); } - return res; + String pathInTarget = target != null ? target.path() : "/"; + return new RequestTargetWithRawPath(pathInTarget, target); } private static String toAbsolutePath(String pattern) {