Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File.readlink cannot read dangling symlinks #210

slippycheeze opened this issue Jun 15, 2012 · 3 comments

File.readlink cannot read dangling symlinks #210

slippycheeze opened this issue Jun 15, 2012 · 3 comments


Copy link

@slippycheeze slippycheeze commented Jun 15, 2012

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"
  puts "fail: couldn't readlink a dangling symlink"

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 the POSIX helper is used to readlink a symlink. This should work fine for a dangling symlink. However... 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:

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

The fix is likely a less hideous version of this:

diff --git i/src/org/jruby/ w/src/org/jruby/
index 38f107e..9571374 100644
--- i/src/org/jruby/
+++ w/src/org/jruby/
@@ -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. :)

Copy link

@eregon eregon commented Jun 19, 2012

Sounds very similar to my

Copy link

@lolindrath lolindrath commented Apr 17, 2014

This appears to be fixed in 1.7.12, I think it can be closed.

[lolindrath:~/proj/triage-jruby] 1 $ jruby -v
jruby 1.7.12 (1.9.3p392) 2014-04-15 643e292 on Java HotSpot(TM) 64-Bit Server VM 1.7.0_25-b15 [darwin-x86_64]
[lolindrath:~/proj/triage-jruby] $ jruby 210.rb
win: could read a dangling symlink
Copy link

@eregon eregon commented Apr 18, 2014

Yeah, this must have been fixed in jnr-posix.

However, The check in still seems weird. I think I intended to remove the first check with RubyFileTest.exist_p. Only if it it a symlink, we should consider the result of readlink().
But maybe now just relaying on jnr-posix layer is enough and no checks are necessary?

I do not intend to change this though as it seems to work as is and I do neither want to break anything nor have the time to test on Windows.

@eregon eregon closed this Apr 18, 2014
@enebo enebo added this to the JRuby 1.7.13 milestone Jun 24, 2014
eregon added a commit that referenced this issue Mar 28, 2016
f5d943e A few cosmetic fixes in Thread#backtrace spec
38cc7ef Basic tests of Thread#backtrace
c062803 Test where class variables are defined and updated
9f94d6c Add a spec for adding two invalid strings together that results in a valid one
86ddccf Fixed Symbol.all_symbols spec.
a98b62d Wait for a thread status change rather than a random amount of time in rb_thread_blocking_region
3dba3bf Merge pull request #219 from donv/basic_coverage_specs
2e7722e adapt realpath specs to run on Windows
6f1fbd4 syslog is POSIX-only
d9b2e19 adapt realdirpath specs to run on Windows
e80ca16 Process.daemon is not implemented on Windows
801850b Guard unicode path spec not working on Windows
b6c738b Dir.home with an argument is not working on Windows
aa55259 Update for changes in mspec's home_directory helper
25e443c Removing problematic path in dir specs for Windows
8f3e280 Show remaining files if delete_mock_dirs fails
83a11ee Merge pull request #216 from ruby/vais/windows
53b800f Skip all specs that rely on fork on Windows
76811e1 Wrap a few remaining fork-using specs in with_feature :fork block
937be98 Use explicit receiver for the helper method
afbcef4 Change namespace odule to CoverageSpecs to avoid clashing with stdlib
0ecdf4d Guarded specs for peek_result for Ruby 2.3 Used raise_error helper instead of customer begin...rescue
da7b44b Added specs for peek_result Pulled helper method to separate SpecHelper module
607e7ea Filter old results from result hash
3f65bbd Added specs for multiple runs
c3e050f Added a basic spec for counts
490cc2d Merge pull request #217 from donv/try_add_coverage_spec
dbaf24f Started adding spec for coverage
0f2743d Merge pull request #218 from donv/test_mri_2.3
86f19a1 Start testing MRI 2.3.x
07ea93b Fix indent
7e241f7 Fix typo
1a2d168 Reasonable precision in Bignum#* spec
342f9ae Merge pull request #215 from ruby/vais/windows
854cd89 Run AppVeyor only on master and try* branches
fe5acba Passing all WIN32OLE specs
e36ff82 Merge pull request #214 from ruby/vais/windows
4575e16 Skip the entire ARGF.read_nonblock spec on Windows
f580a29 Merge pull request #212 from wied03/master
323d258 Cover another alias/super case
504eea0 Revert "Clean up the entire spec temp dir when finished with mock dirs."
8d75627 AppVeyor: use an inclusion filter
182a683 AppVeyor: fix last commit
4f24a03 AppVeyor: exclude C-API specs for now
40411ed AppVeyor: use the file formatter
ea7d4ba Remove branch filter in appveyor.yml
bfa4a13 Make sure the Signalizer is not run on Windows since it hangs
3bbc6ae Try to run the LeakChecker under AppVeyor
a9c496a Add setup for AppVeyor
06e8a89 Merge pull request #210 from odaira/myContribution
9c17cb6 Merge pull request #211 from ruby/vais/windows
8b5c3eb Pass ENV specs on Windows
2307c18 Remove extra File.join in DirSpecs.nonexistent
eaaf635 Module#{include,prepend} are public since 2.1
b144eac Remove 'ruby_version_is ""..."2.1" do' guards since 2.1 is now the lowest version
32c5020 Remove 'ruby_version_is "2.1" do' guards since this is now the lowest version
c0b7368 Modify a test that succeeds or fails depending on today's date in a year. Time#gmtoff values match only when both or neither of the Time objects are in summer time
2458625 Drop support for 2.0.0 as it is no longer supported
6d4e6f4 Merge pull request #208 from ruby/vais/argf
52eff83 Pass ARGF.binmode on Windows
e8c46eb Fix leak in Kernel#open
5080525 Choose a more appropriate error class in C-API thread specs
c55e620 Fix 2 thread leaks in C-API specs
0c65308 Terminate the WEBrick timeout thread when supported
6d0be63 Fix thread leak in Thread#name spec
ddf7d41 Show leaked subprocess pid
c7a99a6 Revert "Remove now useless Process.waitall"
9676480 Fix subprocess leaks in ObjectSpace.define_finalizer specs
3229895 Do not leak a Dir in Marshal#load specs
4ff6dd5 Fix fd leak in Dir#fileno setup
32c48c7 Remove now useless Process.waitall
4d73980 Wait for subprocess to die in Process specs
b513748 Close the pipe reading-side in the parent as well to avoid leaking a fd
a277946 Unshare Process.times since there is only one usage
c67efe7 Merge pull request #205 from unak/patch-4
ff2c878 get rid of blocking on Windows
5a1ea16 Merge pull request #204 from unak/patch-3
7a2f6b0 Should not assume to exist outer commands.
3b52dae Merge pull request #203 from unak/patch-2
87898e6 Wrong assumption
adbe6d5 Make STDIN non-EOF
add86e1 Revert "Remove problematic specs"
8b5cb71 Remove problematic specs
265cc4c fixup! remove unguaranteed behavior
d8aadfb remove unguaranteed behavior
56d2e70 remove rb_class_inherited spec
4056f12 Remove unreliable spec
01bad6a Do not spec Unicode changing behavior
2258d1d Add missing 2.1 version guards.
3a80eb8 Add missing 2.3 version guards
8825b5f Add a spec for autoloading a constant that was already loaded by another thread
9be9a58 ObjectSpace._id2ref raises for invalid ID values
9d7f072 Move String#@{+,-} specs to the right files
a579440 Added String#-@ and String#+@
974a254 Remove redundant specs
6a44638 Added NameError#receiver spec
e976989 Add specs for rb_time_timespec_new
1b4d6a2 Add specs for rb_define_class_id_under
d61a24c Add a spec for rb_define_class_under when given a mismatched superclass
4e64448 add example which specs behavior of Proc#for_define_method when procs @ruby_method is set
cba082b Added local variable methods to Binding
3c5e63d Use the argf helper in ARGF.read_nonblock
0938149 Added specs for ARGF.read_nonblock
b960ebe Spec Enumerable#sort_by when #map does not return an array
51b21d0 Use describe, rephrase spec description
523f780 Fix count when enumerable yields multiple values
62c1c8c Reword spec description
309fda6 Add a spec covering Array#product returning self when given a block
d9bbeb1 Add another failing specs for zsuper with keyword arguments
89e8773 Fix Kernel#rand when given a Float exclusive range.
08dd725 Added failing specs for zsuper with rest kwargs
a828e0d Add failing specs for slice_* when iterator method is changed
7bb5997 Add specs for IO#{read,write}_nonblock accepting exception option
51fe64d Add specs for ARGF.readpartial
f5b730c Add a spec covering clearing output buffer of
214cf9e add failing spec for with optional encoding argument
48fd956 Move Hash comparison specs to the right files
7ed5840 Avoid the new_hash helper
9c43b34 Add specs for Hash comparison operators
4f7eaa5 Fixed some 64bit assumptions in specs.
1e84f38 Fix a typo in Enumerator::Lazy specs
48060b6 Add specs for File.mkfifo
1e971da Add specs for #dig with no arguments
59b2043 Add specs for Enumerator::Lazy#grep_v
38801cd Add specs for Hash#to_proc
93c40ee Add a spec for setting thread's group in rb_thread_create
668056f Add a spec covering rb_rescue returning the break value when the passed function yields to a block with a break
34bf4b5 Added 2.3 specs for #positive? and #negative?
3988aac multiple calls of close, close_read, and close_write should no longer raise IOError
065e422 Add specs for #dig implmenetation for Hash, Array and Struct classes
0be9395 puts: Make it more compatible to MRI
e3c3858 spec 2.2 behavior where limit 0 raises ArgumentError
d4f49e6 puts: Check if argument responds to :to_ary, instead of checking its type being Array
cc7a472 Add spec for Enumerable#first with multiple yield parameters
7ca9596 Automatically run Travis on try* branches
ba0bd69 Merge pull request #202 from wied03/master
f6ed69a NoMethodError should not require a name (because underlying NameError class does not)
81977c9 sprintf: test infinite and nan float values
37d0d3c Merge pull request #201 from nobu/no-new_ostruct_member
c800735 Do not test method for internal use

git-subtree-dir: spec/ruby
git-subtree-split: f5d943e19e980264de972b6d1b49c927e527053b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants