iemejia opened a new pull request, #3875: URL: https://github.com/apache/avro/pull/3875
## What is the purpose of the change Fix a DoS-adjacent defect in the JavaScript `BlockDecoder`, resolving AVRO-4307. `BlockDecoder` decompresses blocks asynchronously, so many block callbacks can fail for the same bad input (corrupt block, invalid sync marker, unknown codec, or the AVRO-4287 decompression size guard). The failure path emitted `'error'` once **per failing block** with a bare `emit()` and never destroyed the stream. Confirmed empirically on plain `main` (independent of the decompression-limit work) with a 394 KB multi-block file decoded through the real `createFileDecoder` path using a codec that always errors: | Metric | Before | After | |---|---|---| | `'error'` events | 947 | **1** | | bytes read after first error | 65536 | **0** | | source stream destroyed (fd released) | false | **true** | | decoder destroyed | false | **true** | Root cause is in `BlockDecoder._createBlockCallback` (bare emit, no `destroy`, no guard). `createFileDecoder`'s plain `.pipe()` compounds it: a destination error only *unpipes* the source, never destroys it, so the file descriptor leaks. Changes: - Add an `_errored` flag and a guarded `_onError(err)` that calls `destroy(err)`, so exactly one error surfaces and the stream unwinds. Route all fatal errors (block callback, invalid magic, unknown codec, schema parse) through it. - Decrement `_pending` before the error check (previously skipped on error, which could hang the stream at `finish`). - Guard `_write`/`_writeChunk` so no further blocks are processed once errored. - Switch `createFileDecoder` from `.pipe()` to `stream.pipeline()` so the source file stream is destroyed and its descriptor released on error. ## Verifying this change This change added tests and can be verified as follows: - Added `surfaces a single error for many failing blocks` — asserts exactly one `'error'` event (no storm) and that the decoder is destroyed when every block fails. - Added `createFileDecoder tears down the source on error` — asserts a single error and decoder teardown on the `createFileDecoder` path. - Full JS suite passes (`npm test`, 385 + new tests) and `jshint` (`npm run lint`) is clean. ## Documentation - Does this pull request introduce a new feature? no - If yes, how is the feature documented? not applicable -- 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]
