You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/08/29 19:20:18 UTC

[commons-vfs] branch master updated: VFS-444: corrected ResourceFileProvider uri/path processing. (#71)

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

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-vfs.git


The following commit(s) were added to refs/heads/master by this push:
     new 049c7f7  VFS-444: corrected ResourceFileProvider uri/path processing. (#71)
049c7f7 is described below

commit 049c7f7e9eb56c973bea3af8c259bb0e6b988a9a
Author: Michiel Hendriks <el...@users.noreply.github.com>
AuthorDate: Thu Aug 29 21:20:14 2019 +0200

    VFS-444: corrected ResourceFileProvider uri/path processing. (#71)
    
    * Changed filename processing for ResourceFileProvider to be consistent with resource locations used by ClassLoader.
    
    Resource locations are absolute, despite not having a leading '/'.
    
    * Absolute resource locations do not start with a slash.
    
    * If the uri has no scheme, use it from the base. If it is still null, default to 'file'.
    
    * Updated documentation concerning the resource URI format.
    
    * Code style corrections.
    
    * Any amount of leading '/' in a resource URI are discarded when looking up a resource.
    
    This makes `res:example` and `res://example` equivalent. This also makes FSM.resolveURI
    and FSM.resolveFile consistent in behavior for any passed URI.
    
    * Added test cases of an eventually blank resource path: res:///
    
    * revert documentation update for 'res:' uri
    
    * Changed example for res: to also include two slashes.
---
 .../org/apache/commons/vfs2/Resources.properties   |   3 +
 .../vfs2/provider/local/LocalFileNameParser.java   |   3 +
 .../vfs2/provider/res/ResourceFileName.java        |  75 ++++++++++++++
 .../vfs2/provider/res/ResourceFileNameParser.java  |  65 ++++++++++++
 .../vfs2/provider/res/ResourceFileProvider.java    |  14 ++-
 .../vfs2/provider/res/test/ResSchemeTestCase.java  |  25 +++--
 .../vfs2/provider/res/test/Vfs444TestCase.java     | 109 +++++++++++++++++++++
 src/site/xdoc/filesystems.xml                      |   2 +-
 8 files changed, 284 insertions(+), 12 deletions(-)

diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/Resources.properties b/commons-vfs2/src/main/java/org/apache/commons/vfs2/Resources.properties
index d9184d4..d6d5bbd 100644
--- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/Resources.properties
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/Resources.properties
@@ -192,6 +192,9 @@ vfs.provider.local/create-folder.error=Could not create directory "{0}".
 vfs.provider.local/not-absolute-file-name.error=URI "{0}" is not an absolute file name.
 vfs.provider.local/missing-share-name.error=Share name missing from UNC file name "{0}".
 
+# Resource Provider
+vfs.provider.res/not-valid-resource-location.error=URI "{0}" is not a valid resource location.
+
 # Temp Provider
 vfs.provider.temp/get-type.error=Could not determine the type of "{0}".
 vfs.provider.temp/delete-file.error=Could not delete "{0}".
diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/local/LocalFileNameParser.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/local/LocalFileNameParser.java
index c7053d4..3c6be1f 100644
--- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/local/LocalFileNameParser.java
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/local/LocalFileNameParser.java
@@ -64,6 +64,9 @@ public abstract class LocalFileNameParser extends AbstractFileNameParser {
 
         // Extract the scheme
         String scheme = UriParser.extractScheme(context.getFileSystemManager().getSchemes(), uri, name);
+        if (scheme == null && base != null) {
+            scheme = base.getScheme();
+        }
         if (scheme == null) {
             scheme = "file";
         }
diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileName.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileName.java
new file mode 100644
index 0000000..623a018
--- /dev/null
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileName.java
@@ -0,0 +1,75 @@
+/*
+ * 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.commons.vfs2.provider.res;
+
+import org.apache.commons.vfs2.FileName;
+import org.apache.commons.vfs2.FileSystemException;
+import org.apache.commons.vfs2.FileType;
+import org.apache.commons.vfs2.provider.AbstractFileName;
+import org.apache.commons.vfs2.provider.UriParser;
+
+/**
+ * A resource file URI.
+ */
+public class ResourceFileName extends AbstractFileName {
+
+    protected ResourceFileName(final String scheme, final String path, final FileType type) {
+        super(scheme, path, type);
+    }
+
+
+    /**
+     * Factory method for creating name instances.
+     *
+     * @param path The file path.
+     * @param type The file type.
+     * @return The FileName.
+     */
+    @Override
+    public FileName createName(final String path, final FileType type) {
+        return new ResourceFileName(getScheme(), path, type);
+    }
+
+    /**
+     * Returns the decoded URI of the file.
+     *
+     * @return the FileName as a URI.
+     */
+    @Override
+    public String toString() {
+        try {
+            return UriParser.decode(super.getURI());
+        } catch (final FileSystemException e) {
+            return super.getURI();
+        }
+    }
+
+    /**
+     * Builds the root URI for this file name.
+     */
+    @Override
+    protected void appendRootUri(final StringBuilder buffer, final boolean addPassword) {
+        buffer.append(getScheme());
+        buffer.append(":");
+    }
+
+    @Override
+    public String getRootURI() {
+        // resource uris have a blank root.
+        return "";
+    }
+}
diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileNameParser.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileNameParser.java
new file mode 100644
index 0000000..0d75e2e
--- /dev/null
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileNameParser.java
@@ -0,0 +1,65 @@
+/*
+ * 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.commons.vfs2.provider.res;
+
+import org.apache.commons.vfs2.FileName;
+import org.apache.commons.vfs2.FileSystemException;
+import org.apache.commons.vfs2.FileType;
+import org.apache.commons.vfs2.provider.local.GenericFileNameParser;
+
+/**
+ * Slightly modified filename parser for resource URIs.
+ */
+public class ResourceFileNameParser extends GenericFileNameParser {
+
+    private static final ResourceFileNameParser INSTANCE = new ResourceFileNameParser();
+
+    /**
+     * retrieve a instance to this parser.
+     *
+     * @return the parser
+     */
+    public static GenericFileNameParser getInstance() {
+        return INSTANCE;
+    }
+
+    @Override
+    protected String extractRootPrefix(final String uri, final StringBuilder name) throws FileSystemException {
+        // Resource uri (as used by ClassLoader.getResource()) are assumed to be absolute despite
+        // lacking a leading '/'. All leading '/' will be stripped from the name.
+
+        int index = 0;
+        while (index < name.length() && name.charAt(index) == '/') {
+            ++index;
+        }
+        if (index > 0) {
+            name.delete(0, index);
+        }
+
+        if (name.length() == 0) {
+            throw new FileSystemException("vfs.provider.res/not-valid-resource-location.error", uri);
+        }
+
+        return "/";
+    }
+    
+    @Override
+    protected FileName createFileName(final String scheme, final String rootFile, final String path,
+            final FileType type) {
+        return new ResourceFileName(scheme, path, type);
+    }
+}
diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileProvider.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileProvider.java
index 0534c69..37ff822 100644
--- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileProvider.java
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/res/ResourceFileProvider.java
@@ -22,6 +22,7 @@ import java.util.Collection;
 import java.util.Collections;
 
 import org.apache.commons.vfs2.Capability;
+import org.apache.commons.vfs2.FileName;
 import org.apache.commons.vfs2.FileObject;
 import org.apache.commons.vfs2.FileSystem;
 import org.apache.commons.vfs2.FileSystemConfigBuilder;
@@ -43,7 +44,7 @@ public class ResourceFileProvider extends AbstractFileProvider {
     private static final int BUFFER_SIZE = 80;
 
     public ResourceFileProvider() {
-        super();
+        setFileNameParser(ResourceFileNameParser.getInstance());
     }
 
     /**
@@ -58,9 +59,14 @@ public class ResourceFileProvider extends AbstractFileProvider {
     @Override
     public FileObject findFile(final FileObject baseFile, final String uri, final FileSystemOptions fileSystemOptions)
             throws FileSystemException {
-        final StringBuilder buf = new StringBuilder(BUFFER_SIZE);
-        UriParser.extractScheme(getContext().getFileSystemManager().getSchemes(), uri, buf);
-        final String resourceName = buf.toString();
+        final FileName fileName;
+        if (baseFile != null) {
+            fileName = parseUri(baseFile.getName(), uri);
+        }
+        else {
+            fileName = parseUri(null, uri);
+        }
+        final String resourceName = fileName.getPath();
 
         ClassLoader classLoader = ResourceFileSystemConfigBuilder.getInstance().getClassLoader(fileSystemOptions);
         if (classLoader == null) {
diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/res/test/ResSchemeTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/res/test/ResSchemeTestCase.java
index cfda132..80e77a0 100644
--- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/res/test/ResSchemeTestCase.java
+++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/res/test/ResSchemeTestCase.java
@@ -33,17 +33,17 @@ public class ResSchemeTestCase {
         Assert.assertTrue(VFS.getManager().resolveFile("res:test.properties").exists());
     }
 
-    @Test(expected = FileSystemException.class)
+    @Test
     public void test_resolveFile_String_S() throws FileSystemException {
         Assert.assertTrue(VFS.getManager().resolveFile("res:/test.properties").exists());
     }
 
-    @Test(expected = FileSystemException.class)
+    @Test
     public void test_resolveFile_String_SS() throws FileSystemException {
         Assert.assertTrue(VFS.getManager().resolveFile("res://test.properties").exists());
     }
 
-    @Test(expected = FileSystemException.class)
+    @Test
     public void test_resolveFile_String_SSS() throws FileSystemException {
         Assert.assertTrue(VFS.getManager().resolveFile("res://test.properties").exists());
     }
@@ -53,23 +53,22 @@ public class ResSchemeTestCase {
         Assert.assertTrue(VFS.getManager().resolveFile(new URI("res:test.properties")).exists());
     }
 
-    @Test(expected = FileSystemException.class)
+    @Test
     public void test_resolveFile_URI_S() throws FileSystemException, URISyntaxException {
         Assert.assertTrue(VFS.getManager().resolveFile(new URI("res:/test.properties")).exists());
     }
 
-    @Test(expected = FileSystemException.class)
+    @Test
     public void test_resolveFile_URI_SS() throws FileSystemException, URISyntaxException {
         Assert.assertTrue(VFS.getManager().resolveFile(new URI("res://test.properties")).exists());
     }
 
-    @Test(expected = FileSystemException.class)
+    @Test
     public void test_resolveFile_URI_SSS() throws FileSystemException, URISyntaxException {
         Assert.assertTrue(VFS.getManager().resolveFile(new URI("res://test.properties")).exists());
     }
 
     @Test
-    @Ignore("It seems like this should be made to work; see VFS-444.")
     public void test_resolveURI_String() throws FileSystemException {
         Assert.assertTrue(VFS.getManager().resolveURI("res:test.properties").isFile());
     }
@@ -88,4 +87,16 @@ public class ResSchemeTestCase {
     public void test_resolveURI_String_SSS() throws FileSystemException {
         Assert.assertTrue(VFS.getManager().resolveURI("res:///test.properties").isFile());
     }
+
+    @Test(expected = FileSystemException.class)
+    public void test_resolveURI_String_SSSnull() throws FileSystemException {
+        // Resulting path is empty
+        Assert.assertTrue(VFS.getManager().resolveURI("res:///").isFile());
+    }
+
+    @Test(expected = FileSystemException.class)
+    public void test_resolveFile_String_SSSnull() throws FileSystemException {
+        // Resulting path is empty
+        Assert.assertTrue(VFS.getManager().resolveFile("res:///").exists());
+    }
 }
diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/res/test/Vfs444TestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/res/test/Vfs444TestCase.java
new file mode 100644
index 0000000..49e3412
--- /dev/null
+++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/res/test/Vfs444TestCase.java
@@ -0,0 +1,109 @@
+/*
+ * 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.commons.vfs2.provider.res.test;
+
+import org.apache.commons.AbstractVfsTestCase;
+import org.apache.commons.vfs2.FileName;
+import org.apache.commons.vfs2.FileObject;
+import org.apache.commons.vfs2.FileSystemException;
+import org.apache.commons.vfs2.FileSystemManager;
+import org.apache.commons.vfs2.impl.DefaultFileSystemManager;
+import org.apache.commons.vfs2.provider.res.ResourceFileProvider;
+import org.apache.commons.vfs2.provider.zip.ZipFileProvider;
+import org.apache.commons.vfs2.test.AbstractProviderTestCase;
+import org.apache.commons.vfs2.test.AbstractProviderTestConfig;
+import org.apache.commons.vfs2.test.ProviderTestSuite;
+import org.junit.Assert;
+
+import junit.framework.Test;
+
+/**
+ * Test cases for VFS-444. 
+ */
+public class Vfs444TestCase extends AbstractProviderTestConfig {
+    public static Test suite() throws Exception {
+        ProviderTestSuite suite = new ProviderTestSuite(new Vfs444TestCase(), true);
+        suite.addTests(Vfs444Tests.class);
+        return suite;
+    }
+
+    /**
+     * Prepares the file system manager. This implementation does nothing.
+     */
+    @Override
+    public void prepare(final DefaultFileSystemManager manager) throws Exception {
+        manager.addProvider("res", new ResourceFileProvider());
+        manager.addProvider("zip", new ZipFileProvider());
+    }
+
+    /**
+     * Returns the base folder for tests.
+     */
+    @Override
+    public FileObject getBaseTestFolder(final FileSystemManager manager) throws Exception {
+        final String baseDir = AbstractVfsTestCase.getResourceTestDirectory();
+        return manager.resolveFile("zip:res:" + baseDir + "/test.zip");
+    }
+
+    public static class Vfs444Tests extends AbstractProviderTestCase {
+
+        public void testResolveFullPathURI0() throws FileSystemException {
+            FileName result = getManager().resolveURI("res:test-data/test.zip");
+            Assert.assertTrue(result.isFile());
+        }
+
+        public void testResolveFullPathFile0() throws FileSystemException {
+            FileObject result = getManager().resolveFile("res:test-data/test.zip");
+            Assert.assertTrue(result.exists());
+        }
+
+        public void testResolveFullPathURI1() throws FileSystemException {
+            FileName result = getManager().resolveURI("res:/test-data/test.zip");
+            Assert.assertTrue(result.isFile());
+        }
+
+        public void testResolveFullPathFile1() throws FileSystemException {
+            FileObject result = getManager().resolveFile("res:/test-data/test.zip");
+            Assert.assertTrue(result.exists());
+        }
+
+        public void testResolveFullPathURI2() throws FileSystemException {
+            FileName result = getManager().resolveURI("res://test-data/test.zip");
+            Assert.assertTrue(result.isFile());
+        }
+
+        public void testResolveFullPathFile2() throws FileSystemException {
+        	FileObject result = getManager().resolveFile("res://test-data/test.zip");
+            Assert.assertTrue(result.exists());
+        }
+
+        public void testResolvePartialPath1() throws FileSystemException {
+            FileName result = getManager().resolveURI("res:test-data");
+            Assert.assertTrue(result.isFile());
+        }
+
+        public void testResolvePartialPath2() throws FileSystemException {
+            FileName root = getManager().resolveURI("res:test-data");
+            FileName file = getManager().resolveName(root, "test.zip");
+            String uri = file.getURI();
+            FileObject result = getManager().resolveFile(uri); 
+            Assert.assertNotNull(result);
+            Assert.assertTrue(result.exists());
+        }
+    }
+}
diff --git a/src/site/xdoc/filesystems.xml b/src/site/xdoc/filesystems.xml
index e0d0559..400843c 100644
--- a/src/site/xdoc/filesystems.xml
+++ b/src/site/xdoc/filesystems.xml
@@ -773,7 +773,7 @@
 
             <ul>
                 <li>
-                    <code>res:path/in/classpath/image.png</code><br/>
+                    <code>res://path/in/classpath/image.png</code><br/>
                     might result in
                     <code>jar:file://my/path/to/images.jar!/path/in/classpath/image.png</code><br/>
                 </li>