Bug #18077
closedMarshal.dump(closed_io) raises IOError instead of TypeError
Description
Marshal.dump is expected to raise a TypeError
for unmarshallable objects. But closed streams raise an IOError
:
$ ruby -e "io=IO.pipe.first; io.close; Marshal.dump(io)"
-e:1:in `internal_encoding': closed stream (IOError)
from -e:1:in `dump'
from -e:1:in `<main>'
This issue is present in all current ruby versions.
Updated by nobu (Nobuyoshi Nakada) about 3 years ago
One concern for that [PR], probably may not be a matter, is closed IO
s will no longer raise IOError
on other Encoding
operations too.
[PR]: https://github.com/ruby/ruby/pull/4746
$ ./ruby -v -e 'p Encoding.compatible?(Encoding::US_ASCII, File.open(IO::NULL).tap(&:close))'
ruby 3.1.0dev (2021-08-16T08:00:19Z master a8714b83c4) [x86_64-darwin19]
-e:1:in `internal_encoding': closed stream (IOError)
from -e:1:in `compatible?'
from -e:1:in `<main>'
$ ./ruby -v -e 'p Encoding.compatible?(Encoding::US_ASCII, File.open(IO::NULL).tap(&:close))'
ruby 3.1.0dev (2021-08-16T12:33:34Z master 9dd58a421b) [x86_64-darwin19]
last_commit=Fix Marshal.dump(closed_io) to raise TypeError
nil
Updated by larskanis (Lars Kanis) about 3 years ago
https://github.com/ruby/ruby/pull/4749 is another fix without the above side effect. Is it OK?
Updated by Eregon (Benoit Daloze) about 3 years ago
Why does IO#internal_encoding (and external_encoding) raise if the IO is closed?
They could just return the encoding, it's still stored in in the IO instance, and we are not trying to access the fd
, right?
JRuby 9.2.17.0 does not raise, and I think it is the better behavior here.
Updated by mame (Yusuke Endoh) about 3 years ago
I don't have any strong opinion but @eregon's approach looks a bit better.
Updated by larskanis (Lars Kanis) about 3 years ago
@Eregon (Benoit Daloze) This was my first though as well, but the current behavior is already defined in ruby-spec.
I would prefer the behavioral change of IO#(in|ex)ternal_encoding
being usable on closed IOs and prepared a PR: https://github.com/ruby/ruby/pull/4758
Updated by nobu (Nobuyoshi Nakada) about 3 years ago
Since this is a bug report, I think that the old example in rubyspec should be removed.
Updated by larskanis (Lars Kanis) about 3 years ago
I removed the old example from https://github.com/ruby/ruby/pull/4758 .
Updated by Eregon (Benoit Daloze) about 3 years ago
larskanis (Lars Kanis) wrote in #note-5:
@Eregon (Benoit Daloze) This was my first though as well, but the current behavior is already defined in ruby-spec.
We can change it in ruby/spec (adding version guards and adding the new expectation).
And in fact I would be glad if the adapted specs would test the more sensible behavior.
ruby/spec often tests whatever the behavior of CRuby is (for best compatibility with other Rubies), it might not always make sense.
Updated by Eregon (Benoit Daloze) about 3 years ago
I should have read the PR first.
Yes, removing the old example and adding the new one with a version guard is perfect here :)
That allows other implementations & older versions to potentially use the new behavior already, and there is probably very little to no code relying on the previous exception.
Updated by Anonymous about 3 years ago
- Status changed from Open to Closed
Applied in changeset git|6594623f623a0982da62cc105094da0701d499da.
Fix Marshal.dump(closed_io) to raise TypeError and allow encoding on closed IO
Mashalling a closed IO object raised "closed stream (IOError)" before instead of TypeError.
This changes IO#(in|ex)ternal_encoding to still return the encoding even if the underlying FD is closed.
Fixes bug #18077