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

Jump to bottom

OutgoingMessage should not allow writing more bytes than Content-Length #39041

Open
ronag opened this issue Jun 15, 2021 · 7 comments · May be fixed by #41166
Open

OutgoingMessage should not allow writing more bytes than Content-Length #39041

ronag opened this issue Jun 15, 2021 · 7 comments · May be fixed by #41166
Labels
good first issue http

Comments

@ronag
Copy link
Member

@ronag ronag commented Jun 15, 2021

I don't think we currently error if the user specifices a content-length header and then writes more bytes to the response. I think this is something we could detect and error. Some http clients have a hard time handling this case (especially with pipelining) and would be nice if we could help with reducing the probability of this occuring.
@ronag ronag added the http label Jun 15, 2021
@ronag
Copy link
Member Author

@ronag ronag commented Jun 15, 2021

@nodejs/http wdyt?

@mcollina
Copy link
Member

@mcollina mcollina commented Jun 15, 2021

I'm in favor if this is not costing much too much throughput.

I think we should add a new option to enable the check (as a minor), and then make a breaking change to flip that default.

@betochf
Copy link

@betochf betochf commented Jun 16, 2021

@mcollina @ronag hey, I'm just getting started, do you mind if I try to pull this off?

@daukadolt
Copy link

@daukadolt daukadolt commented Jun 16, 2021

Looks like an interesting ticket, good luck with it @betochf 👍

Please do let me know though in case if I can take over 🙈

@betochf betochf mentioned this issue Jun 24, 2021
Open
@jodevsa
Copy link

@jodevsa jodevsa commented Oct 30, 2021

@betochf Are you still on it?

@manav014
Copy link

@manav014 manav014 commented Dec 11, 2021

May I start working on this issue? As I think I would be able to create a PR for the same.

@manav014
Copy link

@manav014 manav014 commented Dec 13, 2021

@ronag may please help me with a specific test case which I can add as a test and also I can use that to test my changes.

@JPedroAmorim JPedroAmorim linked a pull request that will close this issue Dec 14, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue http
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

http: throw error if more bytes are written than content-length
6 participants
@mcollina @ronag @jodevsa @daukadolt @manav014 @betochf