Merge pull request #37513 from rmartinc/issue-35960

geoand · web-flow · commit 5ce1abe22afa · 2023-12-05T12:24:39.000+02:00
Save pathParamValues encoded and perform decoding when requested
diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java
@@ -226,18 +226,20 @@ public void setMaxPathParams(int maxPathParams) {
         }
     }
 
-    public String getPathParam(int index) {
-        return doGetPathParam(index, pathParamValues);
+    public String getPathParam(int index, boolean encoded) {
+        return doGetPathParam(index, pathParamValues, encoded);
     }
 
-    private String doGetPathParam(int index, Object pathParamValues) {
+    private String doGetPathParam(int index, Object pathParamValues, boolean encoded) {
         if (pathParamValues instanceof String[]) {
-            return ((String[]) pathParamValues)[index];
+            String pathParam = ((String[]) pathParamValues)[index];
+            return encoded ? pathParam : Encode.decodePath(pathParam);
         }
         if (index > 1) {
             throw new IndexOutOfBoundsException();
         }
-        return (String) pathParamValues;
+        String pathParam = (String) pathParamValues;
+        return encoded ? pathParam : Encode.decodePath(pathParam);
     }
 
     public ResteasyReactiveRequestContext setPathParamValue(int index, String value) {
@@ -926,18 +928,11 @@ public String getPathParameter(String name, boolean encoded) {
         Integer index = target.getPathParameterIndexes().get(name);
         String value;
         if (index != null) {
-            value = getPathParam(index);
-        } else {
-            // Check previous resources if the path is not defined in the current target
-            value = getResourceLocatorPathParam(name);
-        }
-
-        // It's possible to inject a path param that's not defined, return null in this case
-        if (encoded && value != null) {
-            return Encode.encodeQueryParam(value);
+            return getPathParam(index, encoded);
         }
 
-        return value;
+        // Check previous resources if the path is not defined in the current target
+        return getResourceLocatorPathParam(name, encoded);
     }
 
     @Override
@@ -996,8 +991,8 @@ public ResteasyReactiveResourceInfo getResteasyReactiveResourceInfo() {
 
     public abstract Runnable registerTimer(long millis, Runnable task);
 
-    public String getResourceLocatorPathParam(String name) {
-        return getResourceLocatorPathParam(name, (PreviousResource) getProperty(PreviousResource.PROPERTY_KEY));
+    public String getResourceLocatorPathParam(String name, boolean encoded) {
+        return getResourceLocatorPathParam(name, (PreviousResource) getProperty(PreviousResource.PROPERTY_KEY), encoded);
     }
 
     public FormData getFormData() {
@@ -1009,7 +1004,7 @@ public ResteasyReactiveRequestContext setFormData(FormData formData) {
         return this;
     }
 
-    private String getResourceLocatorPathParam(String name, PreviousResource previousResource) {
+    private String getResourceLocatorPathParam(String name, PreviousResource previousResource, boolean encoded) {
         if (previousResource == null) {
             return null;
         }
@@ -1020,13 +1015,13 @@ private String getResourceLocatorPathParam(String name, PreviousResource previou
             for (URITemplate.TemplateComponent component : classPath.components) {
                 if (component.name != null) {
                     if (component.name.equals(name)) {
-                        return doGetPathParam(index, previousResource.locatorPathParamValues);
+                        return doGetPathParam(index, previousResource.locatorPathParamValues, encoded);
                     }
                     index++;
                 } else if (component.names != null) {
                     for (String nm : component.names) {
                         if (nm.equals(name)) {
-                            return doGetPathParam(index, previousResource.locatorPathParamValues);
+                            return doGetPathParam(index, previousResource.locatorPathParamValues, encoded);
                         }
                     }
                     index++;
@@ -1036,19 +1031,19 @@ private String getResourceLocatorPathParam(String name, PreviousResource previou
         for (URITemplate.TemplateComponent component : previousResource.locatorTarget.getPath().components) {
             if (component.name != null) {
                 if (component.name.equals(name)) {
-                    return doGetPathParam(index, previousResource.locatorPathParamValues);
+                    return doGetPathParam(index, previousResource.locatorPathParamValues, encoded);
                 }
                 index++;
             } else if (component.names != null) {
                 for (String nm : component.names) {
                     if (nm.equals(name)) {
-                        return doGetPathParam(index, previousResource.locatorPathParamValues);
+                        return doGetPathParam(index, previousResource.locatorPathParamValues, encoded);
                     }
                 }
                 index++;
             }
         }
-        return getResourceLocatorPathParam(name, previousResource.prev);
+        return getResourceLocatorPathParam(name, previousResource.prev, encoded);
     }
 
     public abstract boolean resumeExternalProcessing();
diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/parameters/LocatableResourcePathParamExtractor.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/parameters/LocatableResourcePathParamExtractor.java
@@ -12,7 +12,7 @@ public LocatableResourcePathParamExtractor(String name) {
 
     @Override
     public Object extractParameter(ResteasyReactiveRequestContext context) {
-        return context.getResourceLocatorPathParam(name);
+        return context.getResourceLocatorPathParam(name, false);
     }
 
 }
diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/parameters/PathParamExtractor.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/parameters/PathParamExtractor.java
@@ -1,6 +1,8 @@
 package org.jboss.resteasy.reactive.server.core.parameters;
 
+import java.util.Arrays;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import org.jboss.resteasy.reactive.common.util.Encode;
 import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;
@@ -19,14 +21,14 @@ public PathParamExtractor(int index, boolean encoded, boolean single) {
 
     @Override
     public Object extractParameter(ResteasyReactiveRequestContext context) {
-        String pathParam = context.getPathParam(index);
-        if (encoded) {
-            pathParam = Encode.encodeQueryParam(pathParam);
-        }
+        String pathParam = context.getPathParam(index, true);
         if (single) {
-            return pathParam;
+            return encoded ? pathParam : Encode.decodePath(pathParam);
         } else {
-            return List.of(pathParam.split("/"));
+            return encoded
+                    ? List.of(pathParam.split("/"))
+                    : Arrays.stream(pathParam.split("/")).map(Encode::decodePath)
+                            .collect(Collectors.toList());
         }
     }
 }
diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/UriInfoImpl.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/jaxrs/UriInfoImpl.java
@@ -152,7 +152,7 @@ public MultivaluedMap<String, String> getPathParameters(boolean decode) {
             RuntimeResource target = currentRequest.getTarget();
             if (target != null) { // a target can be null if this happens in a filter that runs before the target is set
                 for (Entry<String, Integer> pathParam : target.getPathParameterIndexes().entrySet()) {
-                    pathParams.add(pathParam.getKey(), currentRequest.getPathParam(pathParam.getValue()));
+                    pathParams.add(pathParam.getKey(), currentRequest.getPathParam(pathParam.getValue(), false));
                 }
             }
         }
diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/mapping/RequestMapper.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/mapping/RequestMapper.java
@@ -8,8 +8,6 @@
 import java.util.Map;
 import java.util.regex.Matcher;
 
-import org.jboss.resteasy.reactive.common.util.URIDecoder;
-
 public class RequestMapper<T> {
 
     private static final String[] EMPTY_STRING_ARRAY = new String[0];
@@ -111,7 +109,7 @@ private RequestMatch<T> mapFromPathMatcher(String path, PathMatcher.PathMatch<Ar
                     while (matchPos < pathLength && path.charAt(matchPos) != '/') {
                         matchPos++;
                     }
-                    params[paramCount++] = URIDecoder.decodeURIComponent(path.substring(start, matchPos), false);
+                    params[paramCount++] = path.substring(start, matchPos);
                 }
             }
             if (!matched) {
diff --git a/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/matching/RegexMatchTest.java b/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/matching/RegexMatchTest.java
@@ -41,6 +41,10 @@ public void testLiteralInRegex() {
                 .then()
                 .statusCode(200)
                 .body(equalTo("plain:abb/foo/alongpathtotriggerbug"));
+        RestAssured.get("/regex/first space/foo/second space")
+                .then()
+                .statusCode(200)
+                .body(equalTo("plain:first space/foo/second space"));
         RestAssured.get("/regex/abb/literal/ddc")
                 .then()
                 .statusCode(200)

Original file line number	Diff line number	Diff line change
`@@ -226,18 +226,20 @@ public void setMaxPathParams(int maxPathParams) {`
`226`	`226`	`}`
`227`	`227`	`}`
`228`	`228`
`229`		`- public String getPathParam(int index) {`
`230`		`- return doGetPathParam(index, pathParamValues);`
	`229`	`+ public String getPathParam(int index, boolean encoded) {`
	`230`	`+ return doGetPathParam(index, pathParamValues, encoded);`
`231`	`231`	`}`
`232`	`232`
`233`		`- private String doGetPathParam(int index, Object pathParamValues) {`
	`233`	`+ private String doGetPathParam(int index, Object pathParamValues, boolean encoded) {`
`234`	`234`	`if (pathParamValues instanceof String[]) {`
`235`		`- return ((String[]) pathParamValues)[index];`
	`235`	`+ String pathParam = ((String[]) pathParamValues)[index];`
	`236`	`+ return encoded ? pathParam : Encode.decodePath(pathParam);`
`236`	`237`	`}`
`237`	`238`	`if (index > 1) {`
`238`	`239`	`throw new IndexOutOfBoundsException();`
`239`	`240`	`}`
`240`		`- return (String) pathParamValues;`
	`241`	`+ String pathParam = (String) pathParamValues;`
	`242`	`+ return encoded ? pathParam : Encode.decodePath(pathParam);`
`241`	`243`	`}`
`242`	`244`
`243`	`245`	`public ResteasyReactiveRequestContext setPathParamValue(int index, String value) {`
`@@ -926,18 +928,11 @@ public String getPathParameter(String name, boolean encoded) {`
`926`	`928`	`Integer index = target.getPathParameterIndexes().get(name);`
`927`	`929`	`String value;`
`928`	`930`	`if (index != null) {`
`929`		`- value = getPathParam(index);`
`930`		`- } else {`
`931`		`- // Check previous resources if the path is not defined in the current target`
`932`		`- value = getResourceLocatorPathParam(name);`
`933`		`- }`
`934`		`-`
`935`		`- // It's possible to inject a path param that's not defined, return null in this case`
`936`		`- if (encoded && value != null) {`
`937`		`- return Encode.encodeQueryParam(value);`
	`931`	`+ return getPathParam(index, encoded);`
`938`	`932`	`}`
`939`	`933`
`940`		`- return value;`
	`934`	`+ // Check previous resources if the path is not defined in the current target`
	`935`	`+ return getResourceLocatorPathParam(name, encoded);`
`941`	`936`	`}`
`942`	`937`
`943`	`938`	`@Override`
`@@ -996,8 +991,8 @@ public ResteasyReactiveResourceInfo getResteasyReactiveResourceInfo() {`
`996`	`991`
`997`	`992`	`public abstract Runnable registerTimer(long millis, Runnable task);`
`998`	`993`
`999`		`- public String getResourceLocatorPathParam(String name) {`
`1000`		`- return getResourceLocatorPathParam(name, (PreviousResource) getProperty(PreviousResource.PROPERTY_KEY));`
	`994`	`+ public String getResourceLocatorPathParam(String name, boolean encoded) {`
	`995`	`+ return getResourceLocatorPathParam(name, (PreviousResource) getProperty(PreviousResource.PROPERTY_KEY), encoded);`
`1001`	`996`	`}`
`1002`	`997`
`1003`	`998`	`public FormData getFormData() {`
`@@ -1009,7 +1004,7 @@ public ResteasyReactiveRequestContext setFormData(FormData formData) {`
`1009`	`1004`	`return this;`
`1010`	`1005`	`}`
`1011`	`1006`
`1012`		`- private String getResourceLocatorPathParam(String name, PreviousResource previousResource) {`
	`1007`	`+ private String getResourceLocatorPathParam(String name, PreviousResource previousResource, boolean encoded) {`
`1013`	`1008`	`if (previousResource == null) {`
`1014`	`1009`	`return null;`
`1015`	`1010`	`}`
`@@ -1020,13 +1015,13 @@ private String getResourceLocatorPathParam(String name, PreviousResource previou`
`1020`	`1015`	`for (URITemplate.TemplateComponent component : classPath.components) {`
`1021`	`1016`	`if (component.name != null) {`
`1022`	`1017`	`if (component.name.equals(name)) {`
`1023`		`- return doGetPathParam(index, previousResource.locatorPathParamValues);`
	`1018`	`+ return doGetPathParam(index, previousResource.locatorPathParamValues, encoded);`
`1024`	`1019`	`}`
`1025`	`1020`	`index++;`
`1026`	`1021`	`} else if (component.names != null) {`
`1027`	`1022`	`for (String nm : component.names) {`
`1028`	`1023`	`if (nm.equals(name)) {`
`1029`		`- return doGetPathParam(index, previousResource.locatorPathParamValues);`
	`1024`	`+ return doGetPathParam(index, previousResource.locatorPathParamValues, encoded);`
`1030`	`1025`	`}`
`1031`	`1026`	`}`
`1032`	`1027`	`index++;`
`@@ -1036,19 +1031,19 @@ private String getResourceLocatorPathParam(String name, PreviousResource previou`
`1036`	`1031`	`for (URITemplate.TemplateComponent component : previousResource.locatorTarget.getPath().components) {`
`1037`	`1032`	`if (component.name != null) {`
`1038`	`1033`	`if (component.name.equals(name)) {`
`1039`		`- return doGetPathParam(index, previousResource.locatorPathParamValues);`
	`1034`	`+ return doGetPathParam(index, previousResource.locatorPathParamValues, encoded);`
`1040`	`1035`	`}`
`1041`	`1036`	`index++;`
`1042`	`1037`	`} else if (component.names != null) {`
`1043`	`1038`	`for (String nm : component.names) {`
`1044`	`1039`	`if (nm.equals(name)) {`
`1045`		`- return doGetPathParam(index, previousResource.locatorPathParamValues);`
	`1040`	`+ return doGetPathParam(index, previousResource.locatorPathParamValues, encoded);`
`1046`	`1041`	`}`
`1047`	`1042`	`}`
`1048`	`1043`	`index++;`
`1049`	`1044`	`}`
`1050`	`1045`	`}`
`1051`		`- return getResourceLocatorPathParam(name, previousResource.prev);`
	`1046`	`+ return getResourceLocatorPathParam(name, previousResource.prev, encoded);`
`1052`	`1047`	`}`
`1053`	`1048`
`1054`	`1049`	`public abstract boolean resumeExternalProcessing();`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ public LocatableResourcePathParamExtractor(String name) {`
`12`	`12`
`13`	`13`	`@Override`
`14`	`14`	`public Object extractParameter(ResteasyReactiveRequestContext context) {`
`15`		`- return context.getResourceLocatorPathParam(name);`
	`15`	`+ return context.getResourceLocatorPathParam(name, false);`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ public MultivaluedMap<String, String> getPathParameters(boolean decode) {`
`152`	`152`	`RuntimeResource target = currentRequest.getTarget();`
`153`	`153`	`if (target != null) { // a target can be null if this happens in a filter that runs before the target is set`
`154`	`154`	`for (Entry<String, Integer> pathParam : target.getPathParameterIndexes().entrySet()) {`
`155`		`- pathParams.add(pathParam.getKey(), currentRequest.getPathParam(pathParam.getValue()));`
	`155`	`+ pathParams.add(pathParam.getKey(), currentRequest.getPathParam(pathParam.getValue(), false));`
`156`	`156`	`}`
`157`	`157`	`}`
`158`	`158`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,8 +8,6 @@`
`8`	`8`	`import java.util.Map;`
`9`	`9`	`import java.util.regex.Matcher;`
`10`	`10`
`11`		`-import org.jboss.resteasy.reactive.common.util.URIDecoder;`
`12`		`-`
`13`	`11`	`public class RequestMapper<T> {`
`14`	`12`
`15`	`13`	`private static final String[] EMPTY_STRING_ARRAY = new String[0];`
`@@ -111,7 +109,7 @@ private RequestMatch<T> mapFromPathMatcher(String path, PathMatcher.PathMatch<Ar`
`111`	`109`	`while (matchPos < pathLength && path.charAt(matchPos) != '/') {`
`112`	`110`	`matchPos++;`
`113`	`111`	`}`
`114`		`- params[paramCount++] = URIDecoder.decodeURIComponent(path.substring(start, matchPos), false);`
	`112`	`+ params[paramCount++] = path.substring(start, matchPos);`
`115`	`113`	`}`
`116`	`114`	`}`
`117`	`115`	`if (!matched) {`