You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2021/05/28 14:30:14 UTC

[sling-org-apache-sling-engine] branch issues/SLING-10225 created (now fe857ef)

This is an automated email from the ASF dual-hosted git repository.

pauls pushed a change to branch issues/SLING-10225
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git.


      at fe857ef  SLING-10225: don't allow path segements with only dots

This branch includes the following new commits:

     new fe857ef  SLING-10225: don't allow path segements with only dots

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[sling-org-apache-sling-engine] 01/01: SLING-10225: don't allow path segements with only dots

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

pauls pushed a commit to branch issues/SLING-10225
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git

commit fe857ef95046d44465ceb5bebdd1890f42e952f8
Author: Karl Pauls <ka...@gmail.com>
AuthorDate: Fri May 28 16:29:56 2021 +0200

    SLING-10225: don't allow path segements with only dots
---
 .../sling/engine/impl/request/RequestData.java     |  22 +-
 .../sling/engine/impl/request/RequestDataTest.java | 563 ++++++++++++++++++++-
 2 files changed, 566 insertions(+), 19 deletions(-)

diff --git a/src/main/java/org/apache/sling/engine/impl/request/RequestData.java b/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
index e11c15c..c994a02 100644
--- a/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
+++ b/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
@@ -532,7 +532,7 @@ public class RequestData {
             SlingHttpServletResponse response) throws IOException,
             ServletException {
 
-        if (!isValidRequest(request.getPathInfo())) {
+        if (!isValidRequest(request)) {
             response.sendError(HttpServletResponse.SC_BAD_REQUEST,
                     "Malformed request syntax");
             return;
@@ -588,22 +588,14 @@ public class RequestData {
      * @param path The path of request object
      * @return true if path contains no consecutive dots except "/..", false otherwise
      */
-    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 "/.."
-            int doubleDotIndex = path.indexOf(CONSECUTIVE_DOTS);
-            while (doubleDotIndex >= 0) {
-                if (doubleDotIndex == 0 || path.charAt(doubleDotIndex - 1) != '/') {//doubleDotIndex == 0 When path start with ..
-                    isValidRequest = false;
-                    break;
-                }
-                doubleDotIndex = path.indexOf(CONSECUTIVE_DOTS, doubleDotIndex + 2);
+    static boolean isValidRequest(SlingHttpServletRequest request) {
+        RequestPathInfo info = request.getRequestPathInfo();
+        for (String selector : info.getSelectors()) {
+            if (selector.trim().isEmpty()) {
+                return false;
             }
         }
-        return isValidRequest;
+        return !info.getResourcePath().matches(".*/\\[*\\.\\[*\\.[\\[,\\.]*/.*|.*/\\[*\\.\\[*\\.[\\[,\\.]*$");
     }
 
     // ---------- Content inclusion stacking -----------------------------------
diff --git a/src/test/java/org/apache/sling/engine/impl/request/RequestDataTest.java b/src/test/java/org/apache/sling/engine/impl/request/RequestDataTest.java
index 51880a6..5025fe7 100644
--- a/src/test/java/org/apache/sling/engine/impl/request/RequestDataTest.java
+++ b/src/test/java/org/apache/sling/engine/impl/request/RequestDataTest.java
@@ -18,13 +18,24 @@ package org.apache.sling.engine.impl.request;
 
 
 import javax.servlet.*;
+import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import javax.servlet.http.HttpUpgradeHandler;
+import javax.servlet.http.Part;
 
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.SlingHttpServletResponse;
+import org.apache.sling.api.request.RequestDispatcherOptions;
+import org.apache.sling.api.request.RequestParameter;
+import org.apache.sling.api.request.RequestParameterMap;
+import org.apache.sling.api.request.RequestPathInfo;
 import org.apache.sling.api.request.RequestProgressTracker;
 import org.apache.sling.api.request.TooManyCallsException;
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceMetadata;
+import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.engine.impl.SlingHttpServletRequestImpl;
 import org.apache.sling.engine.impl.SlingHttpServletResponseImpl;
 import org.jmock.Expectations;
@@ -33,7 +44,17 @@ import org.jmock.lib.legacy.ClassImposteriser;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.security.Principal;
+import java.util.Collection;
+import java.util.Enumeration;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.ResourceBundle;
 
 import static org.junit.Assert.*;
 
@@ -78,6 +99,8 @@ public class RequestDataTest {
             allowing(servlet).getServletConfig();
             will(returnValue(servletConfig));
 
+            allowing(contentData).getRequestPathInfo();
+
             allowing(servlet).service(with(any(ServletRequest.class)), with(any(ServletResponse.class)));
 
             allowing(servletConfig).getServletName();
@@ -132,16 +155,39 @@ public class RequestDataTest {
 
     @Test
     public void testConsecutiveDots() {
-        assertValidRequest(false, "/path/content../test");
+        assertValidRequest(true, "/path/content../test");
         assertValidRequest(false, "/../path/content../test");
         assertValidRequest(false, "/../path/content/.../test");
         assertValidRequest(false, "../path/content/.../test");
+        assertValidRequest(false, "/content/.../test");
     }
 
     @Test
     public void testConsecutiveDotsAfterPathSeparator() {
         assertValidRequest(false, "/path/....");
-        assertValidRequest(true, "/path/..");
+        assertValidRequest(false, "/path/..");
+        assertValidRequest(true, "/path/foo..");
+        assertValidRequest(true, "/path/..foo..");
+    }
+
+    @Test
+    public void testDots() {
+        assertValidRequest(false, "/a/.../b");
+        assertValidRequest(false, "/a/............../b");
+        assertValidRequest(true, "/a/..........helloo......./b");
+        assertValidRequest(true, "/path/content./test");
+        assertValidRequest(true, "/./path/content./test");
+        assertValidRequest(true, "/./path/content/./test");
+        assertValidRequest(true, "./path/content/./test");
+        assertValidRequest(true, "/content/./test");
+    }
+
+    @Test
+    public void testDotsAnd5B() {
+        assertValidRequest(false, "/a/..[[./b");
+        assertValidRequest(false, "/a/[............../b");
+        assertValidRequest(true, "/a/..........helloo......./b");
+        assertValidRequest(false, "/a/[..");
     }
 
     @Test
@@ -150,11 +196,520 @@ public class RequestDataTest {
         assertValidRequest(true, "/path");
     }
 
-    private static void assertValidRequest(boolean expected, String path) {
+    private static void assertValidRequest(boolean expected, final String path) {
         assertEquals(
                 "Expected " + expected + " for " + path,
                 expected,
-                RequestData.isValidRequest(path)
+                RequestData.isValidRequest(new SlingHttpServletRequest(){
+
+                    @Override
+                    public <AdapterType> AdapterType adaptTo(Class<AdapterType> aClass) {
+                        return null;
+                    }
+
+                    @Override
+                    public Object getAttribute(String s) {
+                        return null;
+                    }
+
+                    @Override
+                    public Enumeration<String> getAttributeNames() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getCharacterEncoding() {
+                        return null;
+                    }
+
+                    @Override
+                    public void setCharacterEncoding(String s) throws UnsupportedEncodingException {
+
+                    }
+
+                    @Override
+                    public int getContentLength() {
+                        return 0;
+                    }
+
+                    @Override
+                    public long getContentLengthLong() {
+                        return 0;
+                    }
+
+                    @Override
+                    public String getContentType() {
+                        return null;
+                    }
+
+                    @Override
+                    public ServletInputStream getInputStream() throws IOException {
+                        return null;
+                    }
+
+                    @Override
+                    public String getParameter(String s) {
+                        return null;
+                    }
+
+                    @Override
+                    public Enumeration<String> getParameterNames() {
+                        return null;
+                    }
+
+                    @Override
+                    public String[] getParameterValues(String s) {
+                        return new String[0];
+                    }
+
+                    @Override
+                    public Map<String, String[]> getParameterMap() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getProtocol() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getScheme() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getServerName() {
+                        return null;
+                    }
+
+                    @Override
+                    public int getServerPort() {
+                        return 0;
+                    }
+
+                    @Override
+                    public BufferedReader getReader() throws IOException {
+                        return null;
+                    }
+
+                    @Override
+                    public String getRemoteAddr() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getRemoteHost() {
+                        return null;
+                    }
+
+                    @Override
+                    public void setAttribute(String s, Object o) {
+
+                    }
+
+                    @Override
+                    public void removeAttribute(String s) {
+
+                    }
+
+                    @Override
+                    public Locale getLocale() {
+                        return null;
+                    }
+
+                    @Override
+                    public Enumeration<Locale> getLocales() {
+                        return null;
+                    }
+
+                    @Override
+                    public boolean isSecure() {
+                        return false;
+                    }
+
+                    @Override
+                    public RequestDispatcher getRequestDispatcher(String s) {
+                        return null;
+                    }
+
+                    @Override
+                    public String getRealPath(String s) {
+                        return null;
+                    }
+
+                    @Override
+                    public int getRemotePort() {
+                        return 0;
+                    }
+
+                    @Override
+                    public String getLocalName() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getLocalAddr() {
+                        return null;
+                    }
+
+                    @Override
+                    public int getLocalPort() {
+                        return 0;
+                    }
+
+                    @Override
+                    public ServletContext getServletContext() {
+                        return null;
+                    }
+
+                    @Override
+                    public AsyncContext startAsync() throws IllegalStateException {
+                        return null;
+                    }
+
+                    @Override
+                    public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse) throws IllegalStateException {
+                        return null;
+                    }
+
+                    @Override
+                    public boolean isAsyncStarted() {
+                        return false;
+                    }
+
+                    @Override
+                    public boolean isAsyncSupported() {
+                        return false;
+                    }
+
+                    @Override
+                    public AsyncContext getAsyncContext() {
+                        return null;
+                    }
+
+                    @Override
+                    public DispatcherType getDispatcherType() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getAuthType() {
+                        return null;
+                    }
+
+                    @Override
+                    public Cookie[] getCookies() {
+                        return new Cookie[0];
+                    }
+
+                    @Override
+                    public long getDateHeader(String s) {
+                        return 0;
+                    }
+
+                    @Override
+                    public String getHeader(String s) {
+                        return null;
+                    }
+
+                    @Override
+                    public Enumeration<String> getHeaders(String s) {
+                        return null;
+                    }
+
+                    @Override
+                    public Enumeration<String> getHeaderNames() {
+                        return null;
+                    }
+
+                    @Override
+                    public int getIntHeader(String s) {
+                        return 0;
+                    }
+
+                    @Override
+                    public String getMethod() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getPathInfo() {
+                        return path;
+                    }
+
+                    @Override
+                    public String getPathTranslated() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getContextPath() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getQueryString() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getRemoteUser() {
+                        return null;
+                    }
+
+                    @Override
+                    public boolean isUserInRole(String s) {
+                        return false;
+                    }
+
+                    @Override
+                    public Principal getUserPrincipal() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getRequestedSessionId() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getRequestURI() {
+                        return null;
+                    }
+
+                    @Override
+                    public StringBuffer getRequestURL() {
+                        return null;
+                    }
+
+                    @Override
+                    public String getServletPath() {
+                        return null;
+                    }
+
+                    @Override
+                    public HttpSession getSession(boolean b) {
+                        return null;
+                    }
+
+                    @Override
+                    public HttpSession getSession() {
+                        return null;
+                    }
+
+                    @Override
+                    public String changeSessionId() {
+                        return null;
+                    }
+
+                    @Override
+                    public boolean isRequestedSessionIdValid() {
+                        return false;
+                    }
+
+                    @Override
+                    public boolean isRequestedSessionIdFromCookie() {
+                        return false;
+                    }
+
+                    @Override
+                    public boolean isRequestedSessionIdFromURL() {
+                        return false;
+                    }
+
+                    @Override
+                    public boolean isRequestedSessionIdFromUrl() {
+                        return false;
+                    }
+
+                    @Override
+                    public boolean authenticate(HttpServletResponse httpServletResponse) throws IOException, ServletException {
+                        return false;
+                    }
+
+                    @Override
+                    public void login(String s, String s1) throws ServletException {
+
+                    }
+
+                    @Override
+                    public void logout() throws ServletException {
+
+                    }
+
+                    @Override
+                    public Collection<Part> getParts() throws IOException, ServletException {
+                        return null;
+                    }
+
+                    @Override
+                    public Part getPart(String s) throws IOException, ServletException {
+                        return null;
+                    }
+
+                    @Override
+                    public <T extends HttpUpgradeHandler> T upgrade(Class<T> aClass) throws IOException, ServletException {
+                        return null;
+                    }
+
+                    @Override
+                    public Resource getResource() {
+                        return null;
+                    }
+
+                    @Override
+                    public ResourceResolver getResourceResolver() {
+                        return null;
+                    }
+
+                    @Override
+                    public RequestPathInfo getRequestPathInfo() {
+                        Resource r = new Resource() {
+                            @Override
+                            public String getPath() {
+                                return path;
+                            }
+
+                            @Override
+                            public String getName() {
+                                return null;
+                            }
+
+                            @Override
+                            public Resource getParent() {
+                                return null;
+                            }
+
+                            @Override
+                            public Iterator<Resource> listChildren() {
+                                return null;
+                            }
+
+                            @Override
+                            public Iterable<Resource> getChildren() {
+                                return null;
+                            }
+
+                            @Override
+                            public Resource getChild(String s) {
+                                return null;
+                            }
+
+                            @Override
+                            public String getResourceType() {
+                                return null;
+                            }
+
+                            @Override
+                            public String getResourceSuperType() {
+                                return null;
+                            }
+
+                            @Override
+                            public boolean hasChildren() {
+                                return false;
+                            }
+
+                            @Override
+                            public boolean isResourceType(String s) {
+                                return false;
+                            }
+
+                            @Override
+                            public ResourceMetadata getResourceMetadata() {
+                                ResourceMetadata metadata = new ResourceMetadata(){
+                                    @Override
+                                    public String getResolutionPath() {
+                                        return path;
+                                    }
+
+                                    @Override
+                                    public String getResolutionPathInfo() {
+                                        return path;
+                                    }
+                                };
+
+                                return metadata;
+                            }
+
+                            @Override
+                            public ResourceResolver getResourceResolver() {
+                                return null;
+                            }
+
+                            @Override
+                            public <AdapterType> AdapterType adaptTo(Class<AdapterType> aClass) {
+                                return null;
+                            }
+                        };
+                        return new SlingRequestPathInfo(r);
+                    }
+
+                    @Override
+                    public RequestParameter getRequestParameter(String s) {
+                        return null;
+                    }
+
+                    @Override
+                    public RequestParameter[] getRequestParameters(String s) {
+                        return new RequestParameter[0];
+                    }
+
+                    @Override
+                    public RequestParameterMap getRequestParameterMap() {
+                        return null;
+                    }
+
+                    @Override
+                    public List<RequestParameter> getRequestParameterList() {
+                        return null;
+                    }
+
+                    @Override
+                    public RequestDispatcher getRequestDispatcher(String s, RequestDispatcherOptions requestDispatcherOptions) {
+                        return null;
+                    }
+
+                    @Override
+                    public RequestDispatcher getRequestDispatcher(Resource resource, RequestDispatcherOptions requestDispatcherOptions) {
+                        return null;
+                    }
+
+                    @Override
+                    public RequestDispatcher getRequestDispatcher(Resource resource) {
+                        return null;
+                    }
+
+                    @Override
+                    public Cookie getCookie(String s) {
+                        return null;
+                    }
+
+                    @Override
+                    public String getResponseContentType() {
+                        return null;
+                    }
+
+                    @Override
+                    public Enumeration<String> getResponseContentTypes() {
+                        return null;
+                    }
+
+                    @Override
+                    public ResourceBundle getResourceBundle(Locale locale) {
+                        return null;
+                    }
+
+                    @Override
+                    public ResourceBundle getResourceBundle(String s, Locale locale) {
+                        return null;
+                    }
+
+                    @Override
+                    public RequestProgressTracker getRequestProgressTracker() {
+                        return null;
+                    }
+                })
         );
     }
 }