iemejia commented on code in PR #3852:
URL: https://github.com/apache/avro/pull/3852#discussion_r3564663674
##########
lang/js/lib/files.js:
##########
@@ -247,11 +297,14 @@ BlockDecoder.prototype._createBlockCallback = function ()
{
this._pending++;
return function (err, data) {
+ // Always account for this block's completion, even on error, so a failed
+ // decompression (e.g. the size safeguard firing) cannot leave `_pending`
+ // permanently above zero and hang the stream at finish.
+ self._pending--;
if (err) {
self.emit('error', err);
return;
}
Review Comment:
Fixed. On the error path in `_createBlockCallback`, if the writable side has
already finished and this was the last pending block, the readable side is now
ended (`push(null)`) so a consumer waiting on the queue is not left hanging
after the error. Pushed in d1370e7.
##########
lang/js/test/test_files.js:
##########
@@ -531,6 +531,69 @@ describe('files', function () {
});
});
+ it('rejects a block that decompresses beyond the limit', function (cb) {
+ // A large, highly compressible payload compresses to a tiny block but
+ // would decompress to far more than the configured limit.
+ var t = createType('string');
+ var big = new Array(100001).join('a'); // 100000 bytes when decompressed
+ var encoder = new streams.BlockEncoder(t, {codec: 'deflate'});
+ var decoder = new streams.BlockDecoder({maxDecompressLength: 1024})
+ .on('data', function () {})
+ .on('error', function () { cb(); });
Review Comment:
The test now asserts the specific error: zlib's `ERR_BUFFER_TOO_LARGE` from
the `maxOutputLength` cap. Pushed in d1370e7.
##########
lang/js/test/test_files.js:
##########
@@ -531,6 +531,69 @@ describe('files', function () {
});
});
+ it('rejects a block that decompresses beyond the limit', function (cb) {
+ // A large, highly compressible payload compresses to a tiny block but
+ // would decompress to far more than the configured limit.
+ var t = createType('string');
+ var big = new Array(100001).join('a'); // 100000 bytes when decompressed
+ var encoder = new streams.BlockEncoder(t, {codec: 'deflate'});
+ var decoder = new streams.BlockDecoder({maxDecompressLength: 1024})
+ .on('data', function () {})
+ .on('error', function () { cb(); });
+ encoder.pipe(decoder);
+ encoder.end(big);
+ });
+
+ it('decompresses a block within the limit', function (cb) {
+ var t = createType('string');
+ var payload = 'hello world';
+ var out = [];
+ var encoder = new streams.BlockEncoder(t, {codec: 'deflate'});
+ var decoder = new streams.BlockDecoder({maxDecompressLength: 1024})
+ .on('data', function (s) { out.push(s); })
+ .on('end', function () {
+ assert.deepEqual(out, [payload]);
+ cb();
+ });
+ encoder.pipe(decoder);
+ encoder.end(payload);
+ });
+
+ it('enforces the limit for custom codecs', function (cb) {
+ // A custom codec that yields more than the limit is rejected by the
+ // size safeguard applied to every codec.
+ var t = createType('int');
+ var codecs = {
+ 'null': function (data, cb) { cb(null, Buffer.alloc(4096)); }
+ };
+ var encoder = new streams.BlockEncoder(t, {codec: 'null'});
+ var decoder = new streams.BlockDecoder({
+ codecs: codecs,
+ maxDecompressLength: 1024
+ })
+ .on('data', function () {})
+ .on('error', function () { cb(); });
+ encoder.pipe(decoder);
+ encoder.end(1);
+ });
+
+ it('caps a custom deflate codec that reuses zlib.inflateRaw', function
(cb) {
+ // A custom codecs map that reuses the built-in raw inflater must still
get
+ // the zlib maxOutputLength cap (not just the post-decompress check).
+ var zlib = require('zlib');
+ var t = createType('string');
+ var big = new Array(100001).join('a'); // 100000 bytes when decompressed
+ var encoder = new streams.BlockEncoder(t, {codec: 'deflate'});
+ var decoder = new streams.BlockDecoder({
+ codecs: {deflate: zlib.inflateRaw},
+ maxDecompressLength: 1024
+ })
+ .on('data', function () {})
+ .on('error', function () { cb(); });
Review Comment:
Updated to assert `err.code === 'ERR_BUFFER_TOO_LARGE'`, proving the
`maxOutputLength` cap fired rather than any incidental error. Pushed in d1370e7.
##########
lang/js/test/test_files.js:
##########
@@ -531,6 +531,69 @@ describe('files', function () {
});
});
+ it('rejects a block that decompresses beyond the limit', function (cb) {
+ // A large, highly compressible payload compresses to a tiny block but
+ // would decompress to far more than the configured limit.
+ var t = createType('string');
+ var big = new Array(100001).join('a'); // 100000 bytes when decompressed
+ var encoder = new streams.BlockEncoder(t, {codec: 'deflate'});
+ var decoder = new streams.BlockDecoder({maxDecompressLength: 1024})
+ .on('data', function () {})
+ .on('error', function () { cb(); });
+ encoder.pipe(decoder);
+ encoder.end(big);
+ });
+
+ it('decompresses a block within the limit', function (cb) {
+ var t = createType('string');
+ var payload = 'hello world';
+ var out = [];
+ var encoder = new streams.BlockEncoder(t, {codec: 'deflate'});
+ var decoder = new streams.BlockDecoder({maxDecompressLength: 1024})
+ .on('data', function (s) { out.push(s); })
+ .on('end', function () {
+ assert.deepEqual(out, [payload]);
+ cb();
+ });
+ encoder.pipe(decoder);
+ encoder.end(payload);
+ });
+
+ it('enforces the limit for custom codecs', function (cb) {
+ // A custom codec that yields more than the limit is rejected by the
+ // size safeguard applied to every codec.
+ var t = createType('int');
+ var codecs = {
+ 'null': function (data, cb) { cb(null, Buffer.alloc(4096)); }
+ };
+ var encoder = new streams.BlockEncoder(t, {codec: 'null'});
+ var decoder = new streams.BlockDecoder({
+ codecs: codecs,
+ maxDecompressLength: 1024
+ })
+ .on('data', function () {})
+ .on('error', function () { cb(); });
Review Comment:
Updated to assert the generic post-decompress safeguard message
(`/decompressed block size exceeds/`) for the custom codec. Pushed in d1370e7.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
