You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2021/05/19 13:23:16 UTC

[sling-org-apache-sling-feature-cpconverter] branch master updated: SLING-10395: ignore empty mappings (#82)

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

pauls pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git


The following commit(s) were added to refs/heads/master by this push:
     new e0a08be  SLING-10395: ignore empty mappings (#82)
e0a08be is described below

commit e0a08be96d18c50bc0d556b66ec577a7f3a2334d
Author: Karl Pauls <pa...@apache.org>
AuthorDate: Wed May 19 15:22:37 2021 +0200

    SLING-10395: ignore empty mappings (#82)
    
    
    Co-authored-by: angela <an...@adobe.com>
---
 .../features/DefaultFeaturesManager.java           | 16 +++++++--
 .../accesscontrol/EnforcePrincipalBasedTest.java   | 38 ++++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java b/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java
index c75c7f8..29452ba 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/features/DefaultFeaturesManager.java
@@ -325,9 +325,19 @@ public class DefaultFeaturesManager implements FeaturesManager, PackagesEventsEm
             if (mappings != null) {
                 List<String> newMappings = new ArrayList<>();
                 for (String usermapping : mappings) {
-                    Mapping mapping = new Mapping(usermapping, enforceServiceMappingByPrincipal);
-                    getAclManager().addMapping(mapping);
-                    newMappings.add(mapping.asString());
+                    if (usermapping == null || usermapping.trim().isEmpty()) {
+                        logger.warn("ServiceUserMapping: Ignoring empty mapping in {}", pid);
+                        // invalid empty mapping => ignore
+                        continue;
+                    }
+                    try {
+                        Mapping mapping = new Mapping(usermapping, enforceServiceMappingByPrincipal);
+                        getAclManager().addMapping(mapping);
+                        newMappings.add(mapping.asString());
+                    } catch (IllegalArgumentException iae) {
+                        logger.error("ServiceUserMapping: Detected invalid mapping in {}", pid);
+                        throw iae;
+                    }
                 }
                 // replace 'user.mapping' property by the new mappings, which may have been refactored
                 if (!newMappings.isEmpty()) {
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
index 62ffcdd..5ca170a 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
@@ -40,10 +40,12 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.List;
 
 import static org.apache.sling.feature.cpconverter.Util.normalize;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -274,6 +276,42 @@ public class EnforcePrincipalBasedTest {
         assertEquals(expected, actual);
     }
 
+    @Test
+    public void testWhitespaceUserMapping() throws IOException {
+        DefaultFeaturesManager fm = new DefaultFeaturesManager(true, 1, File.createTempFile("foo", "bar"), "*", "*", new HashMap<>(), aclManager);
+        Feature seed = new Feature(ArtifactId.fromMvnId("org:foo:2"));
+        
+        // create a user.mapping configuratian with an empty-mapping
+        Configuration foo = new Configuration("org.apache.sling.serviceusermapping.impl.ServiceUserMapperImpl~foo");
+        Dictionary<String, Object> props = foo.getProperties();
+        props.put("user.mapping", new String[]{"serviceName:subservice=[user1]","     ","serviceName2:subservice2=[user2]"});
+        seed.getConfigurations().add(foo);
+
+        fm.init("groupId", "artifactId", "version1.0");
+        fm.addConfiguration("author", foo.getPid(), "/path", props);
+        
+        // verify that invalid empty mapping has been stripped (without Exception)
+        String[] result = (String[]) foo.getProperties().get("user.mapping");
+        assertArrayEquals(new String[]{"serviceName:subservice=[user1]","serviceName2:subservice2=[user2]"}, result);
+    }
+
+    @Test
+    public void testEmptyUserMapping() throws IOException {
+        DefaultFeaturesManager fm = new DefaultFeaturesManager(true, 1, File.createTempFile("foo", "bar"), "*", "*", new HashMap<>(), aclManager);
+
+        // create a user.mapping configuratian with an empty-mapping
+        Configuration foo = new Configuration("org.apache.sling.serviceusermapping.impl.ServiceUserMapperImpl~foo");
+        Dictionary<String, Object> props = foo.getProperties();
+        props.put("user.mapping", new String[]{"serviceName:subservice=[user1]","","serviceName2:subservice2=[user2]"});
+
+        fm.init("groupId", "artifactId", "version1.0");
+        fm.addConfiguration("author", foo.getPid(), "/path", props);
+
+        // verify that invalid empty mapping has been stripped (without Exception)
+        String[] result = (String[]) foo.getProperties().get("user.mapping");
+        assertArrayEquals(new String[]{"serviceName:subservice=[user1]","serviceName2:subservice2=[user2]"}, result);
+    }
+
     @NotNull
     private Extension getRepoInitExtension(@NotNull AclManager aclManager, @NotNull RepoPath accessControlledPath, @NotNull SystemUser systemUser, boolean isPrincipalBased) throws RepoInitParsingException {
         aclManager.addSystemUser(systemUser);