lib: fix unhandled errors in webstream adapters

WebStream's Readable controller does not tolerate `.close()` being
called after an `error`. However, when wrapping a Node's Readable stream
it is possible that the sequence of events leads to `finished()`'s
callback being invoked after such `error`.

In order to handle this, in this change we call the `finished()` handler
earlier when controller is canceled, and always handle this as an error
case.

Fix: https://github.com/nodejs/node/issues/54205
PR-URL: https://github.com/nodejs/node/pull/54206
Fixes: https://github.com/nodejs/node/issues/54205
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Mattias Buelens <mattias@buelens.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit is contained in:
Fedor Indutny 2024-08-06 11:34:35 -07:00 committed by GitHub
parent e2c62203cc
commit 78150f3e9c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 33 additions and 0 deletions

View File

@ -459,6 +459,7 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
const strategy = evaluateStrategyOrFallback(options?.strategy);
let controller;
let wasCanceled = false;
function onData(chunk) {
// Copy the Buffer to detach it from the pool.
@ -480,6 +481,10 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
streamReadable.on('error', () => {});
if (error)
return controller.error(error);
// Was already canceled
if (wasCanceled) {
return;
}
controller.close();
});
@ -491,6 +496,7 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
pull() { streamReadable.resume(); },
cancel(reason) {
wasCanceled = true;
destroy(streamReadable, reason);
},
}, strategy);

View File

@ -0,0 +1,15 @@
'use strict';
require('../common');
const { Readable } = require('stream');
{
const r = Readable.from(['data']);
const wrapper = Readable.fromWeb(Readable.toWeb(r));
wrapper.on('data', () => {
// Destroying wrapper while emitting data should not cause uncaught
// exceptions
wrapper.destroy();
});
}

View File

@ -0,0 +1,12 @@
'use strict';
require('../common');
const { Readable } = require('stream');
{
const r = Readable.from([]);
// Cancelling reader while closing should not cause uncaught exceptions
r.on('close', () => reader.cancel());
const reader = Readable.toWeb(r).getReader();
reader.read();
}