Skip to content

SLING-12980 - escape capabilities with CapReqBuilder #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
niekraaijmakers wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

SLING-12980 - escape capabilities with CapReqBuilder #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/main/java/org/apache/sling/scriptingbundle/plugin/capability/Capabilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

import aQute.bnd.header.Attrs;
import aQute.bnd.header.Parameters;
import aQute.bnd.osgi.resource.CapReqBuilder;

public class Capabilities {

Expand Down Expand Up @@ -71,7 +72,12 @@ public Capabilities(
Parameters parameters = new Parameters();
for (ProvidedResourceTypeCapability capability : getProvidedResourceTypeCapabilities()) {
Attrs attributes = new Attrs();
attributes.putTyped(Constants.CAPABILITY_RESOURCE_TYPE_AT, capability.getResourceTypes());
// Escape special characters in resource types
Set<String> escapedResourceTypes = new LinkedHashSet<>();
for (String resourceType : capability.getResourceTypes()) {
escapedResourceTypes.add(CapReqBuilder.escapeFilterValue(resourceType));
}
attributes.putTyped(Constants.CAPABILITY_RESOURCE_TYPE_AT, escapedResourceTypes);
Optional.ofNullable(capability.getScriptEngine()).ifPresent(scriptEngine ->
attributes.put(Constants.CAPABILITY_SCRIPT_ENGINE_AT, scriptEngine)
);
Expand All @@ -82,7 +88,8 @@ public Capabilities(
attributes.putTyped(Constants.CAPABILITY_VERSION_AT, new aQute.bnd.version.Version(version.toString()))
);
Optional.ofNullable(capability.getExtendsResourceType()).ifPresent(extendedResourceType ->
attributes.put(Constants.CAPABILITY_EXTENDS_AT, extendedResourceType)
// Escape special characters in extended resource type
attributes.put(Constants.CAPABILITY_EXTENDS_AT, CapReqBuilder.escapeFilterValue(extendedResourceType))
);
Optional.ofNullable(capability.getRequestMethod()).ifPresent(method ->
attributes.put(Constants.CAPABILITY_METHODS_AT, method)
Expand All @@ -98,7 +105,8 @@ public Capabilities(

for (ProvidedScriptCapability scriptCapability : getProvidedScriptCapabilities()) {
Attrs attributes = new Attrs();
attributes.put(Constants.CAPABILITY_PATH_AT, scriptCapability.getPath());
// Escape special characters in path (this is where footer(v2) gets escaped!)
attributes.put(Constants.CAPABILITY_PATH_AT, CapReqBuilder.escapeFilterValue(scriptCapability.getPath()));
attributes.put(Constants.CAPABILITY_SCRIPT_ENGINE_AT, scriptCapability.getScriptEngine());
attributes.put(Constants.CAPABILITY_SCRIPT_EXTENSION_AT, scriptCapability.getScriptExtension());
parameters.add(Constants.CAPABILITY_NS, attributes);
Expand All @@ -111,13 +119,16 @@ public Capabilities(
for (RequiredResourceTypeCapability capability : getRequiredResourceTypeCapabilities()) {
Attrs attributes = new Attrs();

// Escape special characters in resource type for use in filter string
String escapedResourceType = CapReqBuilder.escapeFilterValue(capability.getResourceType());

StringBuilder filterValue = new StringBuilder("(&(!(" + ServletResolverConstants.SLING_SERVLET_SELECTORS + "=*))");
VersionRange versionRange = capability.getVersionRange();
if (versionRange != null) {
filterValue.append("(&").append(versionRange.toFilterString("version")).append("(").append(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES).append(
"=").append(capability.getResourceType()).append(")))");
"=").append(escapedResourceType).append(")))");
} else {
filterValue.append("(").append(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES).append("=").append(capability.getResourceType()).append("))");
filterValue.append("(").append(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES).append("=").append(escapedResourceType).append("))");
}

attributes.put(aQute.bnd.osgi.Constants.FILTER_DIRECTIVE, filterValue.toString());
Expand Down
142 changes: 142 additions & 0 deletions src/test/java/org/apache/sling/scriptingbundle/plugin/capability/CapabilityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
package org.apache.sling.scriptingbundle.plugin.capability;

import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;

import org.junit.Assert;
Expand Down Expand Up @@ -61,4 +63,144 @@ public void testGetRequiredCapabilitiesString() {
",sling.servlet;filter:=\"(&(!(sling.servlet.selectors=*))(sling.servlet.resourceTypes=/other/type))\"";
Assert.assertEquals(expectedHeaderValue, caps.getRequiredCapabilitiesString());
}

/**
* Test that parentheses in script paths are properly escaped in provided capabilities.
* This is the main test for the footer(v2) issue.
*/
@Test
public void testProvidedScriptCapabilityWithParenthesesInPath() {
Map<String, String> scriptEngineMappings = new HashMap<>();
scriptEngineMappings.put("html", "htl");

Set<ProvidedScriptCapability> scriptCaps = new LinkedHashSet<>();
scriptCaps.add(ProvidedScriptCapability.builder(scriptEngineMappings)
.withPath("/apps/corp/globals/components/content/footer(v2)/footer.html")
.build());
scriptCaps.add(ProvidedScriptCapability.builder(scriptEngineMappings)
.withPath("/apps/test/component(with)parentheses/script.html")
.build());

Capabilities caps = new Capabilities(Collections.emptySet(), scriptCaps, Collections.emptySet());
String result = caps.getProvidedCapabilitiesString();

// Verify parentheses are escaped with single backslashes
Assert.assertTrue("Path should contain escaped parentheses: " + result, result.contains("footer\\(v2\\)"));
Assert.assertTrue("Path should contain escaped parentheses: " + result, result.contains("\\(with\\)"));

// Verify the complete capability strings
Assert.assertTrue(result.contains("sling.servlet.paths=\"/apps/corp/globals/components/content/footer\\(v2\\)/footer.html\""));
Assert.assertTrue(result.contains("sling.servlet.paths=\"/apps/test/component\\(with\\)parentheses/script.html\""));
}

/**
* Test that parentheses in resource types are properly escaped in provided capabilities.
*/
@Test
public void testProvidedResourceTypeCapabilityWithParenthesesInResourceType() {
Set<ProvidedResourceTypeCapability> resourceTypeCaps = new LinkedHashSet<>();
resourceTypeCaps.add(ProvidedResourceTypeCapability.builder()
.withResourceTypes("my/type(v2)", "/libs/component(test)/type")
.withVersion(new Version("2.1.0"))
.withRequestExtension("json")
.build());

Capabilities caps = new Capabilities(resourceTypeCaps, Collections.emptySet(), Collections.emptySet());
String result = caps.getProvidedCapabilitiesString();

// Verify parentheses are escaped (List<String> output shows double backslashes)
Assert.assertTrue("Resource type " + result + " should contain escaped parentheses", result.contains("my/type\\\\(v2\\\\)"));
Assert.assertTrue("Resource type " + result + " should contain escaped parentheses", result.contains("/libs/component\\\\(test\\\\)/type"));
}

/**
* Test that parentheses in extended resource types are properly escaped.
*/
@Test
public void testProvidedCapabilityWithParenthesesInExtendsResourceType() {
Set<ProvidedResourceTypeCapability> resourceTypeCaps = new LinkedHashSet<>();
resourceTypeCaps.add(ProvidedResourceTypeCapability.builder()
.withResourceType("my/type")
.withExtendsResourceType("parent/type(v1)")
.build());

Capabilities caps = new Capabilities(resourceTypeCaps, Collections.emptySet(), Collections.emptySet());
String result = caps.getProvidedCapabilitiesString();

// Verify parentheses in extends attribute are escaped with single backslashes
Assert.assertTrue("Extended resource type should contain escaped parentheses: " + result,
result.contains("parent/type\\(v1\\)"));
}

/**
* Test that parentheses in required resource types are properly escaped in filter strings.
*/
@Test
public void testRequiredCapabilityWithParenthesesInResourceType() {
Set<RequiredResourceTypeCapability> resourceTypeCaps = new LinkedHashSet<>();
resourceTypeCaps.add(RequiredResourceTypeCapability.builder()
.withResourceType("my/type(v2)")
.build());

Capabilities caps = new Capabilities(Collections.emptySet(), Collections.emptySet(), resourceTypeCaps);
String result = caps.getRequiredCapabilitiesString();

// Verify parentheses are escaped in the filter string with single backslashes
Assert.assertTrue("Filter should contain escaped resource type: " + result,
result.contains("sling.servlet.resourceTypes=my/type\\(v2\\)"));

// Verify the complete filter is valid
Assert.assertTrue("Complete filter should be valid: " + result,
result.contains("filter:=\"(&(!(sling.servlet.selectors=*))(sling.servlet.resourceTypes=my/type\\(v2\\)))\""));
}

/**
* Test that other special filter characters (backslash, asterisk) are also properly escaped.
*/
@Test
public void testSpecialCharactersEscaping() {
Map<String, String> scriptEngineMappings = new HashMap<>();
scriptEngineMappings.put("html", "htl");

Set<ProvidedScriptCapability> scriptCaps = new LinkedHashSet<>();
// Test backslash escaping (backslash should become double-backslash)
scriptCaps.add(ProvidedScriptCapability.builder(scriptEngineMappings)
.withPath("/apps/test\\path/script.html")
.build());
// Test asterisk escaping
scriptCaps.add(ProvidedScriptCapability.builder(scriptEngineMappings)
.withPath("/apps/test*wildcard/script.html")
.build());

Capabilities caps = new Capabilities(Collections.emptySet(), scriptCaps, Collections.emptySet());
String result = caps.getProvidedCapabilitiesString();

// Verify special characters are escaped
// Backslash gets escaped to \\ (4 backslashes in Java = 2 in string)
// Asterisk gets escaped to \* (2 backslashes in Java = 1 in string)
Assert.assertTrue("Backslash should be escaped: " + result, result.contains("test\\\\path"));
Assert.assertTrue("Asterisk should be escaped: " + result, result.contains("test\\*wildcard"));
}

/**
* Test that paths without special characters remain unchanged.
*/
@Test
public void testNormalPathsUnchanged() {
Map<String, String> scriptEngineMappings = new HashMap<>();
scriptEngineMappings.put("html", "htl");

Set<ProvidedScriptCapability> scriptCaps = new LinkedHashSet<>();
scriptCaps.add(ProvidedScriptCapability.builder(scriptEngineMappings)
.withPath("/apps/myapp/components/footer/footer.html")
.build());

Capabilities caps = new Capabilities(Collections.emptySet(), scriptCaps, Collections.emptySet());
String result = caps.getProvidedCapabilitiesString();

// Verify normal paths work correctly without unnecessary escaping
Assert.assertTrue(result.contains("sling.servlet.paths=\"/apps/myapp/components/footer/footer.html\""));
// Verify no backslashes were added where they shouldn't be
Assert.assertFalse("Should not contain escaped forward slashes", result.contains("\\/"));
}
}