This is an automated email from the ASF dual-hosted git repository. sseifert pushed a commit to branch feature/SLING-10167-loadaliases-absolutepaths in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 5106577b256fcf1c3f88ec7edaa981c74f3827ed Author: Stefan Seifert <[email protected]> AuthorDate: Thu Feb 25 19:51:37 2021 +0100 SLING-10167 Ensure only valid absolute paths (not ending with "/") are used in JCR queries for loading existing aliases on startup --- .../impl/ResourceResolverFactoryActivator.java | 6 ++++-- .../resourceresolver/impl/mapping/MapEntries.java | 4 +++- .../impl/MockedResourceResolverImplTest.java | 10 +++++++++- .../resourceresolver/impl/mapping/MapEntriesTest.java | 18 ++++++++++++++++++ 4 files changed, 34 insertions(+), 4 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..30ed350 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,10 +314,11 @@ public class ResourceResolverFactoryActivator { String value = prefix.trim(); if (!value.isEmpty()) { if (value.startsWith("/")) { // absolute path should be given + // path must not end with "/" to be valid absolute path if (value.endsWith("/")) { - prefixSet.add(value); + prefixSet.add(StringUtils.removeEnd(value, "/")); } else { - prefixSet.add(value + "/"); + prefixSet.add(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
