rombert commented on a change in pull request #11:
URL: 
https://github.com/apache/sling-org-apache-sling-engine/pull/11#discussion_r577481888



##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -520,6 +522,11 @@ public static void service(SlingHttpServletRequest request,
             SlingHttpServletResponse response) throws IOException,
             ServletException {
 
+        if(!isValidRequest(request.getPathInfo())){
+            response.sendError(HttpServletResponse.SC_BAD_REQUEST,
+                    "Malformed request syntax");

Review comment:
       Don't you need a return here as well?

##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -563,6 +570,24 @@ public static void service(SlingHttpServletRequest request,
         }
     }
 
+    protected static boolean isValidRequest(String path){
+        boolean isValidRequest = true;
+        if(path.contains("...")){ //any occurrence "..." will mark request 
invalid

Review comment:
       nit: formatting is off compared to the rest of the file, e.g. 
   
   - single white space after `if`
   - whitespace before opening brace - `{`

##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -563,6 +570,24 @@ public static void service(SlingHttpServletRequest request,
         }
     }
 
+    protected static boolean isValidRequest(String path){
+        boolean isValidRequest = true;
+        if(path.contains("...")){ //any occurrence "..." will mark request 
invalid
+            isValidRequest = false;
+        }else {
+            //consecutive dots will be treated as Invalid request except "/.."

Review comment:
       This would be better as a method javadoc

##########
File path: src/main/java/org/apache/sling/engine/impl/request/RequestData.java
##########
@@ -563,6 +570,24 @@ public static void service(SlingHttpServletRequest request,
         }
     }
 
+    protected static boolean isValidRequest(String path){

Review comment:
       This is only protected for testing purposes, right? Then I suggest we 
make this package-private since it's narrower in scope.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to