zlib: fix decompression of empty data streams - #17042
Conversation
Sorry, something went wrong.
add4b0a made the assumption that compressed data would never lead to an empty decompressed stream. Fix that by explicitly checking the number of read bytes. Fixes: nodejs#17041 Refs: nodejs#13322
Sorry, something went wrong.
mcollina
left a comment
There was a problem hiding this comment.
LGTM with a nit.
Sorry, something went wrong.
| if (this.nread >= kMaxLength) { | ||
| err = new errors.RangeError('ERR_BUFFER_TOO_LARGE'); | ||
| } else if (this.nread === 0) { | ||
| buf = Buffer.alloc(0); |
There was a problem hiding this comment.
I don't know if there is any performance penalty to use Buffer.alloc(0) vs Buffer.allocUnsafe(0). However, as it's 0 I would use allocUnsafe.
Sorry, something went wrong.
There was a problem hiding this comment.
alloc() has what is essentially a fast path for 0, allocUnsafe() doesn’t.
Sorry, something went wrong.
There was a problem hiding this comment.
This is very interesting. thanks!
Sorry, something went wrong.
There was a problem hiding this comment.
I mean, I don’t think it’s that way on purpose. :) It just happens to align nicely with how we can use basically new Uint8Array(n) for the Buffer.alloc() fast path, whereas Buffer.allocUnsafe() needs a bit of setup.
Sorry, something went wrong.
add4b0a made the assumption that compressed data would never lead to an empty decompressed stream. Fix that by explicitly checking the number of read bytes. PR-URL: #17042 Fixes: #17041 Refs: #13322 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
add4b0a made the assumption that compressed data would never lead to an empty decompressed stream. Fix that by explicitly checking the number of read bytes. PR-URL: #17042 Fixes: #17041 Refs: #13322 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
add4b0a made the assumption that compressed data
would never lead to an empty decompressed stream.
Fix that by explicitly checking the number of read bytes.
Fixes: #17041
Refs: #13322
Checklist
Affected core subsystem(s)
zlib