You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2021/06/29 09:24:30 UTC

[jackrabbit-filevault] 01/01: JCRVLT-543 ignore protected properties during import

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

kwin pushed a commit to branch bugfix/JCRVLT-543-don't-fail-on-protected-properties
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git

commit 4fe58c11ab00f68d4440c5484fdff4fba128df3c
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Tue Jun 29 11:24:12 2021 +0200

    JCRVLT-543 ignore protected properties during import
    
    dynamically determine if a property is protected
    fail with an exception for all other constraint violations (in mode =
    REPLACE)
---
 .../vault/fs/impl/io/DocViewSAXImporter.java       | 135 ++++++---
 .../packaging/integration/EmptyPackageIT.java      |  14 +-
 .../vault/packaging/integration/ImportIT.java      | 318 +++++++++++----------
 .../packaging/integration/IntegrationTestBase.java |   1 +
 .../packaging/integration/PackageInstallIT.java    |   8 +-
 .../META-INF/vault/filter.xml                      |   4 +
 .../META-INF/vault/properties.xml                  |  18 ++
 .../protected_properties.zip/jcr_root/.content.xml |  17 ++
 .../jcr_root/testroot/.content.xml                 |   7 +
 9 files changed, 334 insertions(+), 188 deletions(-)

diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
index 5799751..56eee3d 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
@@ -20,6 +20,7 @@ package org.apache.jackrabbit.vault.fs.impl.io;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collections;
 import java.util.HashMap;
@@ -28,6 +29,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import javax.jcr.ImportUUIDBehavior;
 import javax.jcr.Item;
@@ -43,13 +46,9 @@ import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.Value;
 import javax.jcr.ValueFactory;
-import javax.jcr.ValueFormatException;
-import javax.jcr.lock.LockException;
 import javax.jcr.nodetype.ConstraintViolationException;
-import javax.jcr.nodetype.NoSuchNodeTypeException;
 import javax.jcr.nodetype.NodeType;
-import javax.jcr.version.VersionException;
-
+import javax.jcr.nodetype.PropertyDefinition;
 import org.apache.jackrabbit.spi.Name;
 import org.apache.jackrabbit.spi.commons.conversion.DefaultNamePathResolver;
 import org.apache.jackrabbit.spi.commons.name.NameConstants;
@@ -124,12 +123,13 @@ public class DocViewSAXImporter extends RejectingEntityDefaultHandler implements
      */
     static final Attributes EMPTY_ATTRIBUTES = new AttributesImpl();
 
+    /**
+     * these properties are protected but can be set nevertheless via system view xml import
+     */
     static final Set<String> PROTECTED_PROPERTIES;
 
-    static final Set<String> IGNORED_PROPERTIES;
-
     static {
-        Set<String> props = new HashSet<String>();
+        Set<String> props = new HashSet<>();
         props.add(JcrConstants.JCR_PRIMARYTYPE);
         props.add(JcrConstants.JCR_MIXINTYPES);
         props.add(JcrConstants.JCR_UUID);
@@ -138,16 +138,9 @@ public class DocViewSAXImporter extends RejectingEntityDefaultHandler implements
         props.add(JcrConstants.JCR_PREDECESSORS);
         props.add(JcrConstants.JCR_SUCCESSORS);
         props.add(JcrConstants.JCR_VERSIONHISTORY);
-        props.add(PROP_OAK_COUNTER);
         PROTECTED_PROPERTIES = Collections.unmodifiableSet(props);
     }
 
-    static {
-        Set<String> props = new HashSet<String>();
-        props.add(PROP_OAK_COUNTER);
-        IGNORED_PROPERTIES = Collections.unmodifiableSet(props);
-    }
-
     /**
      * the current namespace state
      */
@@ -193,7 +186,7 @@ public class DocViewSAXImporter extends RejectingEntityDefaultHandler implements
      * a map of binaries (attachments)
      */
     private Map<String, Map<String, DocViewSAXImporter.BlobInfo>> binaries
-            = new HashMap<String, Map<String, DocViewSAXImporter.BlobInfo>>();
+            = new HashMap<>();
 
     /**
      * map of hint nodes in the same artifact set
@@ -991,23 +984,19 @@ public class DocViewSAXImporter extends RejectingEntityDefaultHandler implements
             modified = true;
         }
 
-        // remove properties not in package
+        // remove properties not in package (which are not protected)
         if (importMode == ImportMode.REPLACE) {
             PropertyIterator pIter = node.getProperties();
             while (pIter.hasNext()) {
                 Property p = pIter.nextProperty();
                 String propName = p.getName();
-                if (!PROTECTED_PROPERTIES.contains(propName)
+                if (!p.getDefinition().isProtected()
                         && !ni.props.containsKey(propName)
                         && !preserveProperties.contains(p.getPath())
                         && wspFilter.includesProperty(p.getPath())) {
-                    try {
-                        vs.ensureCheckedOut();
-                        p.remove();
-                        modified = true;
-                    } catch (RepositoryException e) {
-                        // ignore
-                    }
+                    vs.ensureCheckedOut();
+                    p.remove();
+                    modified = true;
                 }
             }
         }
@@ -1016,7 +1005,8 @@ public class DocViewSAXImporter extends RejectingEntityDefaultHandler implements
         return modified;
     }
     /**
-     * Creates a new node via system view XML and {@link Session#importXML(String, InputStream, int)} to be able to set protected properties as well
+     * Creates a new node via system view XML and {@link Session#importXML(String, InputStream, int)} to be able to set protected properties. 
+     * Afterwards uses regular JCR API to set unprotected properties.
      * @param currentNode
      * @param ni
      * @return
@@ -1071,7 +1061,7 @@ public class DocViewSAXImporter extends RejectingEntityDefaultHandler implements
             for (DocViewProperty p : ni.props.values()) {
                 if (p != null && p.values != null) {
                     // only pass 'protected' properties to the import
-                    if (PROTECTED_PROPERTIES.contains(p.name) && !IGNORED_PROPERTIES.contains(p.name)) {
+                    if (PROTECTED_PROPERTIES.contains(p.name)) {
                         attrs = new AttributesImpl();
                         attrs.addAttribute(Name.NS_SV_URI, "name", "sv:name", "CDATA", p.name);
                         attrs.addAttribute(Name.NS_SV_URI, "type", "sv:type", "CDATA", PropertyType.nameFromValue(p.type));
@@ -1118,6 +1108,81 @@ public class DocViewSAXImporter extends RejectingEntityDefaultHandler implements
         }
     }
 
+    /**
+     * Determines if a given property is protected according to the node type.
+     * <br>
+     * Using the EffectiveNodeType would be faster and require less code, but this is not exposed via JCR API (but only via SPI and differs between Jackrabbit 2 and Oak).
+     * Also {@link NodeType#canSetProperty(String, Value)} cannot be used as this would return false not only for protected property but also for other constraint violations.
+     * This is functionally equivalent to <a href="https://github.com/apache/jackrabbit/blob/ed3124e5fe223dada33ce6ddf53bc666063c3f2f/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/nodetype/EffectiveNodeType.java#L764">EffectiveNodeType.getApplicablePropertyDef(...)</a>.
+     * 
+     * @param node the node
+     * @param docViewProperty the property
+     * @return{@code true} in case the property is protected, {@code false} otherwise
+     * @throws RepositoryException 
+     */
+    private static boolean isPropertyProtected(@NotNull Node node, @NotNull DocViewProperty docViewProperty) throws RepositoryException {
+        // first named property definitions
+        PropertyDefinition propDef = getApplicablePropertyDefinition(node, docViewProperty, true);
+        if (propDef == null) {
+            // then residual property definitions
+            propDef = getApplicablePropertyDefinition(node, docViewProperty, false);
+        }
+        if (propDef != null) {
+            return propDef.isProtected();
+        }
+        return false;
+    }
+
+    private static @Nullable PropertyDefinition getApplicablePropertyDefinition(@NotNull Node node, @NotNull DocViewProperty docViewProperty, boolean onlyNamedDefinitions) throws RepositoryException {
+        PropertyDefinition propDef = getMatchingPropertyDefinition(node.getPrimaryNodeType(), docViewProperty, onlyNamedDefinitions);
+        if (propDef != null) {
+            return propDef;
+        }
+        for (NodeType mixinNodeType : node.getMixinNodeTypes()) {
+            propDef = getMatchingPropertyDefinition(mixinNodeType, docViewProperty, onlyNamedDefinitions);
+            if (propDef != null) {
+                return propDef;
+            }
+        }
+        return null;
+    }
+
+    // functionally almost equivalent to https://github.com/apache/jackrabbit/blob/ed3124e5fe223dada33ce6ddf53bc666063c3f2f/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/nodetype/EffectiveNodeType.java#L825
+    private static PropertyDefinition getMatchingPropertyDefinition(@NotNull NodeType nodeType, @NotNull DocViewProperty docViewProperty, boolean onlyNamedDefinitions) {
+        
+        Predicate<PropertyDefinition> predicate;
+        if (onlyNamedDefinitions) {
+            predicate = (pd) -> docViewProperty.name.equals(pd.getName());
+        } else {
+            predicate = (pd) -> "*".equals(pd.getName());
+        }
+        // either named or residual property definitions
+        Set<PropertyDefinition> relevantPropertyDefinitions = Arrays.stream(nodeType.getPropertyDefinitions()).filter(predicate).collect(Collectors.toSet());
+        
+        PropertyDefinition match = null;
+        for (PropertyDefinition pd : relevantPropertyDefinitions) {
+            int reqType = pd.getRequiredType();
+            // match type
+            if (reqType == PropertyType.UNDEFINED
+                    || docViewProperty.type == PropertyType.UNDEFINED
+                    || reqType == docViewProperty.type) {
+                // match multiValued flag
+                if (docViewProperty.isMulti == pd.isMultiple()) {
+                    // found match
+                    if (pd.getRequiredType() != PropertyType.UNDEFINED) {
+                        // found best possible match, get outta here
+                        return pd;
+                    } else {
+                        if (match == null) {
+                            match = pd;
+                        }
+                    }
+                }
+            }
+        }
+        return match;
+    }
+
     private Node getNodeByUUIDLabelOrName(@NotNull Node currentNode, @NotNull DocViewNode ni) throws RepositoryException {
         Node node = null;
         if (ni.uuid != null) {
@@ -1157,21 +1222,25 @@ public class DocViewSAXImporter extends RejectingEntityDefaultHandler implements
         boolean modified = false;
         // add properties
         for (DocViewProperty prop : ni.props.values()) {
-            if (prop != null && !PROTECTED_PROPERTIES.contains(prop.name) && (overwriteExistingProperties || !node.hasProperty(prop.name)) && wspFilter.includesProperty(node.getPath() + "/" + prop.name)) {
+            if (prop != null && !isPropertyProtected(node, prop) && (overwriteExistingProperties || !node.hasProperty(prop.name)) && wspFilter.includesProperty(node.getPath() + "/" + prop.name)) {
                 // check if property is allowed
                 try {
                     modified |= prop.apply(node);
                 } catch (RepositoryException e) {
-                    if (vs == null) {
-                        throw e;
-                    }
                     try {
+                        if (vs == null) {
+                            throw e;
+                        }
                         // try again with checked out node
                         vs.ensureCheckedOut();
                         modified |= prop.apply(node);
                     } catch (RepositoryException e1) {
-                        log.warn("Error while setting property (ignore): " + e1);
-                        continue;
+                        // be lenient in case of mode != replace
+                        if (wspFilter.getImportMode(node.getPath()) != ImportMode.REPLACE) {
+                            log.warn("Error while setting property {} (ignore due to mode {}): {}", prop.name, wspFilter.getImportMode(node.getPath()), e1);
+                        } else {
+                            throw e;
+                        }
                     }
                 }
             }
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/EmptyPackageIT.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/EmptyPackageIT.java
index 734ce32..2c5545d 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/EmptyPackageIT.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/EmptyPackageIT.java
@@ -26,6 +26,7 @@ import javax.jcr.RepositoryException;
 import javax.jcr.nodetype.NodeType;
 
 import org.apache.jackrabbit.commons.JcrUtils;
+import org.apache.jackrabbit.vault.fs.io.ImportOptions;
 import org.apache.jackrabbit.vault.packaging.JcrPackage;
 import org.apache.jackrabbit.vault.packaging.PackageException;
 import org.junit.Test;
@@ -208,10 +209,12 @@ public class EmptyPackageIT extends IntegrationTestBase {
     public void installInstallNoFilter() throws RepositoryException, IOException, PackageException {
         JcrPackage pack = packMgr.upload(getStream("/test-packages/tmp_foo_bar_test_nofilter.zip"), false);
         assertNotNull(pack);
-        pack.install(getDefaultOptions());
+        ImportOptions opts = getDefaultOptions();
+        opts.setStrict(false);
+        pack.install(opts);
         assertNodeExists("/tmp/foo/bar/test.txt");
 
-        pack.uninstall(getDefaultOptions());
+        pack.uninstall(opts);
         assertNodeExists("/tmp/foo/bar/test.txt");
     }
 
@@ -223,10 +226,13 @@ public class EmptyPackageIT extends IntegrationTestBase {
     public void installInstallMinimal() throws RepositoryException, IOException, PackageException {
         JcrPackage pack = packMgr.upload(getStream("/test-packages/tmp_foo_bar_test_minimal.zip"), false);
         assertNotNull(pack);
-        pack.install(getDefaultOptions());
+        ImportOptions opts = getDefaultOptions();
+        opts.setStrict(false);
+        pack.install(opts);
+        
         assertNodeExists("/tmp/foo/bar/test.txt");
 
-        pack.uninstall(getDefaultOptions());
+        pack.uninstall(opts);
         assertNodeExists("/tmp/foo/bar/test.txt");
     }
 
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java
index 60e922a..fb60e37 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java
@@ -65,88 +65,93 @@ public class ImportIT extends IntegrationTestBase {
 
     @Test
     public void testImport() throws IOException, RepositoryException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/tmp.zip"));
-        archive.open(true);
-        Node rootNode = admin.getRootNode();
-        ImportOptions opts = getDefaultOptions();
-        Importer importer = new Importer(opts);
-        importer.run(archive, rootNode);
-
-        assertNodeExists("/tmp/foo/bar/tobi");
+        try (Archive archive = getFileArchive("/test-packages/tmp.zip")) {
+            archive.open(true);
+            Node rootNode = admin.getRootNode();
+            ImportOptions opts = getDefaultOptions();
+            Importer importer = new Importer(opts);
+            importer.run(archive, rootNode);
+
+            assertNodeExists("/tmp/foo/bar/tobi");
+        }
     }
 
     @Test
     public void testReimportLess() throws IOException, RepositoryException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/tmp.zip"));
-        archive.open(true);
         Node rootNode = admin.getRootNode();
         ImportOptions opts = getDefaultOptions();
         Importer importer = new Importer(opts);
-        importer.run(archive, rootNode);
+        try (Archive archive = getFileArchive("/test-packages/tmp.zip")) {
+            archive.open(true);
+            importer.run(archive, rootNode);
 
-        assertNodeExists("/tmp/foo/bar/tobi");
-
-        ZipArchive archive2 = new ZipArchive(getFile("/test-packages/tmp_less.zip"));
-        archive2.open(true);
-        importer.run(archive2, rootNode);
+            assertNodeExists("/tmp/foo/bar/tobi");
+        }
+        
+        try (Archive archive = getFileArchive("/test-packages/tmp_less.zip")) {
+            archive.open(true);
+            importer.run(archive, rootNode);
 
-        assertNodeMissing("/tmp/foo/bar/tobi");
+            assertNodeMissing("/tmp/foo/bar/tobi");
+        }
     }
 
     @Test
     public void testFilteredImport() throws IOException, RepositoryException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/filtered_package.zip"));
-        archive.open(true);
-        Node rootNode = admin.getRootNode();
-        ImportOptions opts = getDefaultOptions();
-
-        Importer importer = new Importer(opts);
-        importer.run(archive, rootNode);
-
-        assertNodeExists("/tmp");
-        assertNodeExists("/tmp/foo");
-        assertNodeExists("/tmp/foo/bar");
-        assertNodeExists("/tmp/foo/bar/tobi");
-        assertNodeMissing("/tmp/foo/bar/tom");
+        try (Archive archive = getFileArchive("/test-packages/filtered_package.zip")) {
+            archive.open(true);
+            Node rootNode = admin.getRootNode();
+            ImportOptions opts = getDefaultOptions();
+
+            Importer importer = new Importer(opts);
+            importer.run(archive, rootNode);
+
+            assertNodeExists("/tmp");
+            assertNodeExists("/tmp/foo");
+            assertNodeExists("/tmp/foo/bar");
+            assertNodeExists("/tmp/foo/bar/tobi");
+            assertNodeMissing("/tmp/foo/bar/tom");
+        }
     }
 
     @Test
     public void testUnFilteredImport() throws IOException, RepositoryException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/unfiltered_package.zip"));
-        archive.open(true);
-        Node rootNode = admin.getRootNode();
-        ImportOptions opts = getDefaultOptions();
-
-        Importer importer = new Importer(opts);
-        importer.run(archive, rootNode);
-
-        assertNodeExists("/tmp");
-        assertNodeExists("/tmp/foo");
-        assertNodeExists("/tmp/foo/bar");
-        assertNodeExists("/tmp/foo/bar/tobi");
-        assertNodeExists("/tmp/foo/bar/tom");
+        try (Archive archive = getFileArchive("/test-packages/unfiltered_package.zip")) {
+            archive.open(true);
+            Node rootNode = admin.getRootNode();
+            ImportOptions opts = getDefaultOptions();
+
+            Importer importer = new Importer(opts);
+            importer.run(archive, rootNode);
+
+            assertNodeExists("/tmp");
+            assertNodeExists("/tmp/foo");
+            assertNodeExists("/tmp/foo/bar");
+            assertNodeExists("/tmp/foo/bar/tobi");
+            assertNodeExists("/tmp/foo/bar/tom");
+        }
     }
 
     @Test
     public void testRelativeImport() throws IOException, RepositoryException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/tmp.zip"));
-
-        admin.getRootNode().addNode(TEST_ROOT.substring(1, TEST_ROOT.length()));
-        admin.save();
-
-        archive.open(true);
-        Node rootNode = admin.getNode(TEST_ROOT);
-        ImportOptions opts = getDefaultOptions();
-        // manually creating filterPaths with correct coverage
-        WorkspaceFilter filter = archive.getMetaInf().getFilter();
-        for (PathFilterSet pathFilterSet : filter.getFilterSets()) {
-            pathFilterSet.setRoot(TEST_ROOT + pathFilterSet.getRoot());
+        try (Archive archive = getFileArchive("/test-packages/tmp.zip")) {
+            admin.getRootNode().addNode(TEST_ROOT.substring(1, TEST_ROOT.length()));
+            admin.save();
+
+            archive.open(true);
+            Node rootNode = admin.getNode(TEST_ROOT);
+            ImportOptions opts = getDefaultOptions();
+            // manually creating filterPaths with correct coverage
+            WorkspaceFilter filter = archive.getMetaInf().getFilter();
+            for (PathFilterSet pathFilterSet : filter.getFilterSets()) {
+                pathFilterSet.setRoot(TEST_ROOT + pathFilterSet.getRoot());
+            }
+            opts.setFilter(filter);
+            Importer importer = new Importer(opts);
+            importer.run(archive, rootNode);
+
+            assertNodeExists(TEST_ROOT + "/tmp/foo/bar/tobi");
         }
-        opts.setFilter(filter);
-        Importer importer = new Importer(opts);
-        importer.run(archive, rootNode);
-
-        assertNodeExists(TEST_ROOT + "/tmp/foo/bar/tobi");
     }
 
     /**
@@ -155,18 +160,18 @@ public class ImportIT extends IntegrationTestBase {
      */
     @Test
     public void testRelativeEmptyImport() throws IOException, RepositoryException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/empty_testnode.zip"));
-
-        admin.getRootNode().addNode(TEST_ROOT.substring(1, TEST_ROOT.length()));
-        admin.save();
+        try (Archive archive = getFileArchive("/test-packages/empty_testnode.zip")) {
+            admin.getRootNode().addNode(TEST_ROOT.substring(1, TEST_ROOT.length()));
+            admin.save();
 
-        archive.open(true);
-        Node rootNode = admin.getNode(TEST_ROOT);
-        ImportOptions opts = getDefaultOptions();
-        Importer importer = new Importer(opts);
-        importer.run(archive, rootNode);
+            archive.open(true);
+            Node rootNode = admin.getNode(TEST_ROOT);
+            ImportOptions opts = getDefaultOptions();
+            Importer importer = new Importer(opts);
+            importer.run(archive, rootNode);
 
-        assertNodeExists(TEST_ROOT);
+            assertNodeExists(TEST_ROOT);
+        }
     }
 
     /**
@@ -180,99 +185,100 @@ public class ImportIT extends IntegrationTestBase {
         createNodes(archiveNode, 2, 4);
         admin.save();
         assertNodeExists(ARCHIVE_ROOT + "/n3/n3/n3");
-        JcrArchive archive = new JcrArchive(archiveNode, TEST_ROOT);
-
-        Node testRoot = admin.getRootNode().addNode(TEST_ROOT.substring(1, TEST_ROOT.length()));
-        testRoot.addNode("dummy", "nt:folder");
-        admin.save();
-
-        archive.open(true);
-        Node rootNode = admin.getNode(TEST_ROOT);
-        ImportOptions opts = getDefaultOptions();
-        //opts.setListener(new DefaultProgressListener());
-        Importer importer = new Importer(opts);
-        importer.run(archive, rootNode);
-        admin.save();
-
-        assertNodeExists(TEST_ROOT + "/n3/n3/n3");
-        assertNodeMissing(TEST_ROOT + "dummy");
+        try (JcrArchive archive = new JcrArchive(archiveNode, TEST_ROOT)) {
+            Node testRoot = admin.getRootNode().addNode(TEST_ROOT.substring(1, TEST_ROOT.length()));
+            testRoot.addNode("dummy", "nt:folder");
+            admin.save();
+
+            archive.open(true);
+            Node rootNode = admin.getNode(TEST_ROOT);
+            ImportOptions opts = getDefaultOptions();
+            //opts.setListener(new DefaultProgressListener());
+            Importer importer = new Importer(opts);
+            importer.run(archive, rootNode);
+            admin.save();
+
+            assertNodeExists(TEST_ROOT + "/n3/n3/n3");
+            assertNodeMissing(TEST_ROOT + "dummy");
+        }
     }
 
     @Test
     public void testConcurrentModificationHandling() throws IOException, RepositoryException, PackageException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/tags.zip"));
-        archive.open(true);
-        Node rootNode = admin.getRootNode();
-        ImportOptions opts = getDefaultOptions();
-        opts.setAutoSaveThreshold(7);
-        Importer importer = new Importer(opts);
-        importer.setDebugFailAfterSave(2);
-        importer.run(archive, rootNode);
-        admin.save();
-
-        // count nodes
-        assertNodeExists("/etc/tags");
-        Node tags = admin.getNode("/etc/tags");
-        int numNodes = countNodes(tags);
-        assertEquals("Number of tags installed", 487, numNodes);
-
+        try (Archive archive = getFileArchive("/test-packages/tags.zip")) {
+            archive.open(true);
+            Node rootNode = admin.getRootNode();
+            ImportOptions opts = getDefaultOptions();
+            opts.setAutoSaveThreshold(7);
+            Importer importer = new Importer(opts);
+            importer.setDebugFailAfterSave(2);
+            importer.run(archive, rootNode);
+            admin.save();
+
+            // count nodes
+            assertNodeExists("/etc/tags");
+            Node tags = admin.getNode("/etc/tags");
+            int numNodes = countNodes(tags);
+            assertEquals("Number of tags installed", 487, numNodes);
+        }
     }
 
     @Test
     public void testSNSImport() throws IOException, RepositoryException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/test_sns.zip"));
-        archive.open(true);
-        Node rootNode = admin.getRootNode();
-        ImportOptions opts = getDefaultOptions();
-        Importer importer = new Importer(opts);
-        importer.run(archive, rootNode);
-
-        assertNodeExists("/tmp/testroot");
-        assertNodeExists("/tmp/testroot/foo");
-        assertProperty("/tmp/testroot/foo/name", "foo1");
-
-        // only check for SNS nodes if SNS supported
-        if (admin.getRepository().getDescriptorValue(Repository.NODE_TYPE_MANAGEMENT_SAME_NAME_SIBLINGS_SUPPORTED).getBoolean()) {
-            assertNodeExists("/tmp/testroot/foo[2]");
-            assertNodeExists("/tmp/testroot/foo[3]");
-            assertProperty("/tmp/testroot/foo[2]/name", "foo2");
-            assertProperty("/tmp/testroot/foo[3]/name", "foo3");
-        } else {
-            // otherwise nodes must not exist
-            assertNodeMissing("/tmp/testroot/foo[2]");
-            assertNodeMissing("/tmp/testroot/foo[3]");
+        try (Archive archive = getFileArchive("/test-packages/test_sns.zip")) {
+            archive.open(true);
+            Node rootNode = admin.getRootNode();
+            ImportOptions opts = getDefaultOptions();
+            Importer importer = new Importer(opts);
+            importer.run(archive, rootNode);
+
+            assertNodeExists("/tmp/testroot");
+            assertNodeExists("/tmp/testroot/foo");
+            assertProperty("/tmp/testroot/foo/name", "foo1");
+
+            // only check for SNS nodes if SNS supported
+            if (admin.getRepository().getDescriptorValue(Repository.NODE_TYPE_MANAGEMENT_SAME_NAME_SIBLINGS_SUPPORTED).getBoolean()) {
+                assertNodeExists("/tmp/testroot/foo[2]");
+                assertNodeExists("/tmp/testroot/foo[3]");
+                assertProperty("/tmp/testroot/foo[2]/name", "foo2");
+                assertProperty("/tmp/testroot/foo[3]/name", "foo3");
+            } else {
+                // otherwise nodes must not exist
+                assertNodeMissing("/tmp/testroot/foo[2]");
+                assertNodeMissing("/tmp/testroot/foo[3]");
+            }
         }
-
     }
 
 
     @Test
     public void testSubArchiveExtract() throws IOException, RepositoryException, ConfigurationException {
-        ZipArchive archive = new ZipArchive(getFile("/test-packages/tmp_with_thumbnail.zip"));
-        archive.open(true);
-        Node rootNode = admin.getRootNode();
-        Node tmpNode = rootNode.addNode("tmp");
-        Node fileNode = tmpNode.addNode("package.zip", "nt:file");
-        Node contentNode = fileNode.addNode("jcr:content", "nt:resource");
-        contentNode.setProperty("jcr:data", "");
-        contentNode.setProperty("jcr:lastModified", 0);
-        contentNode.addMixin("vlt:Package");
-        Node defNode = contentNode.addNode("vlt:definition", "vlt:PackageDefinition");
-
-        ImportOptions opts = getDefaultOptions();
-        Archive subArchive =  archive.getSubArchive("META-INF/vault/definition", true);
-
-        DefaultWorkspaceFilter filter = new DefaultWorkspaceFilter();
-        filter.add(new PathFilterSet(defNode.getPath()));
-
-        Importer importer = new Importer(opts);
-        importer.getOptions().setAutoSaveThreshold(Integer.MAX_VALUE);
-        importer.getOptions().setFilter(filter);
-        importer.run(subArchive, defNode);
-        admin.save();
-
-        assertFalse("Importer must not have any errors", importer.hasErrors());
-        assertNodeExists("/tmp/package.zip/jcr:content/vlt:definition/thumbnail.png");
+        try (Archive archive = getFileArchive("/test-packages/tmp_with_thumbnail.zip")) {
+            archive.open(true);
+            Node rootNode = admin.getRootNode();
+            Node tmpNode = rootNode.addNode("tmp");
+            Node fileNode = tmpNode.addNode("package.zip", "nt:file");
+            Node contentNode = fileNode.addNode("jcr:content", "nt:resource");
+            contentNode.setProperty("jcr:data", "");
+            contentNode.setProperty("jcr:lastModified", 0);
+            contentNode.addMixin("vlt:Package");
+            Node defNode = contentNode.addNode("vlt:definition", "vlt:PackageDefinition");
+
+            ImportOptions opts = getDefaultOptions();
+            Archive subArchive =  archive.getSubArchive("META-INF/vault/definition", true);
+
+            DefaultWorkspaceFilter filter = new DefaultWorkspaceFilter();
+            filter.add(new PathFilterSet(defNode.getPath()));
+
+            Importer importer = new Importer(opts);
+            importer.getOptions().setAutoSaveThreshold(Integer.MAX_VALUE);
+            importer.getOptions().setFilter(filter);
+            importer.run(subArchive, defNode);
+            admin.save();
+            
+            assertFalse("Importer must not have any errors", importer.hasErrors());
+            assertNodeExists("/tmp/package.zip/jcr:content/vlt:definition/thumbnail.png");
+        }
     }
 
     @Test
@@ -302,6 +308,7 @@ public class ImportIT extends IntegrationTestBase {
         ZipArchive archive = new ZipArchive(getFile("/test-packages/tmp.zip"));
         archive.open(true);
         ImportOptions opts = getDefaultOptions();
+        opts.setStrict(false);
         Importer importer = new Importer(opts);
         importer.run(archive, session, "/");
         session.logout();
@@ -336,6 +343,7 @@ public class ImportIT extends IntegrationTestBase {
         ZipArchive archive = new ZipArchive(getFile("/test-packages/tmp_foo.zip"));
         archive.open(true);
         ImportOptions opts = getDefaultOptions();
+        opts.setStrict(false);
         Importer importer = new Importer(opts);
         importer.run(archive, session, "/");
         session.logout();
@@ -343,4 +351,16 @@ public class ImportIT extends IntegrationTestBase {
         assertNodeExists("/tmp/foo/bar/tobi");
     }
 
+    @Test
+    public void testImportProtectedProperties() throws IOException, RepositoryException, ConfigurationException {
+        try (Archive archive = getFileArchive("/test-packages/protected_properties.zip")) {
+            Node rootNode = admin.getRootNode();
+            ImportOptions opts = getDefaultOptions();
+            Importer importer = new Importer(opts);
+            archive.open(true);
+            importer.run(archive, rootNode);
+        }
+        admin.save();
+        assertProperty("/testroot/jcr:createdBy", "admin");
+    }
 }
\ No newline at end of file
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/IntegrationTestBase.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/IntegrationTestBase.java
index 17e2804..734498c 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/IntegrationTestBase.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/IntegrationTestBase.java
@@ -459,6 +459,7 @@ public class IntegrationTestBase  {
     public ImportOptions getDefaultOptions() {
         ImportOptions opts = new ImportOptions();
         opts.setListener(getLoggingProgressTrackerListener());
+        opts.setStrict(true);
         return opts;
     }
 
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/PackageInstallIT.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/PackageInstallIT.java
index 3282d71..1f1e6d0 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/PackageInstallIT.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/PackageInstallIT.java
@@ -500,11 +500,14 @@ public class PackageInstallIT extends IntegrationTestBase {
         assertNotNull(pack);
 
         // extract should not generate snapshots
-        pack.extract(getDefaultOptions());
+        ImportOptions opts = getDefaultOptions();
+        opts.setStrict(false);
+        
+        pack.extract(opts);
         assertNodeExists("/tmp/foo/bar/tobi");
         assertPackageNodeMissing(TMP_SNAPSHOT_PACKAGE_ID);
 
-        pack.uninstall(getDefaultOptions());
+        pack.uninstall(opts);
         assertNodeExists("/tmp/foo/bar/tobi");
     }
 
@@ -813,6 +816,7 @@ public class PackageInstallIT extends IntegrationTestBase {
         JcrPackageManagerImpl userPackMgr = new JcrPackageManagerImpl(session, new String[0]);
         pack = userPackMgr.open(id);
         ImportOptions opts = getDefaultOptions();
+        opts.setStrict(false);
         pack.extract(opts);
         pack.close();
         session.logout();
diff --git a/vault-core/src/test/resources/test-packages/protected_properties.zip/META-INF/vault/filter.xml b/vault-core/src/test/resources/test-packages/protected_properties.zip/META-INF/vault/filter.xml
new file mode 100644
index 0000000..bbbd616
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/protected_properties.zip/META-INF/vault/filter.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<workspaceFilter version="1.0">
+    <filter root="/testroot"/>
+</workspaceFilter>
diff --git a/vault-core/src/test/resources/test-packages/protected_properties.zip/META-INF/vault/properties.xml b/vault-core/src/test/resources/test-packages/protected_properties.zip/META-INF/vault/properties.xml
new file mode 100644
index 0000000..c606e19
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/protected_properties.zip/META-INF/vault/properties.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8" standalone="no"?>
+<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
+<properties>
+<comment>FileVault Package Properties</comment>
+<entry key="createdBy">admin</entry>
+<entry key="name">import-modes-test</entry>
+<entry key="lastModified">2011-11-15T09:45:14.664+01:00</entry>
+<entry key="lastModifiedBy">admin</entry>
+<entry key="created">2011-11-15T09:45:14.685+01:00</entry>
+<entry key="buildCount">1</entry>
+<entry key="version"/>
+<entry key="dependencies"/>
+<entry key="packageFormatVersion">2</entry>
+<entry key="description"/>
+<entry key="lastWrapped">2011-11-15T09:45:14.664+01:00</entry>
+<entry key="group"/>
+<entry key="lastWrappedBy">admin</entry>
+</properties>
diff --git a/vault-core/src/test/resources/test-packages/protected_properties.zip/jcr_root/.content.xml b/vault-core/src/test/resources/test-packages/protected_properties.zip/jcr_root/.content.xml
new file mode 100644
index 0000000..e546f29
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/protected_properties.zip/jcr_root/.content.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<jcr:root xmlns:sling="http://sling.apache.org/jcr/sling/1.0" xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal"
+    jcr:mixinTypes="[rep:AccessControllable]"
+    jcr:primaryType="rep:root"
+    sling:resourceType="sling:redirect"
+    sling:target="/index.html">
+    <rep:policy/>
+    <jcr:system/>
+    <var/>
+    <libs/>
+    <etc/>
+    <apps/>
+    <content/>
+    <tmp/>
+    <home/>
+    <testroot/>
+</jcr:root>
\ No newline at end of file
diff --git a/vault-core/src/test/resources/test-packages/protected_properties.zip/jcr_root/testroot/.content.xml b/vault-core/src/test/resources/test-packages/protected_properties.zip/jcr_root/testroot/.content.xml
new file mode 100644
index 0000000..4e7b5f9
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/protected_properties.zip/jcr_root/testroot/.content.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<jcr:root xmlns:sling="http://sling.apache.org/jcr/sling/1.0" xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal"
+    jcr:primaryType="rep:AuthorizableFolder"
+    jcr:createdBy="myself"
+    
+    >
+</jcr:root>