JZlibRubyGzipWriter doesn't handle nil arguments gracefully #1088

Closed
gaurish opened this Issue Oct 7, 2013 · 1 comment

Projects

None yet

1 participant

@gaurish

this works on MRI

> Zlib::GzipWriter.new('works ONLY on MRI', nil, nil)
#<Zlib::GzipWriter:0xa42755c>

but blows up on JRuby

> Zlib::GzipWriter.new('works ONLY on MRI', nil, nil)
TypeError: no implicit conversion from nil to integer
    from org/jruby/ext/zlib/JZlibRubyGzipWriter.java:108:in `initialize'
    from org/jruby/ext/zlib/JZlibRubyGzipWriter.java:49:in `new'
    from (irb):2:in `evaluate'
    from org/jruby/RubyKernel.java:1121:in `eval'
    from org/jruby/RubyKernel.java:1517:in `loop'
    from org/jruby/RubyKernel.java:1282:in `catch'
    from org/jruby/RubyKernel.java:1282:in `catch'
    from /home/gaurish/code/repo/jruby/bin/jirb:13:in `(root)'

Why?
This is needed to get this failing test passing in rails. Also, I think its always a good practice to check for nil values & discard them instead of throwing an exception.

Tested on jruby-head

#11700

@headius headius added a commit that closed this issue Oct 10, 2013
@headius headius Rework some GZipWriter construction logic to fix #1088
* Fix open->newInstance sequence so initialize has the right arity
* Process level and strategy on the way in
* Pass level to jzlib
* Peel off opt hash correctly

Still to do:

* Pass strategy to jzlib
* Match MRI call args for deflateInit2
* Other tweakery to match MRI logic.

See also #1108
32a2f33
@headius headius closed this in 32a2f33 Oct 10, 2013
@headius headius added a commit that referenced this issue Oct 10, 2013
@headius headius Rework some GZipWriter construction logic to fix #1088
* Fix open->newInstance sequence so initialize has the right arity
* Process level and strategy on the way in
* Pass level to jzlib
* Peel off opt hash correctly

Still to do:

* Pass strategy to jzlib
* Match MRI call args for deflateInit2
* Other tweakery to match MRI logic.

See also #1108
817aa2c
@gaurish

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment