make sure we contruct the path correctly when testing on isDirectory

this is only an issue with uri:classloader:/ and a classloader setup where the .jrubydir needs to be loaded to treat the path as directory fixes #3342 Sponsored by Lookout Inc.
jruby · Sep 22, 2015 · 085b5dd · 085b5dd
1 parent 4b6c24f
commit 085b5dd
Showing 4 changed files with 64 additions and 14 deletions.
diff --git a/core/src/main/java/org/jruby/util/URLResource.java b/core/src/main/java/org/jruby/util/URLResource.java
@@ -153,7 +153,7 @@ public Channel openChannel( ModeFlags flags, int perm ) throws ResourceException
         return Channels.newChannel(inputStream());
     }
 
-    public static FileResource createClassloaderURI(Ruby runtime, String pathname, boolean isFile) {
+    public static FileResource createClassloaderURI(Ruby runtime, String pathname, boolean asFile) {
         ClassLoader cl = runtime != null ? runtime.getJRubyClassLoader() : RubyInstanceConfig.defaultClassLoader();
         try
         {
@@ -163,7 +163,7 @@ public static FileResource createClassloaderURI(Ruby runtime, String pathname, b
       }
       final URL url = cl.getResource(pathname);
       String[] files = null;
-      if (!isFile) {
+      if (!asFile) {
           files = listClassLoaderFiles(cl, pathname);
           if (files == null) {
               // no .jrubydir found
@@ -238,19 +238,19 @@ private static boolean addDirectoryEntries(Set<String> entries, URL url,
         return isDirectory;
     }
 
-    public static FileResource create(Ruby runtime, String pathname, boolean isFile) {
+    public static FileResource create(Ruby runtime, String pathname, boolean asFile) {
         if (!pathname.startsWith(URI)) {
             return null;
         }
         // GH-2005 needs the replace
         pathname = pathname.substring(URI.length()).replace("\\", "/");
         if (pathname.startsWith(CLASSLOADER)) {
-            return createClassloaderURI(runtime, pathname.substring(CLASSLOADER.length()), isFile);
+            return createClassloaderURI(runtime, pathname.substring(CLASSLOADER.length()), asFile);
         }
-        return createRegularURI(pathname, isFile);
+        return createRegularURI(pathname, asFile);
     }
 
-    private static FileResource createRegularURI(String pathname, boolean isFile) {
+    private static FileResource createRegularURI(String pathname, boolean asFile) {
         URL url;
         try
         {
@@ -270,7 +270,7 @@ private static FileResource createRegularURI(String pathname, boolean isFile) {
             // file does not exists
             return new URLResource(URI + pathname, (URL)null, null);
         }
-        String[] files = isFile ? null : listFiles(pathname);
+        String[] files = asFile ? null : listFiles(pathname);
         if (files != null) {
             return new URLResource(URI + pathname, (URL)null, files);
         }
@@ -324,7 +324,7 @@ private static String[] listFilesFromInputStream(InputStream is) {
     private static String[] listClassLoaderFiles(ClassLoader classloader, String pathname) {
         try
         {
-            String path = pathname + (pathname.equals("") ? ".jrubydir" : "/.jrubydir");
+            String path = pathname + (pathname.equals("") || pathname.endsWith("/") ? ".jrubydir" : "/.jrubydir");
             Enumeration<URL> urls = classloader.getResources(path);
             if (!urls.hasMoreElements()) {
                 return null;

diff --git a/core/src/test/java/org/jruby/util/URLResourceTest.java b/core/src/test/java/org/jruby/util/URLResourceTest.java
@@ -17,7 +17,19 @@ public void testDirectory(){
         assertTrue(resource.isDirectory());
         assertTrue(resource.exists());
         assertEquals(Arrays.asList(resource.list()),
-                     Arrays.asList(new String[] {".", "dir_without_listing", "dir_with_listing"}));
+                Arrays.asList(new String[] {".", "dir_without_listing", "dir_with_listing"}));
+    }
+
+    public void testDirectoryWithTrailingSlash(){
+        String uri = Thread.currentThread().getContextClassLoader().getResource( "somedir" ).toExternalForm();
+        FileResource resource = URLResource.create((Ruby) null, "uri:" + uri + "/", false);
+
+        assertNotNull(resource );
+        assertFalse(resource.isFile());
+        assertTrue(resource.isDirectory());
+        assertTrue(resource.exists());
+        assertEquals(Arrays.asList(resource.list()),
+                Arrays.asList(new String[] {".", "dir_without_listing", "dir_with_listing"}));
     }
 
     public void testNoneDirectory(){
@@ -53,18 +65,31 @@ public void testNonExistingFile(){
         assertNull(resource.list());
     }
 
-    public void testDirectoryClassloader()
-    {
+    public void testDirectoryClassloader() {
         FileResource resource = URLResource.create((Ruby) null,
                 "uri:classloader:/somedir", false);
 
+        assertNotNull(resource);
+        assertFalse(resource.isFile());
+        assertTrue(resource.isDirectory());
+        assertTrue(resource.exists());
+        assertEquals(Arrays.asList(resource.list()),
+                Arrays.asList(new String[]{".", "dir_without_listing",
+                        "dir_with_listing"}));
+    }
+
+    public void testDirectoryWithTrailingClassloader()
+    {
+        FileResource resource = URLResource.create((Ruby) null,
+                "uri:classloader:/somedir/", false);
+
         assertNotNull( resource );
         assertFalse( resource.isFile() );
         assertTrue( resource.isDirectory() );
         assertTrue( resource.exists() );
         assertEquals( Arrays.asList( resource.list() ),
-                      Arrays.asList( new String[] { ".", "dir_without_listing",
-                                                   "dir_with_listing" } ) );
+                Arrays.asList( new String[] { ".", "dir_without_listing",
+                        "dir_with_listing" } ) );
     }
 
     public void testNoneDirectoryClassloader()
@@ -83,7 +108,7 @@ public void testNoneDirectoryClassloader()
     public void testFileClassloader()
     {
         FileResource resource = URLResource.create((Ruby) null,
-                "uri:classloader:/somedir/.jrubydir", false );
+                "uri:classloader:/somedir/.jrubydir", true );
 
         assertNotNull( resource );
         assertTrue( resource.isFile() );

diff --git a/maven/jruby-complete/src/it/integrity/pom.xml b/maven/jruby-complete/src/it/integrity/pom.xml
@@ -233,6 +233,28 @@
               </arguments>
             </configuration>
           </execution>
+          <execution>
+            <id>META-INF/jruby.home is not a file - GH-3342</id>
+            <phase>test</phase>
+            <goals>
+              <goal>exec</goal>
+            </goals>
+            <configuration>
+              <workingDirectory>${basedir}</workingDirectory>
+              <executable>java</executable>
+              <arguments>
+                <argument>-classpath</argument>
+                <classpath/>
+                <argument>org.jruby.Main</argument>
+                <argument>-e</argument>
+                <argument>
+                  puts "jruby home is a file: " +
+                  File.file?("uri:classloader:/META-INF/jruby.home").to_s +
+                  File.file?("uri:classloader:/META-INF/jruby.home/").to_s
+                </argument>
+              </arguments>
+            </configuration>
+          </execution>
         </executions>
       </plugin>
     </plugins>

diff --git a/maven/jruby-complete/src/it/integrity/verify.bsh b/maven/jruby-complete/src/it/integrity/verify.bsh
@@ -61,3 +61,6 @@ if ( !file.exists() )
 
 expected = "uri:classloader://META-INF/jruby.home";
 if ( !log.contains( expected ) ) throw new RuntimeException( "log file does not contain '" + expected + "'" );
+
+expected = "jruby home is a file: falsefalse";
+if ( !log.contains( expected ) ) throw new RuntimeException( "log file does not contain '" + expected + "'" );