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-security.git
The following commit(s) were added to refs/heads/master by this push:
new 68ab7a1 SLING-13317 improve configuration parsing for the
ContentDispositionFilter (#17)
68ab7a1 is described below
commit 68ab7a142ef920e43077bdef5a383d98a9391724
Author: Jörg Hoh <[email protected]>
AuthorDate: Thu Aug 20 12:27:07 2026 +0200
SLING-13317 improve configuration parsing for the ContentDispositionFilter
(#17)
---
.../security/impl/ContentDispositionFilter.java | 94 +++++++++++++++-------
.../ContentDispositionFilterConfiguration.java | 3 +
.../impl/ContentDispositionFilterTest.java | 53 ++++++++++++
3 files changed, 122 insertions(+), 28 deletions(-)
diff --git
a/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java
b/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java
index f9f3005..20f7314 100644
--- a/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java
+++ b/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java
@@ -90,37 +90,49 @@ public class ContentDispositionFilter implements Filter {
for (String path :
configuration.sling_content_disposition_paths()) {
path = path.trim();
if (path.length() > 0) {
- int idx = path.indexOf('*');
- int colonIdx = path.indexOf(":");
-
- if (colonIdx > -1 && colonIdx < idx) {
- // ':' in paths is not allowed
- logger.info(
- "wildcard ('*') in content type is not
allowed, but found content type with value '{}'",
- path.substring(colonIdx));
- } else {
- String p = null;
- if (idx >= 0) {
- if (idx > 0) {
- p = path.substring(0, idx);
- pfxs.add(p);
- } else {
- // we don't allow "*" - that would defeat the
- // purpose.
- logger.info("catch-all wildcard for paths not
allowed.");
- }
+ // Media types always contain '/' and never ':', whereas
JCR resource paths
+ // may contain ':' in namespaced segments (e.g.
'jcr:content'). The optional
+ // content type list is therefore separated by the *last*
':' of the entry,
+ // and only if the remainder actually parses as a list of
media types.
+ String entryPath = path;
+ String contentTypesDefinition = null;
+ final int colonIdx = path.lastIndexOf(':');
+ if (colonIdx > -1) {
+ final String candidate = path.substring(colonIdx + 1);
+ if (isContentTypeList(candidate)) {
+ entryPath = path.substring(0, colonIdx);
+ contentTypesDefinition = candidate;
+ } else if (candidate.indexOf('/') >= 0 ||
candidate.indexOf('*') >= 0) {
+ // Neither a valid media type list nor a JCR name:
reject the entry
+ // loudly instead of guessing - a protection entry
must never be
+ // silently rewritten. This also rejects wildcard
content types.
+ logger.warn(
+ "Ignoring invalid content disposition
entry '{}': the text after the last ':' ('{}') is neither a valid content type
list nor a JCR name.",
+ path,
+ candidate);
+ continue;
} else {
- if (colonIdx > -1) {
- p = path.substring(0, colonIdx);
- } else {
- p = path;
- }
- paths.add(p);
+ logger.info(
+ "Content disposition entry '{}' contains
':' but no content type list; the complete entry is used as the resource path.",
+ path);
}
- if (colonIdx != -1 && p != null) {
- Set<String> contentTypes =
getContentTypes(path.substring(colonIdx + 1));
- contentTypesMap.put(p, contentTypes);
+ }
+ final int idx = entryPath.indexOf('*');
+ String p = null;
+ if (idx >= 0) {
+ if (idx > 0) {
+ p = entryPath.substring(0, idx);
+ pfxs.add(p);
+ } else {
+ // we don't allow the "*" wildcard, it would
defeat the purpose.
+ logger.info("catch-all wildcard for paths not
allowed.");
}
+ } else {
+ p = entryPath;
+ paths.add(p);
+ }
+ if (contentTypesDefinition != null && p != null) {
+ contentTypesMap.put(p,
getContentTypes(contentTypesDefinition));
}
}
}
@@ -178,6 +190,32 @@ public class ContentDispositionFilter implements Filter {
return contentTypesSet;
}
+ /**
+ * Checks whether the provided value parses as a comma separated list of
media
+ * types: each entry must contain exactly one '/', which must not be the
first
+ * or last character, and must not contain ':' or '*'.
+ *
+ * @param value the text after the last ':' of a configuration entry
+ * @return {@code true} if the value is a valid content type list
+ */
+ private static boolean isContentTypeList(final String value) {
+ if (value.trim().isEmpty()) {
+ return false;
+ }
+ for (final String token : value.split(",")) {
+ final String type = token.trim();
+ final int slashIdx = type.indexOf('/');
+ if (slashIdx <= 0
+ || slashIdx == type.length() - 1
+ || type.indexOf('/', slashIdx + 1) != -1
+ || type.indexOf(':') != -1
+ || type.indexOf('*') != -1) {
+ return false;
+ }
+ }
+ return true;
+ }
+
// ----------- INNER CLASSES ------------
protected class RewriterResponse extends SlingHttpServletResponseWrapper {
diff --git
a/src/main/java/org/apache/sling/security/impl/ContentDispositionFilterConfiguration.java
b/src/main/java/org/apache/sling/security/impl/ContentDispositionFilterConfiguration.java
index 5d9baec..27bbe95 100644
---
a/src/main/java/org/apache/sling/security/impl/ContentDispositionFilterConfiguration.java
+++
b/src/main/java/org/apache/sling/security/impl/ContentDispositionFilterConfiguration.java
@@ -32,6 +32,9 @@ public @interface ContentDispositionFilterConfiguration {
description =
"These resource paths are covered by the filter. "
+ "Each entry is of the form '<path> [ : <excluded
content type> {,<excluded content type>} ]'. "
+ + "The path and the optional content type list are
separated by the last ':' of the entry that is "
+ + "followed by a valid content type list; a ':'
inside a JCR namespaced path segment (e.g. 'jcr:content') "
+ + "is treated as part of the path. "
+ "Invalid entries are logged and ignored. <path>
must be an absolute path and may contain a wildcard ('*') at the end, to match
every resource path with the given path prefix.")
String[] sling_content_disposition_paths() default {};
diff --git
a/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java
b/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java
index b85f309..ba12361 100644
---
a/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java
+++
b/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java
@@ -217,6 +217,59 @@ public class ContentDispositionFilterTest {
Assert.assertEquals(2, contentDispositionExcludedPaths.size());
}
+ /**
+ * A ':' in a JCR namespaced path segment (e.g. 'jcr:content') must not
silently
+ * truncate the configured path (which would leave the intended path
unprotected).
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void test_activator_namespaced_path() throws Throwable {
+ callActivateWithConfiguration(new String[]
{"/content/site/file.svg/jcr:content"}, new String[] {""});
+ Set<String> contentDispositionPaths =
+ (Set<String>)
PrivateAccessor.getField(contentDispositionFilter, "contentDispositionPaths");
+ Assert.assertEquals(1, contentDispositionPaths.size());
+
Assert.assertTrue(contentDispositionPaths.contains("/content/site/file.svg/jcr:content"));
+ String[] contentDispositionPathsPfx =
+ (String[]) PrivateAccessor.getField(contentDispositionFilter,
"contentDispositionPathsPfx");
+ Assert.assertEquals(0, contentDispositionPathsPfx.length);
+ Map<String, Set<String>> contentTypesMapping =
+ (Map<String, Set<String>>)
PrivateAccessor.getField(contentDispositionFilter, "contentTypesMapping");
+ Assert.assertEquals(0, contentTypesMapping.size());
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void test_activator_namespaced_path_with_content_types() throws
Throwable {
+ callActivateWithConfiguration(
+ new String[]
{"/content/site/file.svg/jcr:content:image/svg+xml,text/html"}, new String[]
{""});
+ Set<String> contentDispositionPaths =
+ (Set<String>)
PrivateAccessor.getField(contentDispositionFilter, "contentDispositionPaths");
+ Assert.assertEquals(1, contentDispositionPaths.size());
+
Assert.assertTrue(contentDispositionPaths.contains("/content/site/file.svg/jcr:content"));
+ Map<String, Set<String>> contentTypesMapping =
+ (Map<String, Set<String>>)
PrivateAccessor.getField(contentDispositionFilter, "contentTypesMapping");
+ Assert.assertEquals(1, contentTypesMapping.size());
+ Set<String> mapping =
contentTypesMapping.get("/content/site/file.svg/jcr:content");
+ Assert.assertEquals(2, mapping.size());
+ Assert.assertTrue(mapping.contains("image/svg+xml"));
+ Assert.assertTrue(mapping.contains("text/html"));
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void test_activator_invalid_content_type_list_rejected() throws
Throwable {
+ callActivateWithConfiguration(new String[] {"/libs:text/html/*"}, new
String[] {""});
+ Set<String> contentDispositionPaths =
+ (Set<String>)
PrivateAccessor.getField(contentDispositionFilter, "contentDispositionPaths");
+ Assert.assertEquals(0, contentDispositionPaths.size());
+ String[] contentDispositionPathsPfx =
+ (String[]) PrivateAccessor.getField(contentDispositionFilter,
"contentDispositionPathsPfx");
+ Assert.assertEquals(0, contentDispositionPathsPfx.length);
+ Map<String, Set<String>> contentTypesMapping =
+ (Map<String, Set<String>>)
PrivateAccessor.getField(contentDispositionFilter, "contentTypesMapping");
+ Assert.assertEquals(0, contentTypesMapping.size());
+ }
+
@SuppressWarnings("unchecked")
@Test
public void test_getContentTypes() throws Throwable {
