You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2021/08/23 07:01:58 UTC

[sling-scriptingbundle-maven-plugin] branch master updated: SLING-10707 correctly escape values in capability headers (#5)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-scriptingbundle-maven-plugin.git


The following commit(s) were added to refs/heads/master by this push:
     new 02d8d9c  SLING-10707 correctly escape values in capability headers (#5)
02d8d9c is described below

commit 02d8d9cf4d345b21da073068a9cbd4252ab9487a
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Mon Aug 23 09:01:52 2021 +0200

    SLING-10707 correctly escape values in capability headers (#5)
    
    leverage bnd Parameters class for escaping
---
 .../plugin/capability/Capabilities.java            | 95 ++++++++--------------
 .../capability/ProvidedResourceTypeCapability.java | 10 +++
 .../plugin/processor/Constants.java                |  6 +-
 .../plugin/capability/CapabilityTest.java          | 64 +++++++++++++++
 4 files changed, 110 insertions(+), 65 deletions(-)

diff --git a/src/main/java/org/apache/sling/scriptingbundle/plugin/capability/Capabilities.java b/src/main/java/org/apache/sling/scriptingbundle/plugin/capability/Capabilities.java
index e332ac4..d8fc5cb 100644
--- a/src/main/java/org/apache/sling/scriptingbundle/plugin/capability/Capabilities.java
+++ b/src/main/java/org/apache/sling/scriptingbundle/plugin/capability/Capabilities.java
@@ -37,6 +37,9 @@ import org.apache.sling.scriptingbundle.plugin.processor.ResourceTypeFolderAnaly
 import org.jetbrains.annotations.NotNull;
 import org.osgi.framework.VersionRange;
 
+import aQute.bnd.header.Attrs;
+import aQute.bnd.header.Parameters;
+
 public class Capabilities {
 
     private final Set<ProvidedResourceTypeCapability> providedResourceTypeCapabilities;
@@ -74,84 +77,65 @@ public class Capabilities {
     }
 
     public @NotNull String getProvidedCapabilitiesString() {
-        StringBuilder builder = new StringBuilder();
-        int pcNum = getProvidedResourceTypeCapabilities().size();
-        int psNum = getProvidedScriptCapabilities().size();
-        int pcIndex = 0;
-        int psIndex = 0;
+        Parameters parameters = new Parameters();
         for (ProvidedResourceTypeCapability capability : getProvidedResourceTypeCapabilities()) {
-            builder.append(Constants.CAPABILITY_NS).append(";");
-            processListAttribute(Constants.CAPABILITY_RESOURCE_TYPE_AT, builder,capability.getResourceTypes());
+            Attrs attributes = new Attrs();
+            attributes.putTyped(Constants.CAPABILITY_RESOURCE_TYPE_AT, capability.getResourceTypes());
             Optional.ofNullable(capability.getScriptEngine()).ifPresent(scriptEngine ->
-                    builder.append(";")
-                            .append(Constants.CAPABILITY_SCRIPT_ENGINE_AT).append("=").append("\"").append(scriptEngine).append("\"")
+                    attributes.put(Constants.CAPABILITY_SCRIPT_ENGINE_AT, scriptEngine)
             );
             Optional.ofNullable(capability.getScriptExtension()).ifPresent(scriptExtension ->
-                    builder.append(";")
-                            .append(Constants.CAPABILITY_SCRIPT_EXTENSION_AT).append("=").append("\"").append(scriptExtension).append("\"")
+                    attributes.put(Constants.CAPABILITY_SCRIPT_EXTENSION_AT, scriptExtension)
             );
             Optional.ofNullable(capability.getVersion()).ifPresent(version ->
-                    builder.append(";")
-                            .append(Constants.CAPABILITY_VERSION_AT).append("=").append("\"").append(version).append("\"")
+                    attributes.putTyped(Constants.CAPABILITY_VERSION_AT, new aQute.bnd.version.Version(version.toString()))
             );
             Optional.ofNullable(capability.getExtendsResourceType()).ifPresent(extendedResourceType ->
-                    builder.append(";")
-                            .append(Constants.CAPABILITY_EXTENDS_AT).append("=").append("\"").append(extendedResourceType).append("\"")
+                    attributes.put(Constants.CAPABILITY_EXTENDS_AT, extendedResourceType)
             );
             Optional.ofNullable(capability.getRequestMethod()).ifPresent(method ->
-                    builder.append(";")
-                            .append(Constants.CAPABILITY_METHODS_AT).append("=").append("\"").append(method).append("\"")
+                    attributes.put(Constants.CAPABILITY_METHODS_AT, method)
             );
             Optional.ofNullable(capability.getRequestExtension()).ifPresent(requestExtension ->
-                    builder.append(";")
-                            .append(Constants.CAPABILITY_EXTENSIONS_AT).append("=").append("\"").append(requestExtension).append("\"")
+                    attributes.put(Constants.CAPABILITY_EXTENSIONS_AT, requestExtension)
             );
             if (!capability.getSelectors().isEmpty()) {
-                builder.append(";");
-                processListAttribute(Constants.CAPABILITY_SELECTORS_AT, builder, capability.getSelectors());
-            }
-            if (pcIndex < pcNum - 1) {
-                builder.append(",");
+                attributes.putTyped(Constants.CAPABILITY_SELECTORS_AT, capability.getSelectors());
             }
-            pcIndex++;
-        }
-        if (builder.length() > 0 && psNum > 0) {
-            builder.append(",");
+            parameters.add(Constants.CAPABILITY_NS, attributes);
         }
+
         for (ProvidedScriptCapability scriptCapability : getProvidedScriptCapabilities()) {
-            builder.append(Constants.CAPABILITY_NS).append(";").append(Constants.CAPABILITY_PATH_AT).append("=\"").append(scriptCapability.getPath()).append(
-                    "\";").append(Constants.CAPABILITY_SCRIPT_ENGINE_AT).append("=").append(scriptCapability.getScriptEngine()).append(";")
-                    .append(Constants.CAPABILITY_SCRIPT_EXTENSION_AT).append("=").append(scriptCapability.getScriptExtension());
-            if (psIndex < psNum - 1) {
-                builder.append(",");
-            }
-            psIndex++;
+            Attrs attributes = new Attrs();
+            attributes.put(Constants.CAPABILITY_PATH_AT, 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);
         }
-        return builder.toString();
+        return parameters.toString();
     }
 
     public @NotNull String getRequiredCapabilitiesString() {
-        StringBuilder builder = new StringBuilder();
-        int pcNum = getRequiredResourceTypeCapabilities().size();
-        int pcIndex = 0;
+        Parameters parameters = new Parameters();
         for (RequiredResourceTypeCapability capability : getRequiredResourceTypeCapabilities()) {
-            builder.append(Constants.CAPABILITY_NS).append(";filter:=\"").append("(&(!(" + ServletResolverConstants.SLING_SERVLET_SELECTORS + "=*))");
+            Attrs attributes = new Attrs();
+
+            StringBuilder filterValue = new StringBuilder("(&(!(" + ServletResolverConstants.SLING_SERVLET_SELECTORS + "=*))");
             VersionRange versionRange = capability.getVersionRange();
             if (versionRange != null) {
-                builder.append("(&").append(versionRange.toFilterString("version")).append("(").append(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES).append(
-                        "=").append(capability.getResourceType()).append(")))\"");
+                filterValue.append("(&").append(versionRange.toFilterString("version")).append("(").append(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES).append(
+                        "=").append(capability.getResourceType()).append(")))");
             } else {
-                builder.append("(").append(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES).append("=").append(capability.getResourceType()).append("))\"");
+                filterValue.append("(").append(ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES).append("=").append(capability.getResourceType()).append("))");
             }
+            
+            attributes.put(aQute.bnd.osgi.Constants.FILTER_DIRECTIVE, filterValue.toString());
             if (capability.isOptional()) {
-                builder.append(aQute.bnd.osgi.Constants.RESOLUTION_DIRECTIVE).append("=").append(aQute.bnd.osgi.Constants.OPTIONAL);
-            }
-            if (pcIndex < pcNum - 1) {
-                builder.append(",");
+                attributes.put(aQute.bnd.osgi.Constants.RESOLUTION_DIRECTIVE, aQute.bnd.osgi.Constants.OPTIONAL);
             }
-            pcIndex++;
+            parameters.add(Constants.CAPABILITY_NS, attributes);
         }
-        return builder.toString();
+        return parameters.toString();
     }
 
     public static @NotNull Capabilities fromFileSystemTree(@NotNull Path root, @NotNull Stream<Path> files, @NotNull Logger logger,
@@ -177,17 +161,4 @@ public class Capabilities {
         return new Capabilities(providedResourceTypeCapabilities, providedScriptCapabilities, requiredResourceTypeCapabilities);
     }
 
-    private void processListAttribute(@NotNull String capabilityAttribute, @NotNull StringBuilder builder, @NotNull Set<String> values) {
-        builder.append(capabilityAttribute).append("=").append("\"");
-        int valuesSize = values.size();
-        int valueIndex = 0;
-        for (String item : values) {
-            builder.append(item);
-            if (valueIndex < valuesSize - 1) {
-                builder.append(",");
-            }
-            valueIndex++;
-        }
-        builder.append("\"");
-    }
 }
diff --git a/src/main/java/org/apache/sling/scriptingbundle/plugin/capability/ProvidedResourceTypeCapability.java b/src/main/java/org/apache/sling/scriptingbundle/plugin/capability/ProvidedResourceTypeCapability.java
index 2b425c7..3da1a13 100644
--- a/src/main/java/org/apache/sling/scriptingbundle/plugin/capability/ProvidedResourceTypeCapability.java
+++ b/src/main/java/org/apache/sling/scriptingbundle/plugin/capability/ProvidedResourceTypeCapability.java
@@ -18,8 +18,10 @@
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
 package org.apache.sling.scriptingbundle.plugin.capability;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Objects;
 import java.util.Set;
 
@@ -148,6 +150,10 @@ public class ProvidedResourceTypeCapability {
             return this;
         }
 
+        public Builder withResourceTypes(@NotNull String... resourceTypes) {
+            return withResourceTypes(new LinkedHashSet<>(Arrays.asList(resourceTypes)));
+        }
+
         public Builder withResourceType(@NotNull String resourceType) {
             if (StringUtils.isEmpty(resourceType)) {
                 throw new IllegalArgumentException("The script's resourceType cannot be null or empty.");
@@ -191,6 +197,10 @@ public class ProvidedResourceTypeCapability {
             return this;
         }
 
+        public Builder withSelectors(@NotNull String... selectors) {
+            return withSelectors(new LinkedHashSet<>(Arrays.asList(selectors)));
+        }
+
         public Builder fromCapability(@NotNull ProvidedResourceTypeCapability capability) {
             if (capability.getResourceTypes().isEmpty()) {
                 throw new IllegalArgumentException("The script's resourceTypes cannot be null or empty.");
diff --git a/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/Constants.java b/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/Constants.java
index ed4b6f4..c9da665 100644
--- a/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/Constants.java
+++ b/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/Constants.java
@@ -37,12 +37,12 @@ public final class Constants {
     public static final String EXTENDS_FILE = "extends";
     public static final String REQUIRES_FILE = "requires";
     public static final String CAPABILITY_NS = "sling.servlet";
-    public static final String CAPABILITY_RESOURCE_TYPE_AT = ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES + ":List<String>";
-    public static final String CAPABILITY_SELECTORS_AT = ServletResolverConstants.SLING_SERVLET_SELECTORS + ":List<String>";
+    public static final String CAPABILITY_RESOURCE_TYPE_AT = ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES;
+    public static final String CAPABILITY_SELECTORS_AT = ServletResolverConstants.SLING_SERVLET_SELECTORS;
     public static final String CAPABILITY_EXTENSIONS_AT = ServletResolverConstants.SLING_SERVLET_EXTENSIONS;
     public static final String CAPABILITY_METHODS_AT = ServletResolverConstants.SLING_SERVLET_METHODS;
     public static final String CAPABILITY_PATH_AT = ServletResolverConstants.SLING_SERVLET_PATHS;
-    public static final String CAPABILITY_VERSION_AT = "version:Version";
+    public static final String CAPABILITY_VERSION_AT = aQute.bnd.osgi.Constants.VERSION_ATTRIBUTE;
     public static final String CAPABILITY_EXTENDS_AT = "extends";
     public static final String CAPABILITY_SCRIPT_ENGINE_AT = "scriptEngine";
     public static final String CAPABILITY_SCRIPT_EXTENSION_AT = "scriptExtension";
diff --git a/src/test/java/org/apache/sling/scriptingbundle/plugin/capability/CapabilityTest.java b/src/test/java/org/apache/sling/scriptingbundle/plugin/capability/CapabilityTest.java
new file mode 100644
index 0000000..bce37c6
--- /dev/null
+++ b/src/test/java/org/apache/sling/scriptingbundle/plugin/capability/CapabilityTest.java
@@ -0,0 +1,64 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ 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.scriptingbundle.plugin.capability;
+
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.osgi.framework.Version;
+import org.osgi.framework.VersionRange;
+
+public class CapabilityTest {
+
+    @Test
+    public void testGetProvidedCapabilitiesString() {
+        Set<ProvidedResourceTypeCapability> resourceTypeCaps = new LinkedHashSet<>();
+        resourceTypeCaps.add(ProvidedResourceTypeCapability.builder()
+                .withResourceTypes("my/type", "/libs/my/type")
+                .withVersion(new Version("2.1.0"))
+                .withRequestExtension("json")
+                .withRequestMethod("POST")
+                .withSelectors("selector1", "selector,2")
+                .build());
+        // TODO: add script capabilities
+        Capabilities caps = new Capabilities(resourceTypeCaps, Collections.emptySet(), Collections.emptySet());
+        String expectedHeaderValue = "sling.servlet;sling.servlet.resourceTypes:List<String>=\"my/type,/libs/my/type\";version:Version=\"2.1.0\";sling.servlet.methods=POST;sling.servlet.extensions=json;sling.servlet.selectors:List<String>=\"selector1,selector\\,2\"";
+        Assert.assertEquals(expectedHeaderValue, caps.getProvidedCapabilitiesString());
+    }
+
+    @Test
+    public void testGetRequiredCapabilitiesString() {
+        Set<RequiredResourceTypeCapability> resourceTypeCaps = new LinkedHashSet<>();
+        resourceTypeCaps.add(RequiredResourceTypeCapability.builder()
+                .withResourceType("my/type")
+                .withVersionRange(new VersionRange("(1.0,3.0)"))
+                .withIsOptional()
+                .build());
+        resourceTypeCaps.add(RequiredResourceTypeCapability.builder()
+                .withResourceType("/other/type")
+                .build());
+        Capabilities caps = new Capabilities(Collections.emptySet(), Collections.emptySet(), resourceTypeCaps);
+        String expectedHeaderValue = "sling.servlet;filter:=\"(&(!(sling.servlet.selectors=*))(&(&(version=*)(!(version<=1.0.0))(!(version>=3.0.0)))(sling.servlet.resourceTypes=my/type)))\";resolution:=optional" + 
+        ",sling.servlet;filter:=\"(&(!(sling.servlet.selectors=*))(sling.servlet.resourceTypes=/other/type))\"";
+        Assert.assertEquals(expectedHeaderValue, caps.getRequiredCapabilitiesString());
+    }
+}