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

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

Closed
the-michael-toy opened this issue Dec 20, 2016 · 7 comments
Closed
Milestone

Comments

@the-michael-toy
Copy link

the-michael-toy commented Dec 20, 2016

% 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
Copy link
Member

enebo commented Dec 20, 2016

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

@enebo
Copy link
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
Copy link
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
Copy link

sumitmah commented Jan 6, 2017

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

@enebo
Copy link
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 pushed a commit to sumitmah/jruby that referenced this issue Jan 6, 2017
@sumitmah
Copy link

sumitmah commented Jan 6, 2017

@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
Copy link
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.

@headius headius modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 15, 2018
@kares kares closed this as completed in f4d76f5 Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants