Bug #13675: Should Zlib::GzipReader#ungetc accept nil?

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
• Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
• Tags: patch, ext

[Eregon (Benoit Daloze)]

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

[shevegen (Robert A. Heiler)]

I like symmetry. :)

[matz (Yukihiro Matsumoto)]

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.

[Eregon (Benoit Daloze)]

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.

[jeremyevans0 (Jeremy Evans)]

• File io-ungetc-nil-13675.patch io-ungetc-nil-13675.patch added

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

[matz (Yukihiro Matsumoto)]

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

Matz.

• Status changed from Open to Closed

Applied in changeset git|54499d78109037d7c37bc09a8c3ffa0050da5aca.

---

Remove support for passing nil to IO#ungetc

Fixes [Bug #13675]