Copilot commented on code in PR #3852:
URL: https://github.com/apache/avro/pull/3852#discussion_r3564613880
##########
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:
This test currently passes on any error, which means it would still succeed
even if the zlib `maxOutputLength` cap stopped being applied (the
post-decompress length check would still emit an error). To validate the
intended behavior, assert that the emitted error is the zlib maxOutputLength
failure (e.g., `ERR_BUFFER_TOO_LARGE`).
##########
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:
In `_createBlockCallback`, `_pending` is now decremented on the error path,
but the readable side can still hang if `finish` fired while a block was
pending: `_read()` may already have set `_needPush = true`, and when the
pending block later errors there is no follow-up that will push `null` (since
the success path is the only one that calls `_read()`). This can leave
consumers waiting indefinitely for `'end'` after an `'error'` event.
##########
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:
This test currently passes on any error. Since the purpose is to ensure the
generic post-decompress size safeguard is enforced for *custom* codecs, it
should assert that the specific "decompressed block size exceeds..." error is
raised (not some unrelated failure).
##########
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:
This test currently passes on any error, so it doesn't actually prove that
`maxOutputLength` was applied when a custom codecs map reuses `zlib.inflateRaw`
(it would also fail via the post-decompress size check). Consider asserting the
zlib maxOutputLength error code (`ERR_BUFFER_TOO_LARGE`) here to ensure the
allocation cap is in effect.
--
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]
