-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
v12.17.0 zlib.gzipSync changed compression result #33610
Comments
I'm not sure why anyone would be comparing compressed output like that, especially when different zlib compression parameters could easily change the resulting output. The only important check should be that the data can be decompressed successfully and that the resulting plaintext matches what you expect. |
That's just what I have observed, feel free to close it. |
For example, we used bin compare to verify js port of zlib, with all possible options. See nodeca/pako#193 It would be nice to understand what exactly caused change of deflate binary layer. It's very stable, and there is no much room for deviations. I can imagine only one reasonable possibility - if you slice input for multithread processing, then out will be not single stream, but sequence of properly-finished streams (with flushed end on each). I'm not going to cry about breaking changes :). My case is specific. But i appreciate any help about difference in deflate output. |
@puzrin As for the why, you would have to ask that question to the Chromium team as we are using their zlib fork. |
Probably related to: #31201
I don't know if this is a real world issue, but...
What steps will reproduce the bug?
Compressing a large string in v12.17.0 changes the result, compared to the result in v12.16.3
Also attaching the (zipped) test.js file...
test.js.zip
This is the code that shows what I mean:
The decompression seems to work with both versions, vice-versa... so there should be no big problem.
But if someone compares the compressed output this is a problem.
Additional information
Shouldn't this be a major version regarding semver?
The text was updated successfully, but these errors were encountered: