Project

General

Profile

Actions

Bug #13675

closed

Should Zlib::GzipReader#ungetc accept nil?

Added by Eregon (Benoit Daloze) almost 7 years ago. Updated about 4 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
[ruby-core:81752]
Tags:

Description

IO#ungetc accepts nil and just does nothing:

p File.new(__FILE__).ungetc(nil)

But on a GzipReader it fails:

require 'zlib'
zip = "\x1F\x8B\b\x00,\xDC\xD1G\x00\x0334261MLJNI\x05\x00\x9D\x05\x00$\n\x00\x00\x00"
io = StringIO.new zip
gz = Zlib::GzipReader.new(io)
p gz.ungetc(nil)
# =>
repro.rb:8:in `ungetc': no implicit conversion of nil into String (TypeError)
    from repro.rb:8:in `<main>'

Should Zlib::GzipReader#ungetc accept nil?
Or should IO#ungetc also raise when given nil?

Discovered in https://github.com/jruby/jruby/pull/4636#issuecomment-305325839


Files

io-ungetc-nil-13675.patch (1.56 KB) io-ungetc-nil-13675.patch jeremyevans0 (Jeremy Evans), 08/25/2019 06:28 PM

Updated by shevegen (Robert A. Heiler) almost 7 years ago

I like symmetry. :)

Updated by matz (Yukihiro Matsumoto) over 6 years ago

Making Zlib::GzipReader#ungetc accept nil does not help anybody. You should keep as it is.
I am not sure if we should fix IO#ungetc or not (yet). Let me consider.

Matz.

Updated by Eregon (Benoit Daloze) over 6 years ago

matz (Yukihiro Matsumoto) wrote:

Making Zlib::GzipReader#ungetc accept nil does not help anybody. You should keep as it is.

Agreed.

I am not sure if we should fix IO#ungetc or not (yet). Let me consider.

I think we should, it seems a bug to me.

Updated by jeremyevans0 (Jeremy Evans) over 4 years ago

If we want to fix IO#ungetc, attached is a patch that will fix it.

Updated by matz (Yukihiro Matsumoto) about 4 years ago

IO#ungetc should be fixed. The patch looks good to me.

Matz.

Actions #6

Updated by jeremyevans (Jeremy Evans) about 4 years ago

  • Status changed from Open to Closed

Applied in changeset git|54499d78109037d7c37bc09a8c3ffa0050da5aca.


Remove support for passing nil to IO#ungetc

Fixes [Bug #13675]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0