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.

Sign up for GitHub

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

Jump to bottom

Raise Errno::ENOTDIR if the parent of an opened file is not a directory #4816

Closed
wants to merge 1 commit into from
Closed

Raise Errno::ENOTDIR if the parent of an opened file is not a directory #4816

wants to merge 1 commit into from
+13 −0

Conversation

philr
Copy link
Contributor

@philr philr commented Oct 15, 2017 •
edited
Loading

MRI 2.4.2 raises Errno::ENOTDIR when attempting to open a path where the parent entry is not a directory. For example, supposing /tmp/file1 is a regular file:

> File.open('/tmp/file1/file2')
Errno::ENOTDIR: Not a directory @ rb_sysopen - /tmp/file1/file2
        from (irb):1:in `initialize'
        from (irb):1:in `open'
        from (irb):1

However, JRuby 9.1.13.0 (on platforms other than Windows) raises Java::JavaLang::RuntimeException instead:

> File.open('/tmp/file1/file2')
Java::JavaLang::RuntimeException: Unhandled IOException: java.io.IOException: unhandled errno: Not a directory
        from org.jruby.util.io.PosixShim.open(PosixShim.java:426)
        from org.jruby.RubyIO.cloexecOpen(RubyIO.java:1266)
        from org.jruby.RubyIO.sysopenFunc(RubyIO.java:1252)
        from org.jruby.RubyIO.sysopenInternal(RubyIO.java:1244)
        from org.jruby.RubyIO.sysopen(RubyIO.java:1229)
        from org.jruby.RubyFile.fileOpenGeneric(RubyFile.java:1373)
        from org.jruby.RubyFile.openFile(RubyFile.java:1350)
        from org.jruby.RubyFile.initialize(RubyFile.java:366)
        from org.jruby.RubyFile$INVOKER$i$0$3$initialize.call(RubyFile$INVOKER$i$0$3$initialize.gen)
        from org.jruby.runtime.callsite.CachingCallSite.callBlock(CachingCallSite.java:77)
        from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:83)
        from org.jruby.RubyClass.newInstance(RubyClass.java:1022)
        from org.jruby.RubyIO.open(RubyIO.java:1154)
        from org.jruby.RubyIO$INVOKER$s$0$0$open.call(RubyIO$INVOKER$s$0$0$open.gen)

This pull request changes RegularFileResource.openChannel() and PosixShim.open() so that they handle the ENOTDIR case, making File.open raise the correct exception.

The change only applies to platforms other than Windows. On Windows, both JRuby and MRI raise Errno::ENOENT for such paths.

@philr
Raise Errno::ENOTDIR if the parent of an opened file is not a directory.
752eedd 
Note that Errno::ENOENT is raised on Windows. This is consistent with
the MRI implementation.
@kares
Copy link
Member

kares commented Oct 16, 2017

well done, thanks Phil

@kares
Copy link
Member

kares commented Oct 16, 2017 •
edited
Loading

seems that some tests from the test:jruby suite started failing, afterwards e.g. https://travis-ci.org/jruby/jruby/jobs/288206627#L1452-L1461 ... maybe just need adjusting - could you check them with MRI 2.3/2.4?

@philr
Copy link
Contributor Author

philr commented Oct 16, 2017 •
edited
Loading

@kares Those tests were already failing on master prior to this change (see https://travis-ci.org/jruby/jruby/jobs/287161724#L1414-L1424).

It looks like either 404fdc9 or 471b9c9 caused the breakage.

@kares
Copy link
Member

kares commented Oct 18, 2017

that seems right - 6 failures of the same but your added test-case passed, sorry for the confusion

@kares
Copy link
Member

kares commented Oct 18, 2017

picked on master as 870011e

keeping open as likely this makes sense to be backported for 9.1 ...

@kares kares added this to the JRuby 9.1.14.0 milestone Oct 18, 2017
@enebo enebo modified the milestones: JRuby 9.1.14.0, JRuby 9.2.0.0 Oct 18, 2017
@enebo
Copy link
Member

enebo commented Oct 18, 2017

This appears to be only for 2.4 and higher so no backporting.

@enebo enebo closed this Oct 18, 2017
@philr
Copy link
Contributor Author

philr commented Oct 18, 2017

@enebo The Errno::ENOTDIR exception is raised by earlier MRI versions too. I've just checked 1.9.3, 2.0.0, 2.1.10, 2.2.8 and 2.3.5 and they all behave identically to 2.4.2.

@enebo
Copy link
Member

enebo commented Oct 18, 2017

@philr omg I did not read that file1 had to exist. You are correct and we should backport as well. I will re-open.

@enebo enebo modified the milestones: JRuby 9.2.0.0, JRuby 9.1.14.0 Oct 18, 2017
@enebo enebo reopened this Oct 18, 2017
@kares
Copy link
Member

kares commented Oct 19, 2017

double checked on 2.3.3 :

2.3.3 :001 > File.open('/tmp/file1/file2')
Errno::ENOENT: No such file or directory @ rb_sysopen - /tmp/file1/file2
	from (irb):1:in `initialize'
	from (irb):1:in `open'
	from (irb):1
	from /opt/local/rvm/rubies/ruby-2.3.3/bin/irb:11:in `<main>'
2.3.3 :002 > exit
kares@sputnik:~/workspace/oss/jruby-rack-worker$ touch /tmp/file1
kares@sputnik:~/workspace/oss/jruby-rack-worker$ irb
2.3.3 :001 > File.open('/tmp/file1/file2')
Errno::ENOTDIR: Not a directory @ rb_sysopen - /tmp/file1/file2
	from (irb):1:in `initialize'
	from (irb):1:in `open'
	from (irb):1
	from /opt/local/rvm/rubies/ruby-2.3.3/bin/irb:11:in `<main>'

@kares
Copy link
Member

kares commented Oct 19, 2017

now on jruby-.9.1 as well ... e7086e5
Thanks, Phil.

@kares kares closed this Oct 19, 2017
@philr philr deleted the raise_errno_enotdir branch October 28, 2017 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
JRuby 9.1.14.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@philr @kares @enebo