Skip to content
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

stream: pipeline should error if any stream is destroyed #36674

Open
ronag opened this issue Dec 29, 2020 · 2 comments · May be fixed by #36791
Open

stream: pipeline should error if any stream is destroyed #36674

ronag opened this issue Dec 29, 2020 · 2 comments · May be fixed by #36791

Comments

@ronag
Copy link
Member

@ronag ronag commented Dec 29, 2020

pipeline should immediately fail with ERR_STREAM_DESTROYED when any of the streams have already been destroyed.

Readable might need a little extra consideration since it's possible to read the data after being destroyed. Should maybe check _readableState.errored and/or _readableState.ended.

Refs: #29227 (comment)

@kalenikalexander
Copy link

@kalenikalexander kalenikalexander commented Dec 29, 2020

I would like to work on this problem

@misos1
Copy link

@misos1 misos1 commented May 19, 2021

I also bumped into this. Http client can easily cause this to http server which is using pipeline (this could cause really bad things):

let { PassThrough, pipeline } = require("stream");
let http = require("http");

let server = http.createServer(async function(req, res)
{
	await new Promise(r => setTimeout(r, 1000));
	console.log("request destroyed", req.destroyed);
	let pass = new PassThrough();
	pipeline(req, pass, e => console.log("pipeline finished", e));
	for await (let chunk of pass) console.log("received", chunk.length);
	console.log("body processed");
	res.end();
});

(async function()
{
	await new Promise(resolve => server.listen(resolve));
	let req = http.request({ port: server.address().port, method: "post" });
	req.on("error", () => null);
	req.write(Buffer.alloc(10000));
	setTimeout(() => req.destroy(), 500);
}());
request destroyed true
received 10000
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
@ronag @misos1 @kalenikalexander and others