Feature #17994: Clarify `IO.read` behavior and add `File.read` method

• Status: Open
• Assignee: -
• Target version: -

[mame (Yusuke Endoh)]

IO.read creates a subprocess when a given file name starts with a | character.

irb(main):001:0> IO.read("| ls /etc/passwd")
=> "/etc/passwd\n"

To disable this feature, File.read can be used.

irb(main):002:0> File.read("| ls /etc/passwd")
(irb):2:in `read': No such file or directory @ rb_sysopen - | ls /etc/passwd (Errno::ENOENT)

So, as far as I know, File.read is more prefereable to IO.read if a user want to just read a file.

However, in terms of the implementation, there is no definition of File.read. File.read invokes IO.read because IO is a superclass of File, and IO.read creates a subprocess only when its receiver is exactly the IO class.

I think there are two problems in the current situation:

1. The rdoc of IO.read does not explain the behavior to disable a subprocess invocation.
2. The rdoc does not have an entry for File.read.

I've created a PR to address the two issues by clarifying IO.read behavior and defining File.read as an alias to IO.read.

https://github.com/ruby/ruby/pull/4579

[jeremyevans0 (Jeremy Evans)]

My only concern is that File isn't the only subclasses of IO. There are also:

UNIXServer
UNIXSocket
UDPSocket
TCPServer
TCPSocket
IPSocket
Socket
BasicSocket

Do we want Socket.read to handle | specially (as IO.read does), or should it operate as File.read (as it does now)? If it currently works as File.read, then switching to IO.read behavior could potentially result in security issues. I doubt there is significant real-world usage of Socket.read or similar, so maybe this is not a significant concern.

Honestly, I don't think read makes sense as a class method on socket classes, so maybe we can avoid the issue by undefing it (after a deprecation period).

[mame (Yusuke Endoh)]

Jeremy, thanks you for your comment.

jeremyevans0 (Jeremy Evans) wrote in #note-1:

Honestly, I don't think read makes sense as a class method on socket classes, so maybe we can avoid the issue by undefing it (after a deprecation period).

It makes sense. Because undefing all subclasses is a bit annoying (and we cannot change user-defined subclasses of IO), it may be good for IO.read to raise an exception when the reciever is neither IO or File. I'd like to bring the deprecation to the agenda of the next dev-meeting.

BTW, do you have any concern against merging my current PR? It just clarifies the current behavior of IO.read and adds File.read alias. We can implement the deprecation warnings later if it is approved.

[jeremyevans0 (Jeremy Evans)]

mame (Yusuke Endoh) wrote in #note-2:

BTW, do you have any concern against merging my current PR? It just clarifies the current behavior of IO.read and adds File.read alias. We can implement the deprecation warnings later if it is approved.

Apologies, I should have reviewed the PR before responding. I see that it does not change the behavior, so I am in favor of merging it.

[akr (Akira Tanaka)]

I feel adding File.read method is uselessly complex behavior:
Socket.read (no pipe) inherits IO.read (support pipe) but behaves as File.read (no pipe).

Also, adding method require more document maintenance cost.

So, I think updating the document of IO.read is best way to go.

I think disabling Socket.read has very few benefit over its effect:
I guess someone would propose disabling other class methods as well.

[mame (Yusuke Endoh)]

I noticed that this issue is not only in IO.read but also in IO.binread, IO.write, IO.binwrite, and IO.readlines.
Adding File.binread etc. is clearly hard to maintain, so I agree that we should change only the doc of IO.read etc.

I have no strong opinion about prohibiting Socket.read etc. IO.read is a tricky class method because it changes its behavior depending on its receiver class, so it makes sense if the receiver is neither IO or File, but I'd like to keep the behavior if anyone is against the change.