This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-api.git
The following commit(s) were added to refs/heads/master by this push:
new 6d8d251 SLING-13382 warn on parent traversal in
DeepReadValueMapDecorator (#91)
6d8d251 is described below
commit 6d8d251dec823c26fe6a8e3b62292fdee2c9840b
Author: Jörg Hoh <[email protected]>
AuthorDate: Thu Nov 12 12:54:12 2026 +0100
SLING-13382 warn on parent traversal in DeepReadValueMapDecorator (#91)
---
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);
+ }
+}
