-
Notifications
You must be signed in to change notification settings - Fork 30k
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
zlib.inflateRawSync how do I get the number of bytes read? #8874
Comments
The bytes consumed count is tracked internally but isn't exposed. It could be retrofitted onto the asynchronous API but it would be very awkward with the synchronous API. Either it would have to be a property on the returned buffer or a side channel such as a function or variable that holds the count of the last operation. |
I would love to see this feature added. Having to use a non-async pure-JS implementation like pako, just to get this little bit of information is pretty lame. It can be hijacked from the private API, by monkey-patching If we can decide on an API, I might be willing to implement it. Some ideas:
zlib.inflate(buff, opts, function(err, buffer, read) {
});
zlib.inflate(buff, {bytesRead: true}, function(err, data) {
var buffer = data.buffer;
var bytesRead = data.bytesRead;
});
zlib.inflate(buff, opts, function(err, buffer) {
var bytesRead = buffer.bytesRead;
});
(see idea 4 before) |
Option 2 - opting in to a different return value / callback signature - is probably the most acceptable. There are other APIs in core that work the same. (No panacea though, it destroys composability.) |
Options 2 and 3 both sound good to me. |
I personally like option 3. |
Adding a property changes the object's shape (hidden class), that has performance implications. |
One other possible issue, is supporting Node streams. I'm not sure any in my suggested ideas cover that use case. |
Another option is to append the length to the end of the buffer (written as an unsigned 32 bit integer) if requested. Default behavior would not write the length (so this would not disrupt existing code), but if you request the length the buffer will be 4 bytes longer. So long as |
For Node stream usage, somehow this information should be available here: const MemoryStream = require('memorystream');
const stream = new MemoryStream();
const engine = zlib.createInflate();
engine.on('data', function(buffer) {
// How to know how much data was read?
});
stream.pipe(engine);
stream.write(buffer); Some ideas:
This also gave me another idea. What if these var engine = zlib.createInflate();
// Async:
engine.process(buff, opts, function(err, buffer) {
var bytesRead = engine.bytesRead;
});
// Sync:
var buffer = engine.processSync(buff, opts);
var bytesRead = engine.bytesRead; I think |
That's overengineering it. We're unlikely to introduce completely new APIs just to expose a simple data property. As well, it's a variation of the side channel I mentioned earlier. You could accomplish the same thing by adding a |
Added bytesRead property to Zlib engines and an option to expose the engine to convenience method callbacks. Fixes: nodejs#8874
Added bytesRead property to Zlib engines Refs: nodejs#8874
Added option to expose engine to convenience methods Refs: nodejs#8874
Refactored bytesRead test to use callbacks instead of timers Refs: nodejs#8874
The introduction of `.bytesRead` to zlib streams was unfortunate, because other streams in Node.js core use the exact opposite naming of `.bytesRead` and `.bytesWritten`. While one could see how the original naming makes sense in a `Transform` stream context, we should try to work towards more consistent APIs in core for these things. This introduces `zlib.bytesWritten` and documentation-only deprecates `zlib.bytesRead`. (Luckily, the old property name hasn’t even been around for a whole year yet.) Refs: nodejs#8874 Refs: nodejs#13088
The introduction of `.bytesRead` to zlib streams was unfortunate, because other streams in Node.js core use the exact opposite naming of `.bytesRead` and `.bytesWritten`. While one could see how the original naming makes sense in a `Transform` stream context, we should try to work towards more consistent APIs in core for these things. This introduces `zlib.bytesWritten` and documentation-only deprecates `zlib.bytesRead`. PR-URL: nodejs#19414 Refs: nodejs#8874 Refs: nodejs#13088 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The introduction of `.bytesRead` to zlib streams was unfortunate, because other streams in Node.js core use the exact opposite naming of `.bytesRead` and `.bytesWritten`. While one could see how the original naming makes sense in a `Transform` stream context, we should try to work towards more consistent APIs in core for these things. This introduces `zlib.bytesWritten` and documentation-only deprecates `zlib.bytesRead`. PR-URL: #19414 Refs: #8874 Refs: #13088 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The introduction of `.bytesRead` to zlib streams was unfortunate, because other streams in Node.js core use the exact opposite naming of `.bytesRead` and `.bytesWritten`. While one could see how the original naming makes sense in a `Transform` stream context, we should try to work towards more consistent APIs in core for these things. This introduces `zlib.bytesWritten` and documentation-only deprecates `zlib.bytesRead`. PR-URL: nodejs#19414 Refs: nodejs#8874 Refs: nodejs#13088 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
zlib.inflateRawSync
returns the decompressed data. AFAICT it does not indicate how many bytes were read from the input buffer. Is there a way to find that?Use case: some ZIP writers use the "data descriptor" feature of the pkzip file format. The CRC-32 checksum of the uncompressed data and the lengths actually appear directly after the deflated data, so you need to know the number of bytes that the deflator read in order to jump ahead to those fields. FWIW zip libraries leveraging zlib, like yauzl, do not bother with the checksum.
The text was updated successfully, but these errors were encountered: