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

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

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@philr
Contributor

philr commented Oct 15, 2017

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.

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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Oct 16, 2017

Member

well done, thanks Phil

Member

kares commented Oct 16, 2017

well done, thanks Phil

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Oct 16, 2017

Member

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?

Member

kares commented Oct 16, 2017

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

This comment has been minimized.

Show comment
Hide comment
@philr

philr Oct 16, 2017

Contributor

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

Contributor

philr commented Oct 16, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Oct 18, 2017

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Oct 18, 2017

Member

picked on master as 870011e

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

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

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Oct 18, 2017

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@philr

philr Oct 18, 2017

Contributor

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

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Oct 18, 2017

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Oct 19, 2017

Member

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>'

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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Oct 19, 2017

Member

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

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 philr:raise_errno_enotdir branch Oct 28, 2017

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