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