Skip to content

File.readlink cannot read dangling symlinks #210

@slippycheeze

Description

@slippycheeze

Code to reproduce:

File.exist?('/tmp/link-target') and raise "/tmp/link-target exists"

File.symlink('/tmp/link-target', '/tmp/link')

if File.readlink('/tmp/link') == '/tmp/link-target'
  puts "win: could read a dangling symlink"
else
  puts "fail: couldn't readlink a dangling symlink"
end

The JRuby implementation (as of master, right now, and also present in 1.6.7) doesn't handle dangling symlinks terribly well. In part this is a limitation of Java which Java 7 may lift in the NIO area, but isn't resolvable in earlier versions.

In https://github.com/jruby/jruby/blob/master/src/org/jruby/RubyFile.java#L927 the POSIX helper is used to readlink a symlink. This should work fine for a dangling symlink. However...

https://github.com/jruby/jruby/blob/master/src/org/jruby/RubyFile.java#L929 then checks if the file "exists" which, ultimately, turns out to test using the Java File exists method. That method doesn't work "right" with dangling symlinks - it treats them as non-existent.

Sadly, that is a WONTFIX bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4956115

The reasoning is that the API doesn't really expose symbolic links, so there is no "right" answer.

This could probably be worked around in the JRuby runtime by, eg, checking if the POSIX readlink implementation returned null before doing the checks that try to figure out what failed.

This is probably also the root cause of http://jira.codehaus.org/browse/JRUBY-6614

The fix is likely a less hideous version of this:

diff --git i/src/org/jruby/RubyFile.java w/src/org/jruby/RubyFile.java
index 38f107e..9571374 100644
--- i/src/org/jruby/RubyFile.java
+++ w/src/org/jruby/RubyFile.java
@@ -925,17 +925,18 @@ public class RubyFile extends RubyIO implements EncodingCapable {
         
         try {
             String realPath = runtime.getPosix().readlink(path.convertToString().getUnicodeValue());
+            if (realPath == null) {
+                if (!RubyFileTest.exist_p(recv, path).isTrue()) {
+                    throw runtime.newErrnoENOENTError(path.toString());
+                }
         
-            if (!RubyFileTest.exist_p(recv, path).isTrue()) {
-                throw runtime.newErrnoENOENTError(path.toString());
-            }
-        
-            if (!RubyFileTest.symlink_p(recv, path).isTrue()) {
-                throw runtime.newErrnoEINVALError(path.toString());
-            }
+                if (!RubyFileTest.symlink_p(recv, path).isTrue()) {
+                    throw runtime.newErrnoEINVALError(path.toString());
+                }
         
-            if (realPath == null) {
                 //FIXME: When we get JNA3 we need to properly write this to errno.
+                //FIXME: EIO is probably the most sane thing to throw, but you don't have one defined yet.  I
+                throw runtime.newErrnoEINVALError(path.toString());
             }
 
             return runtime.newString(realPath);

Notably, treat any null return as an error, and don't try to guess the errno without doing that.

(Alternately, use the POSIX exist rather than the Java exist to test if the symlink exists but points to a non-existent file. :)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions