File.readlink cannot read dangling symlinks #210
Milestone
Comments
|
Sounds very similar to my https://jira.codehaus.org/browse/JRUBY-6578. |
|
This appears to be fixed in 1.7.12, I think it can be closed. |
|
Yeah, this must have been fixed in jnr-posix. However, The check in https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/RubyFile.java#L978-L1000 still seems weird. I think I intended to remove the first check with 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. |
8 tasks
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 ARGF.read 214cf9e add failing spec for String.new 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
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" endThe 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
existsmethod. 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
readlinkimplementation returnednullbefore 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. :)
The text was updated successfully, but these errors were encountered: