Merge pull request #13514 from gytis/sort-query-validator

Validate REST Data Panache sort query parameter
quarkusio · Nov 30, 2020 · cd844aa · cd844aa
2 parents b8f59e9 + 9d0a176
commit cd844aa
Show file tree

Hide file tree

Showing 8 changed files with 182 additions and 1 deletion.
diff --git a/...est/java/io/quarkus/hibernate/orm/rest/data/panache/deployment/AbstractGetMethodTest.java b/...est/java/io/quarkus/hibernate/orm/rest/data/panache/deployment/AbstractGetMethodTest.java
@@ -120,6 +120,22 @@ void shouldListSimpleDescendingObjects() {
                 .and().body("name", contains("second", "first"));
     }
 
+    @Test
+    void shouldNotListWithInvalidSortParam() {
+        given().accept("application/json")
+                .when().get("/items?sort=1name")
+                .then().statusCode(400)
+                .and().body(is(equalTo("Invalid sort parameter '1name'")));
+    }
+
+    @Test
+    void shouldNotListHalWithInvalidSortParam() {
+        given().accept("application/hal+json")
+                .when().get("/items?sort=1name")
+                .then().statusCode(400)
+                .and().body(is(equalTo("Invalid sort parameter '1name'")));
+    }
+
     @Test
     void shouldListComplexObjects() {
         given().accept("application/json")

diff --git a/.../src/main/java/io/quarkus/rest/data/panache/deployment/methods/ListMethodImplementor.java b/.../src/main/java/io/quarkus/rest/data/panache/deployment/methods/ListMethodImplementor.java
@@ -39,7 +39,7 @@ public final class ListMethodImplementor extends StandardMethodImplementor {
      * Generate JAX-RS GET method that exposes {@link RestDataResource#list(Page, Sort)}.
      * Generated pseudo-code with enabled pagination is shown below. If pagination is disabled pageIndex and pageSize
      * query parameters are skipped and null {@link Page} instance is used.
-     * 
+     *
      * <pre>
      * {@code
      *     &#64;GET
@@ -90,6 +90,7 @@ private void implementPaged(ClassCreator classCreator, ResourceMetadata resource
         addPathAnnotation(methodCreator, resourceProperties.getMethodPath(RESOURCE_METHOD_NAME));
         addProducesAnnotation(methodCreator, APPLICATION_JSON);
         addLinksAnnotation(methodCreator, resourceMetadata.getEntityType(), REL);
+        addSortQueryParamValidatorAnnotation(methodCreator);
         addQueryParamAnnotation(methodCreator.getParameterAnnotations(0), "sort");
         addQueryParamAnnotation(methodCreator.getParameterAnnotations(1), "page");
         addDefaultValueAnnotation(methodCreator.getParameterAnnotations(1), Integer.toString(DEFAULT_PAGE_INDEX));

diff --git a/.../main/java/io/quarkus/rest/data/panache/deployment/methods/StandardMethodImplementor.java b/.../main/java/io/quarkus/rest/data/panache/deployment/methods/StandardMethodImplementor.java
@@ -21,6 +21,7 @@
 import io.quarkus.gizmo.FieldDescriptor;
 import io.quarkus.rest.data.panache.deployment.ResourceMetadata;
 import io.quarkus.rest.data.panache.deployment.properties.ResourceProperties;
+import io.quarkus.rest.data.panache.runtime.sort.SortQueryParamValidator;
 
 /**
  * A standard JAX-RS method implementor.
@@ -103,6 +104,10 @@ protected void addContextAnnotation(AnnotatedElement element) {
         element.addAnnotation(Context.class);
     }
 
+    protected void addSortQueryParamValidatorAnnotation(AnnotatedElement element) {
+        element.addAnnotation(SortQueryParamValidator.class);
+    }
+
     protected String appendToPath(String path, String suffix) {
         if (path.endsWith("/")) {
             path = path.substring(0, path.lastIndexOf("/"));

diff --git a/...in/java/io/quarkus/rest/data/panache/deployment/methods/hal/ListHalMethodImplementor.java b/...in/java/io/quarkus/rest/data/panache/deployment/methods/hal/ListHalMethodImplementor.java
@@ -88,6 +88,7 @@ private void implementPaged(ClassCreator classCreator, ResourceMetadata resource
         addPathAnnotation(methodCreator, resourceProperties.getMethodPath(RESOURCE_METHOD_NAME));
         addGetAnnotation(methodCreator);
         addProducesAnnotation(methodCreator, APPLICATION_HAL_JSON);
+        addSortQueryParamValidatorAnnotation(methodCreator);
         addQueryParamAnnotation(methodCreator.getParameterAnnotations(0), "sort");
         addQueryParamAnnotation(methodCreator.getParameterAnnotations(1), "page");
         addDefaultValueAnnotation(methodCreator.getParameterAnnotations(1), Integer.toString(DEFAULT_PAGE_INDEX));

diff --git a/extensions/panache/rest-data-panache/runtime/pom.xml b/extensions/panache/rest-data-panache/runtime/pom.xml
@@ -45,5 +45,10 @@
             <artifactId>assertj-core</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-junit-jupiter</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
diff --git a/...runtime/src/main/java/io/quarkus/rest/data/panache/runtime/sort/SortQueryParamFilter.java b/...runtime/src/main/java/io/quarkus/rest/data/panache/runtime/sort/SortQueryParamFilter.java
@@ -0,0 +1,41 @@
+package io.quarkus.rest.data.panache.runtime.sort;
+
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+
+import java.util.Collections;
+
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.container.ContainerRequestFilter;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.Provider;
+
+@Provider
+@SortQueryParamValidator
+public class SortQueryParamFilter implements ContainerRequestFilter {
+
+    private static final String SORT_REGEX = "-?([a-z]|[A-Z]|_|\\$|[\u0080-\ufffe])([a-z]|[A-Z]|_|\\$|[0-9]|[\u0080-\ufffe])*";
+
+    /**
+     * Verifies that sort query parameters are valid.
+     * Valid examples:
+     * * ?sort=name,surname
+     * * ?sort=$surname&sort=-age
+     * * ?sort=_id
+     */
+    @Override
+    public void filter(ContainerRequestContext requestContext) {
+        MultivaluedMap<String, String> queryParams = requestContext.getUriInfo().getQueryParameters();
+        for (String sort : queryParams.getOrDefault("sort", Collections.emptyList())) {
+            for (String sortPart : sort.split(",")) {
+                String trimmed = sortPart.trim();
+                if (trimmed.length() > 0 && !trimmed.matches(SORT_REGEX)) {
+                    requestContext.abortWith(
+                            Response.status(BAD_REQUEST)
+                                    .entity(String.format("Invalid sort parameter '%s'", sort))
+                                    .build());
+                }
+            }
+        }
+    }
+}
diff --git a/...time/src/main/java/io/quarkus/rest/data/panache/runtime/sort/SortQueryParamValidator.java b/...time/src/main/java/io/quarkus/rest/data/panache/runtime/sort/SortQueryParamValidator.java
@@ -0,0 +1,14 @@
+package io.quarkus.rest.data.panache.runtime.sort;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+import javax.ws.rs.NameBinding;
+
+@NameBinding
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ ElementType.TYPE, ElementType.METHOD })
+public @interface SortQueryParamValidator {
+}
diff --git a/...ime/src/test/java/io/quarkus/rest/data/panache/runtime/sort/SortQueryParamFilterTest.java b/...ime/src/test/java/io/quarkus/rest/data/panache/runtime/sort/SortQueryParamFilterTest.java
@@ -0,0 +1,98 @@
+package io.quarkus.rest.data.panache.runtime.sort;
+
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.core.MultivaluedHashMap;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.UriInfo;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentMatcher;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class SortQueryParamFilterTest {
+
+    @Mock
+    private ContainerRequestContext requestContext;
+
+    @Mock
+    private UriInfo uriInfo;
+
+    private final SortQueryParamFilter filter = new SortQueryParamFilter();
+
+    @BeforeEach
+    void setUp() {
+        given(requestContext.getUriInfo()).willReturn(uriInfo);
+    }
+
+    @Test
+    void shouldAllowValidParameters() {
+        MultivaluedMap<String, String> map = new MultivaluedHashMap<>();
+        map.putSingle("sort", "$name");
+        map.putSingle("sort", "-surname_1");
+        given(uriInfo.getQueryParameters()).willReturn(map);
+
+        filter.filter(requestContext);
+
+        verify(requestContext, times(0)).abortWith(any());
+    }
+
+    @Test
+    void shouldAllowValidParametersWithMultipleValues() {
+        MultivaluedMap<String, String> map = new MultivaluedHashMap<>();
+        map.putSingle("sort", "$name,-surname_1");
+        given(uriInfo.getQueryParameters()).willReturn(map);
+
+        filter.filter(requestContext);
+
+        verify(requestContext, times(0)).abortWith(any());
+    }
+
+    @Test
+    void shouldAllowEmptyParameters() {
+        given(uriInfo.getQueryParameters()).willReturn(new MultivaluedHashMap<>());
+
+        filter.filter(requestContext);
+
+        verify(requestContext, times(0)).abortWith(any());
+    }
+
+    @Test
+    void shouldCatchInvalidParameter() {
+        MultivaluedMap<String, String> map = new MultivaluedHashMap<>();
+        map.putSingle("sort", "$name");
+        map.putSingle("sort", "(surname_1");
+        given(uriInfo.getQueryParameters()).willReturn(map);
+
+        filter.filter(requestContext);
+
+        verify(requestContext).abortWith(argThat(abortResponseMatcher("(surname_1")));
+    }
+
+    @Test
+    void shouldCatchInvalidParameterValue() {
+        MultivaluedMap<String, String> map = new MultivaluedHashMap<>();
+        map.putSingle("sort", "$name,1_surname");
+        given(uriInfo.getQueryParameters()).willReturn(map);
+
+        filter.filter(requestContext);
+
+        verify(requestContext).abortWith(argThat(abortResponseMatcher("$name,1_surname")));
+    }
+
+    private ArgumentMatcher<Response> abortResponseMatcher(String sortValue) {
+        return response -> response.getStatus() == BAD_REQUEST.getStatusCode()
+                && response.getEntity().equals(String.format("Invalid sort parameter '%s'", sortValue));
+    }
+}