Project

General

Profile

Actions

Bug #18077

closed

Marshal.dump(closed_io) raises IOError instead of TypeError

Added by larskanis (Lars Kanis) about 3 years ago. Updated about 3 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 3.1.0dev (2021-08-16T08:00:19Z master a8714b83c4) [x86_64-linux]
[ruby-core:104927]

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

Actions #10

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

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0