MRI and JRuby disagree about File.exist?("/non_directory_file.name/") #4403

Open
the-michael-toy opened this Issue Dec 20, 2016 · 7 comments

Projects

None yet

4 participants

@the-michael-toy
the-michael-toy commented Dec 20, 2016 edited
% cat test.rb
puts "This file is #{__FILE__}, add a slash to it in #{RUBY_DESCRIPTION}"
puts File.exist?(__FILE__+"/") ? "is INCORRECTLY found" : "is correctly not found"

Here's MRI

% rvm use 2.3.1
Using /Users/mtoy/.rvm/gems/ruby-2.3.1
% ruby test.rb 
This file is test.rb, add a slash to it in ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin16]
is correctly not found

And both 9.1.5.0 and 1.7.19 ( the two versions we use around here) ...

% rvm use jruby-9.1.5.0
Using /Users/mtoy/.rvm/gems/jruby-9.1.5.0
% ruby test.rb 
This file is test.rb, add a slash to it in jruby 9.1.5.0 (2.3.1) 2016-09-07 036ce39 Java HotSpot(TM) 64-Bit Server VM 25.112-b16 on 1.8.0_112-b16 +jit [darwin-x86_64]
is INCORRECTLY found
@enebo
Member
enebo commented Dec 20, 2016

It appears jnr-posix JavaSecureFile will strip off the extra slash erroneously here.

@enebo
Member
enebo commented Dec 20, 2016

Actually two things to point out in this issue:

  1. java.io.File actually exhibits the bad behavior too:
jruby -e 'f = java.io.File.new("../snippets/cp2.rb/"); p f.exists?'
true

cp2.rb is a regular file. It is possible we can just secondarily always check isDirectory() here and then throw ENOTDIR.

  1. The bad behavior in 1 actually strips out the / when it normalizes the path. So even if we use the path from this file to call a native stat() we have already stripped the trailing / so we cannot know to ENOTDIR.

The base reported problem is this should not stat but it is coupled with proper return value as well. is cp2.rb did not exist we should return ENOENT but if it does exist we should return ENOTDIR.

@enebo enebo added this to the JRuby 9.2.0.0 milestone Dec 20, 2016
@enebo
Member
enebo commented Dec 20, 2016

Since we have had this problem perhaps for the entire length of JRuby I am not prioritizing this for the current release.

@sumitmah
sumitmah commented Jan 6, 2017

@enebo shouldn't cruby resolve the path and return true?

@enebo
Member
enebo commented Jan 6, 2017

@sumitmah you can raise it as an issue no cruby's site as a bug and see if they agree or not. I don't actually understand why they return false. It could be as simple as they split on / and get '' as name?

@sumitmah sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 6, 2017
Sumit M [#4403] To be consistent with MRI, File.exists? for <path to file> en…
…ding with / should return false
8b9d7e9
@sumitmah
sumitmah commented Jan 6, 2017 edited

@enebo as per the ruby-doc “file exists” means that stat() or fstat() system call is successful. And since java normalises the file path before resolving I think we need to have explicit check if we want to be consistent with mri. What do you think?

@headius
Member
headius commented Jan 6, 2017

A consistent check would be a one-off, but perhaps it could be written into the contract of JavaSecuredFile or a similar utility. It does seem like a weird choice on the Java side, but I guess they figured no systems can have both a file and a dir of the same name, so removing the trailing element should not be meaningfully different to the FS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment