This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13382-2 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-api.git
commit 9c6113e3e2a2f631569e7cc26e150534c04e27da Author: Joerg Hoh <[email protected]> AuthorDate: Wed Nov 11 17:50:49 2026 +0100 SLING-13382 warn on parent traversal in DeepReadValueMapDecorator --- pom.xml | 5 + .../api/wrappers/DeepReadValueMapDecorator.java | 39 ++++- .../wrappers/DeepReadValueMapDecoratorTest.java | 163 +++++++++++++++++++++ 3 files changed, 206 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 9244f3c..b2494d5 100644 --- a/pom.xml +++ b/pom.xml @@ -95,6 +95,11 @@ </dependency> <!-- Testing --> + <dependency> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-simple</artifactId> + <scope>test</scope> + </dependency> <dependency> <groupId>junit</groupId> <artifactId>junit</artifactId> diff --git a/src/main/java/org/apache/sling/api/wrappers/DeepReadValueMapDecorator.java b/src/main/java/org/apache/sling/api/wrappers/DeepReadValueMapDecorator.java index 51f73b5..6ceaf47 100644 --- a/src/main/java/org/apache/sling/api/wrappers/DeepReadValueMapDecorator.java +++ b/src/main/java/org/apache/sling/api/wrappers/DeepReadValueMapDecorator.java @@ -18,18 +18,29 @@ */ package org.apache.sling.api.wrappers; +import java.util.Locale; + import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; +import org.apache.sling.api.resource.ResourceUtil; import org.apache.sling.api.resource.ValueMap; import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A value map wrapper which implements deep reading of properties * based on the resource tree. + * <p>Deep reads outside the starting resource's subtree, or with paths that + * cannot be normalized, log a WARN and continue the original lookup. + * This decorator does not provide subtree confinement; access is governed + * by the starting resource's resolver permissions. * @since 2.5 (Sling API Bundle 2.7.0) */ public class DeepReadValueMapDecorator extends ValueMapDecorator { + private static final Logger LOG = LoggerFactory.getLogger(DeepReadValueMapDecorator.class); + private final String pathPrefix; private final ResourceResolver resolver; @@ -48,7 +59,20 @@ public class DeepReadValueMapDecorator extends ValueMapDecorator { if (pos == -1) { return this.base; } - final Resource rsrc = this.resolver.getResource(pathPrefix + name.substring(0, pos)); + final String requestedPath = pathPrefix + name.substring(0, pos); + final String normalizedBasePath = ResourceUtil.normalize(pathPrefix); + final String normalizedPath = ResourceUtil.normalize(requestedPath); + if (normalizedBasePath == null + || normalizedPath == null + || !(normalizedBasePath.equals("/") + || normalizedPath.equals(normalizedBasePath) + || normalizedPath.startsWith(normalizedBasePath + "/"))) { + LOG.warn( + "Deep read outside resource subtree or path cannot be normalized: resource '{}', requested path '{}'", + sanitizeForMessage(pathPrefix.substring(0, pathPrefix.length() - 1)), + sanitizeForMessage(requestedPath)); + } + final Resource rsrc = this.resolver.getResource(requestedPath); if (rsrc != null) { final ValueMap vm = rsrc.adaptTo(ValueMap.class); if (vm != null) { @@ -58,6 +82,19 @@ public class DeepReadValueMapDecorator extends ValueMapDecorator { return ValueMap.EMPTY; // fall back } + private static String sanitizeForMessage(final String value) { + final StringBuilder result = new StringBuilder(value.length()); + for (int i = 0; i < value.length(); i++) { + final char character = value.charAt(i); + if (Character.isISOControl(character) || character == '\u2028' || character == '\u2029') { + result.append(String.format(Locale.ROOT, "\\u%04x", (int) character)); + } else { + result.append(character); + } + } + return result.toString(); + } + private String getPropertyName(final String name) { final int pos = name.lastIndexOf("/"); if (pos == -1) { diff --git a/src/test/java/org/apache/sling/api/wrappers/DeepReadValueMapDecoratorTest.java b/src/test/java/org/apache/sling/api/wrappers/DeepReadValueMapDecoratorTest.java new file mode 100644 index 0000000..c9352b8 --- /dev/null +++ b/src/test/java/org/apache/sling/api/wrappers/DeepReadValueMapDecoratorTest.java @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sling.api.wrappers; + +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; +import java.util.Map; + +import org.apache.sling.api.resource.Resource; +import org.apache.sling.api.resource.ResourceResolver; +import org.apache.sling.api.resource.ValueMap; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +public class DeepReadValueMapDecoratorTest { + + private ResourceResolver resolver; + private DeepReadValueMapDecorator decorator; + private ByteArrayOutputStream logOutput; + private PrintStream originalError; + private PrintStream capturedError; + + @Before + public void setUp() { + originalError = System.err; + logOutput = new ByteArrayOutputStream(); + capturedError = new PrintStream(logOutput, true, StandardCharsets.UTF_8); + System.setErr(capturedError); + resolver = mock(ResourceResolver.class); + decorator = createDecorator("/content/site/page"); + logOutput.reset(); + } + + @After + public void tearDown() { + System.setErr(originalError); + capturedError.close(); + } + + private DeepReadValueMapDecorator createDecorator(final String path) { + final Resource resource = mock(Resource.class); + when(resource.getPath()).thenReturn(path); + when(resource.getResourceResolver()).thenReturn(resolver); + return new DeepReadValueMapDecorator(resource, new ValueMapDecorator(Map.of("title", "base"))); + } + + // Stub the original lookup string: normalization must only affect diagnostics. + private void stubTarget(final String path) { + final Resource target = mock(Resource.class); + when(resolver.getResource(path)).thenReturn(target); + when(target.adaptTo(ValueMap.class)).thenReturn(new ValueMapDecorator(Map.of("title", "target"))); + } + + private String logs() { + return logOutput.toString(StandardCharsets.UTF_8); + } + + private void assertWarning(final String resourcePath, final String requestedPath, final long count) { + assertEquals(count, logs().lines().filter(line -> line.contains("WARN")).count()); + assertTrue(logs().contains("resource '" + resourcePath + "', requested path '" + requestedPath + "'")); + } + + @Test + public void testParentTraversalWarnsAndContinuesForAllReadMethods() { + final String requestedPath = "/content/site/page/../sibling"; + stubTarget(requestedPath); + + assertEquals("target", decorator.get("../sibling/title")); + assertEquals("target", decorator.get("../sibling/title", String.class)); + assertEquals("target", decorator.get("../sibling/title", "default")); + assertTrue(decorator.containsKey("../sibling/title")); + assertWarning("/content/site/page", requestedPath, 4); + } + + @Test + public void testContainedPathsDoNotWarn() { + for (final String basePath : new String[] {"/content/site/page", "/content//site/page/"}) { + final DeepReadValueMapDecorator map = createDecorator(basePath); + for (final String key : + new String[] {"child/title", "child/../title", "./title", "child//grandchild/title"}) { + stubTarget(basePath + "/" + key.substring(0, key.lastIndexOf('/'))); + assertEquals(key, "target", map.get(key)); + } + } + assertTrue(logs().isEmpty()); + } + + @Test + public void testSimilarPrefixIsOutsideSubtree() { + final String requestedPath = "/content/site/page/../page2"; + stubTarget(requestedPath); + + assertEquals("target", decorator.get("../page2/title")); + assertWarning("/content/site/page", requestedPath, 1); + } + + @Test + public void testUnnormalizablePathsWarnAndContinue() { + for (final String key : new String[] {"../../../../secret/title", ".../title"}) { + final String requestedPath = "/content/site/page/" + key.substring(0, key.lastIndexOf('/')); + stubTarget(requestedPath); + + assertEquals(key, "target", decorator.get(key)); + assertWarning("/content/site/page", requestedPath, 1); + logOutput.reset(); + } + } + + @Test + public void testRootWarnsOnlyForTraversalAboveRoot() { + final DeepReadValueMapDecorator root = createDecorator("/"); + stubTarget("//content"); + stubTarget("//."); + stubTarget("//.."); + + assertEquals("target", root.get("content/title")); + assertEquals("target", root.get("./title")); + assertTrue(logs().isEmpty()); + + assertEquals("target", root.get("../title")); + assertWarning("/", "//..", 1); + } + + @Test + public void testDirectPropertyReadDoesNotResolveOrWarn() { + assertEquals("base", decorator.get("title")); + verifyNoInteractions(resolver); + assertTrue(logs().isEmpty()); + } + + @Test + public void testDiagnosticsEscapeControlCharacters() { + final DeepReadValueMapDecorator map = createDecorator("/content/site\npage"); + stubTarget("/content/site\npage/../sibling\r\u001b\u2028"); + + assertEquals("target", map.get("../sibling\r\u001b\u2028/title")); + assertWarning("/content/site\\u000apage", "/content/site\\u000apage/../sibling\\u000d\\u001b\\u2028", 1); + } +}
