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

sseifert pushed a commit to branch master
in repository 
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git


The following commit(s) were added to refs/heads/master by this push:
     new 30bf212  SLING-10167 Ensure only valid absolute paths (not ending with 
"/") are used in JCR queries for loading existing aliases on startup (#42)
30bf212 is described below

commit 30bf2129817ae20f9d480f8571d7219aef51a6ab
Author: Stefan Seifert <[email protected]>
AuthorDate: Tue Mar 2 12:32:01 2021 +0100

    SLING-10167 Ensure only valid absolute paths (not ending with "/") are used 
in JCR queries for loading existing aliases on startup (#42)
---
 .../impl/ResourceResolverFactoryActivator.java         |  8 +++-----
 .../resourceresolver/impl/mapping/MapEntries.java      |  4 +++-
 .../impl/MockedResourceResolverImplTest.java           | 10 +++++++++-
 .../resourceresolver/impl/mapping/MapEntriesTest.java  | 18 ++++++++++++++++++
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git 
a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
 
b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
index 94dd37d..f2feca6 100644
--- 
a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
+++ 
b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
@@ -26,6 +26,7 @@ import java.util.*;
 
 import org.apache.commons.collections4.BidiMap;
 import org.apache.commons.collections4.bidimap.TreeBidiMap;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.sling.api.resource.ResourceDecorator;
 import org.apache.sling.api.resource.ResourceResolverFactory;
 import org.apache.sling.api.resource.path.Path;
@@ -313,11 +314,8 @@ public class ResourceResolverFactoryActivator {
                 String value = prefix.trim();
                 if (!value.isEmpty()) {
                     if (value.startsWith("/")) { // absolute path should be 
given
-                        if (value.endsWith("/")) {
-                            prefixSet.add(value);
-                        } else {
-                            prefixSet.add(value + "/");
-                        }
+                        // path must not end with "/" to be valid absolute path
+                        prefixSet.add(StringUtils.removeEnd(value, "/"));
                     }else{
                         logger.warn("Path [{}] is ignored. As only absolute 
paths are allowed for alias optimization", value);
                     }
diff --git 
a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java 
b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
index fe3db2f..2608119 100644
--- 
a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
+++ 
b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
@@ -18,6 +18,7 @@
  */
 package org.apache.sling.resourceresolver.impl.mapping;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.sling.api.SlingConstants;
 import org.apache.sling.api.resource.*;
 import 
org.apache.sling.api.resource.observation.ExternalResourceChangeListener;
@@ -1036,8 +1037,9 @@ public class MapEntries implements
         baseQuery.append(" ").append("WHERE");
 
         if(allowedLocations.isEmpty()){
+            String jcrSystemPath = StringUtils.removeEnd(JCR_SYSTEM_PREFIX, 
"/");
             baseQuery.append(" ").append("(").append("NOT 
ISDESCENDANTNODE(page,")
-                    
.append("\"").append(JCR_SYSTEM_PREFIX).append("\"").append("))");
+                    
.append("\"").append(jcrSystemPath).append("\"").append("))");
 
         }else{
             Iterator<String> pathIterator = allowedLocations.iterator();
diff --git 
a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
 
b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
index a9f42ae..d0c7b84 100644
--- 
a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
+++ 
b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
@@ -19,6 +19,7 @@ package org.apache.sling.resourceresolver.impl;
 
 import static 
org.apache.sling.resourceresolver.util.MockTestUtil.getResourceName;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
@@ -36,6 +37,7 @@ import java.util.Set;
 
 import javax.servlet.http.HttpServletRequest;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.sling.api.resource.LoginException;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceMetadata;
@@ -237,7 +239,8 @@ public class MockedResourceResolverImplTest {
 
             @Override
             public String[] resource_resolver_allowed_alias_locations() {
-                return new String[]{"/apps/", "/libs/", "/content/"};
+                // deliberately put a mixture of paths with and without ending 
slash in here - both should be handled correct
+                return new String[]{"/apps/", "/libs", "/content/"};
             }
 
             @Override
@@ -317,6 +320,11 @@ public class MockedResourceResolverImplTest {
         }
         Assert.assertTrue(rrf instanceof ResourceResolverFactoryImpl);
         resourceResolverFactory = (ResourceResolverFactoryImpl) rrf;
+
+        // ensure allowed alias locations are *not* ending with a slash 
(invalid absolut path)
+        for (String path : activator.getAllowedAliasLocations()) {
+            assertFalse("Path must not end with '/': " + path, 
StringUtils.endsWith(path, "/"));
+        }
     }
 
     public static ResourceProviderHandler createRPHandler(ResourceProvider<?> 
rp, String pid, long ranking,
diff --git 
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
 
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
index eef569a..9672020 100644
--- 
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
+++ 
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
@@ -38,6 +38,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.observation.ResourceChange;
@@ -2089,4 +2090,21 @@ public class MapEntriesTest extends 
AbstractMappingMapEntriesTest {
             }
         }
     }
+
+    @Test
+    public void testLoadAliases_ValidAbsolutePath_DefaultPaths() {
+        
when(resourceResolverFactory.getAllowedAliasLocations()).thenReturn(Collections.emptySet());
+
+        when(resourceResolver.findResources(anyString(), 
eq("sql"))).thenAnswer(new Answer<Iterator<Resource>>() {
+            @Override
+            public Iterator<Resource> answer(InvocationOnMock invocation) 
throws Throwable {
+                String query = 
StringUtils.trim((String)invocation.getArguments()[0]);
+                assertEquals("SELECT sling:alias FROM nt:base AS page WHERE 
(NOT ISDESCENDANTNODE(page,\"/jcr:system\")) AND sling:alias IS NOT NULL", 
query);
+                return Collections.<Resource> emptySet().iterator();
+            }
+        });
+
+        mapEntries.doInit();
+    }
+
 }
\ No newline at end of file

Reply via email to