-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix zlib missing handling of buffer error #6610
Fix zlib missing handling of buffer error #6610
Conversation
Probably fixes #4650 too... |
spec/std/zlib/reader_spec.cr
Outdated
@@ -65,5 +65,15 @@ module Zlib | |||
slice = Bytes.empty | |||
reader.read(slice).should eq(0) | |||
end | |||
|
|||
it "should raise bufer error on error (#6575)" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buffer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 5c314e0
@asterite do you want do a rebase and squash that last commit or you prefer me to squash all together? |
5c314e0
to
a1d775f
Compare
@bcardiff Rebased against |
I ment that the last commit (typo) could be squashed into one of the previous ones. |
I like this fix, it at allows fibers to at least not hang now, sweet! I had some question marks about if it should retry on Z_BUF_ERROR if there's more input to be had but it can be addressed later, if it's even a problem, thanks! ref: http://zlib.net/zlib_how.html |
Fixes #6575
The first commit fixes #6575: translating the code that reproduces the bug to ruby raises "buffer error", and we were missing handling that error. I additionally added a check for other potential errors too.
The next commits refactor and improve zilb, fixing a few bugs too, where a function would return
Int
and it was expected to return anError
(an enum). This was easily detected by using the question methods of enums instead of comparing them with enum values.Finally, peeking some Ruby code that deals with zlib, I found out about the
zError
function to get a nice error message in case zlib's stream doesn't provide one.It might be nice to include this fix in 0.26.1 /cc @bcardiff