Introduce rawPath in RequestTarget and ServiceRequestContext (#5932)

yzfeng2020 · web-flow · commit 95ed9e1cfb07 · 2024-11-12T00:10:59.000+09:00
Motivation: It may be useful if we have access to the original request path unmodified. For example we have use cases where the decoding rules are different from the ones at `decodedPath`. Modifications: - Added `rawPath` method in `RequestTarget` to store the rawPath. - The rawPath is from the unmodified ':path' header at the server side; the client side target, it's always null. - Added `rawPath` method in `ServiceRequestContext`, it's always non null for server-side targets. Result: - Closes #5931. - Users will get access to the raw request path for a request.
diff --git 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(), null,
+                    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
@@ -131,6 +131,14 @@ static RequestTarget forClient(String reqTarget, @Nullable String prefix) {
      */
     String maybePathWithMatrixVariables();
 
+    /**
+     * Returns the server-side raw path of this {@link RequestTarget} from the ":path" header.
+     * Unlike {@link #path()}, the returned string is the original path without any normalization.
+     * For client-side target it always returns {@code null}.
+     */
+    @Nullable
+    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
@@ -163,16 +163,8 @@ boolean mustPreserveEncoding(int cp) {
     private static final Bytes EMPTY_BYTES = new Bytes(0);
     private static final Bytes SLASH_BYTES = new Bytes(new byte[] { '/' });
 
-    private static final RequestTarget INSTANCE_ASTERISK = createWithoutValidation(
-            RequestTargetForm.ASTERISK,
-            null,
-            null,
-            null,
-            -1,
-            "*",
-            "*",
-            null,
-            null);
+    private static final RequestTarget CLIENT_INSTANCE_ASTERISK = createAsterisk(false);
+    private static final RequestTarget SERVER_INSTANCE_ASTERISK = createAsterisk(true);
 
     /**
      * The main implementation of {@link RequestTarget#forServer(String)}.
@@ -233,9 +225,9 @@ 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 query, @Nullable String fragment) {
+            @Nullable 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;
@@ -249,14 +241,16 @@ public static RequestTarget createWithoutValidation(
     private final String path;
     private final String maybePathWithMatrixVariables;
     @Nullable
+    private final String rawPath;
+    @Nullable
     private final String query;
     @Nullable
     private final String fragment;
     private boolean cached;
 
     private DefaultRequestTarget(RequestTargetForm form, @Nullable String scheme,
                                  @Nullable String authority, @Nullable String host, int port,
-                                 String path, String maybePathWithMatrixVariables,
+                                 String path, String maybePathWithMatrixVariables, @Nullable String rawPath,
                                  @Nullable String query, @Nullable String fragment) {
 
         assert (scheme != null && authority != null && host != null) ||
@@ -270,6 +264,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 +307,12 @@ public String maybePathWithMatrixVariables() {
         return maybePathWithMatrixVariables;
     }
 
+    @Override
+    @Nullable
+    public String rawPath() {
+        return rawPath;
+    }
+
     @Nullable
     @Override
     public String query() {
@@ -380,6 +381,21 @@ public String toString() {
         }
     }
 
+    private static RequestTarget createAsterisk(boolean server) {
+        final String rawPath = server ? "*" : null;
+        return createWithoutValidation(
+                RequestTargetForm.ASTERISK,
+                null,
+                null,
+                null,
+                -1,
+                "*",
+                "*",
+                rawPath,
+                null,
+                null);
+    }
+
     @Nullable
     private static RequestTarget slowForServer(String reqTarget, boolean allowSemicolonInPathComponent,
                                                boolean allowDoubleDotsInQueryString) {
@@ -411,7 +427,7 @@ private static RequestTarget slowForServer(String reqTarget, boolean allowSemico
         // Reject a relative path and accept an asterisk (e.g. OPTIONS * HTTP/1.1).
         if (isRelativePath(path)) {
             if (query == null && path.length == 1 && path.data[0] == '*') {
-                return INSTANCE_ASTERISK;
+                return SERVER_INSTANCE_ASTERISK;
             } else {
                 // Do not accept a relative path.
                 return null;
@@ -443,6 +459,7 @@ private static RequestTarget slowForServer(String reqTarget, boolean allowSemico
                                         -1,
                                         matrixVariablesRemovedPath,
                                         encodedPath,
+                                        reqTarget,
                                         encodeQueryToPercents(query),
                                         null);
     }
@@ -622,7 +639,7 @@ private static RequestTarget slowForClient(String reqTarget,
 
         // Accept an asterisk (e.g. OPTIONS * HTTP/1.1).
         if (query == null && path.length == 1 && path.data[0] == '*') {
-            return INSTANCE_ASTERISK;
+            return CLIENT_INSTANCE_ASTERISK;
         }
 
         final String encodedPath;
@@ -645,7 +662,9 @@ private static RequestTarget slowForClient(String reqTarget,
                                             null,
                                             -1,
                                             encodedPath,
-                                            encodedPath, encodedQuery,
+                                            encodedPath,
+                                            null,
+                                            encodedQuery,
                                             encodedFragment);
         }
     }
@@ -692,6 +711,7 @@ private static DefaultRequestTarget newAbsoluteTarget(
                                         port,
                                         encodedPath,
                                         encodedPath,
+                                        null,
                                         encodedQuery,
                                         encodedFragment);
     }
diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java
@@ -305,6 +305,14 @@ public String decodedMappedPath() {
         return routingResult.decodedPath();
     }
 
+    @Override
+    public String rawPath() {
+        final String rawPath = requestTarget().rawPath();
+        // rawPath should not be null for server-side targets.
+        assert rawPath != null;
+        return rawPath;
+    }
+
     @Override
     public URI uri() {
         final HttpRequest request = request();
diff --git 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/main/java/com/linecorp/armeria/server/ServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContext.java
@@ -335,6 +335,11 @@ default String queryParam(String name) {
      */
     String decodedMappedPath();
 
+    /**
+     * Returns the original path without normalization from the ":path" header.
+     */
+    String rawPath();
+
     /**
      * Returns the {@link URI} associated with the current {@link Request}.
      * Note that this method is a shortcut of calling {@link HttpRequest#uri()} on {@link #request()}.
diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextWrapper.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextWrapper.java
@@ -121,6 +121,11 @@ public String decodedMappedPath() {
         return unwrap().decodedMappedPath();
     }
 
+    @Override
+    public String rawPath() {
+        return unwrap().rawPath();
+    }
+
     @Nullable
     @Override
     public MediaType negotiatedResponseMediaType() {
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
@@ -143,27 +143,27 @@ 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", "..=")
                 );
             });
 
             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", "=..")
                 );
             });
 
             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 {
+        @Nullable
+        final String rawPath;
+        @Nullable
+        final RequestTarget requestTarget;
+
+        RequestTargetWithRawPath(@Nullable 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;
+        return new RequestTargetWithRawPath(null, target);
     }
 
     private static String toAbsolutePath(String pattern) {
diff --git a/core/src/test/java/com/linecorp/armeria/server/RawPathTest.java b/core/src/test/java/com/linecorp/armeria/server/RawPathTest.java

Original file line number	Diff line number	Diff line change
`@@ -155,7 +155,8 @@ protected AbstractRequestContextBuilder(boolean server, RpcRequest rpcReq, URI u`
`155`	`155`	`} else {`
`156`	`156`	`reqTarget = DefaultRequestTarget.createWithoutValidation(`
`157`	`157`	`RequestTargetForm.ORIGIN, null, null, null, -1,`
`158`		`- uri.getRawPath(), uri.getRawPath(), uri.getRawQuery(), uri.getRawFragment());`
	`158`	`+ uri.getRawPath(), uri.getRawPath(), null,`
	`159`	`+ uri.getRawQuery(), uri.getRawFragment());`
`159`	`160`	`}`
`160`	`161`	`}`
`161`	`162`