fix: handle path parameters containing special character

https://gravitee.atlassian.net/browse/APIM-1879 gravitee-io/issues#9081
gravitee-io · Jun 8, 2023 · a853d70 · a853d70
1 parent 6a81246
commit a853d70
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 11 deletions.
diff --git a/...rc/main/java/io/gravitee/gateway/handlers/api/processor/pathparameters/PathParameter.java b/...rc/main/java/io/gravitee/gateway/handlers/api/processor/pathparameters/PathParameter.java
@@ -0,0 +1,36 @@
+/**
+ * Copyright (C) 2015 The Gravitee team (http://gravitee.io)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.gravitee.gateway.handlers.api.processor.pathparameters;
+
+public class PathParameter {
+
+    private final String id;
+    private final String name;
+
+    public PathParameter(String name, Integer position) {
+        this.name = name;
+        // We use a prefix to ensure id is a valid group name based on the Java Pattern documentation
+        this.id = "group" + position;
+    }
+
+    public String getId() {
+        return id;
+    }
+
+    public String getName() {
+        return name;
+    }
+}
diff --git a/...c/main/java/io/gravitee/gateway/handlers/api/processor/pathparameters/PathParameters.java b/...c/main/java/io/gravitee/gateway/handlers/api/processor/pathparameters/PathParameters.java
@@ -35,7 +35,7 @@ public class PathParameters {
     private final String originalPath;
     private final Operator operator;
     private Pattern pathPattern;
-    private final List<String> parameters = new ArrayList<>();
+    private final List<PathParameter> parameters = new ArrayList<PathParameter>();
 
     public PathParameters(String originalPath, Operator operator) {
         this.originalPath = originalPath;
@@ -51,8 +51,9 @@ private void extractPathParamsAndPattern() {
             if (!branches[i].isEmpty()) {
                 if (branches[i].startsWith(PATH_PARAM_PREFIX)) {
                     String paramWithoutColon = branches[i].substring(1);
-                    parameters.add(paramWithoutColon);
-                    patternizedPath.append("(?<" + paramWithoutColon + ">" + PATH_PARAM_REGEX + ")");
+                    PathParameter pathParameter = new PathParameter(paramWithoutColon, parameters.size());
+                    parameters.add(pathParameter);
+                    patternizedPath.append("(?<" + pathParameter.getId() + ">" + PATH_PARAM_REGEX + ")");
                 } else {
                     patternizedPath.append(branches[i]);
                 }
@@ -74,7 +75,7 @@ public Pattern getPathPattern() {
         return pathPattern;
     }
 
-    public List<String> getParameters() {
+    public List<PathParameter> getParameters() {
         return parameters;
     }
 

diff --git a/...va/io/gravitee/gateway/handlers/api/processor/pathparameters/PathParametersExtractor.java b/...va/io/gravitee/gateway/handlers/api/processor/pathparameters/PathParametersExtractor.java
@@ -54,7 +54,7 @@ public boolean canExtractPathParams() {
             .stream()
             .flatMap(Collection::stream)
             .flatMap(p -> p.getParameters().stream())
-            .anyMatch(parameters -> parameters.length() > 0);
+            .anyMatch(parameters -> !parameters.getName().isEmpty());
     }
 
     /**
@@ -113,7 +113,8 @@ private static Map<PathParameterHttpMethod, Set<PathParameters>> groupPatternsBy
                         f
                             .getMethods()
                             .stream()
-                            .map(m -> Map.entry(PathParameterHttpMethod.valueOf(m.name()), new PathParameters(f.getPath(), f.getOperator())))
+                            .map(m -> Map.entry(PathParameterHttpMethod.valueOf(m.name()), new PathParameters(f.getPath(), f.getOperator()))
+                            )
                             .collect(Collectors.toList());
                 }
                 return flowByMethod.stream();
@@ -157,7 +158,7 @@ private void computePathParam(final PathParameterHttpMethod method, final String
 
                 final Matcher matcher = pattern.getPathPattern().matcher(path);
                 if (matcher.find()) {
-                    pattern.getParameters().forEach(p -> pathParameters.put(p, matcher.group(p)));
+                    pattern.getParameters().forEach(p -> pathParameters.put(p.getName(), matcher.group(p.getId())));
                 }
             });
     }

diff --git a/...o/gravitee/gateway/handlers/api/processor/pathparameters/PathParametersExtractorTest.java b/...o/gravitee/gateway/handlers/api/processor/pathparameters/PathParametersExtractorTest.java
@@ -97,6 +97,7 @@ public static Stream<Arguments> provideParameters() throws IOException {
             Arguments.of(readApi("simple-api"), "GET", "/products", Map.of(), Set.of()),
             Arguments.of(readApi("simple-api"), "TRACE", "/products", Map.of(), Set.of()),
             Arguments.of(readApi("simple-api"), "GET", "/products/my-product", Map.of("productId", "my-product"), Set.of()),
+            Arguments.of(readApi("simple-api"), "GET", "/products-special-char/my-product", Map.of("product-id", "my-product"), Set.of()),
             Arguments.of(
                 readApi("simple-api"),
                 "GET",
@@ -120,6 +121,13 @@ public static Stream<Arguments> provideParameters() throws IOException {
                 Map.of("productId", "my-product", "itemId", "my-item"),
                 Set.of()
             ),
+            Arguments.of(
+                readApi("simple-api"),
+                "GET",
+                "/products-special-char/my-product/items/my-item",
+                Map.of("product-id", "my-product", "It€m_Id", "my-item"),
+                Set.of()
+            ),
             Arguments.of(readApi("api-flows-equals-operator"), "GET", "/products", Map.of(), Set.of()),
             Arguments.of(readApi("api-flows-equals-operator"), "TRACE", "/products", Map.of(), Set.of()),
             Arguments.of(readApi("api-flows-equals-operator"), "GET", "/products/my-product", Map.of("productId", "my-product"), Set.of()),

diff --git a/...st/java/io/gravitee/gateway/handlers/api/processor/pathparameters/PathParametersTest.java b/...st/java/io/gravitee/gateway/handlers/api/processor/pathparameters/PathParametersTest.java
@@ -51,12 +51,12 @@ void should_not_have_parameters_equals_operator() {
     @Test
     void should_have_parameters_starts_with_operator() {
         final PathParameters cut = new PathParameters("/product/:productId/item/:itemId", Operator.STARTS_WITH);
-        assertThat(cut.getParameters()).hasSize(2).contains("productId", "itemId");
+        assertThat(cut.getParameters()).hasSize(2).extracting(PathParameter::getName).contains("productId", "itemId");
         assertThat(cut.getPathPattern())
             .hasToString(
                 Pattern
                     .compile(
-                        "^/product/(?<productId>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)/item/(?<itemId>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)(?:/.*)?$"
+                        "^/product/(?<group0>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)/item/(?<group1>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)(?:/.*)?$"
                     )
                     .toString()
             );
@@ -65,12 +65,24 @@ void should_have_parameters_starts_with_operator() {
     @Test
     void should_have_parameters_equals_operator() {
         final PathParameters cut = new PathParameters("/product/:productId/item/:itemId", Operator.EQUALS);
-        assertThat(cut.getParameters()).hasSize(2).contains("productId", "itemId");
+        assertThat(cut.getParameters()).hasSize(2).extracting(PathParameter::getName).contains("productId", "itemId");
+        assertThat(cut.getPathPattern())
+            .hasToString(
+                Pattern
+                    .compile("^/product/(?<group0>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)/item/(?<group1>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)/?$")
+                    .toString()
+            );
+    }
+
+    @Test
+    void should_have_parameters_with_special_character() {
+        final PathParameters cut = new PathParameters("/product/:product-id/item/:It€m_Id", Operator.STARTS_WITH);
+        assertThat(cut.getParameters()).hasSize(2).extracting(PathParameter::getName).contains("product-id", "It€m_Id");
         assertThat(cut.getPathPattern())
             .hasToString(
                 Pattern
                     .compile(
-                        "^/product/(?<productId>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)/item/(?<itemId>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)/?$"
+                        "^/product/(?<group0>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)/item/(?<group1>[a-zA-Z0-9\\-._~%!$&'()* +,;=:@]+)(?:/.*)?$"
                     )
                     .toString()
             );

diff --git a/...ers/gravitee-apim-gateway-handlers-api/src/test/resources/apis/pathparams/simple-api.json b/...ers/gravitee-apim-gateway-handlers-api/src/test/resources/apis/pathparams/simple-api.json
@@ -143,6 +143,36 @@
       "pre": [],
       "post": [],
       "enabled": true
+    },
+    {
+      "name": "Product",
+      "path-operator": {
+        "path": "/products-special-char/:product-id",
+        "operator": "STARTS_WITH"
+      },
+      "condition": "",
+      "methods": [
+        "DELETE",
+        "GET"
+      ],
+      "pre": [],
+      "post": [],
+      "enabled": true
+    },
+    {
+      "name": "Product",
+      "path-operator": {
+        "path": "/products-special-char/:product-id/items/:It€m_Id",
+        "operator": "STARTS_WITH"
+      },
+      "condition": "",
+      "methods": [
+        "DELETE",
+        "GET"
+      ],
+      "pre": [],
+      "post": [],
+      "enabled": true
     }
   ],
   "resources": []