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

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

Closed
ronag opened this issue Jun 15, 2021 · 12 comments · Fixed by #44378
Closed

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

ronag opened this issue Jun 15, 2021 · 12 comments · Fixed by #44378
Labels
good first issue Issues that are suitable for first-time contributors. http Issues or PRs related to the http subsystem.

Comments

@ronag
Copy link
Member

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 Issues or PRs related to the http subsystem. label Jun 15, 2021
@ronag
Copy link
Member Author

ronag commented Jun 15, 2021

@nodejs/http wdyt?

@mcollina
Copy link
Member

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.

@ronag ronag added the good first issue Issues that are suitable for first-time contributors. label Jun 15, 2021
@betochf
Copy link

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

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

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

@jodevsa
Copy link
Contributor

jodevsa commented Oct 30, 2021

@betochf Are you still on it?

@manav014
Copy link

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

@manav014
Copy link

@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.

@mpodolsk
Copy link

mpodolsk commented Jan 3, 2022

if this will be a perf drop, what about a config flag for strict validation?
also this assumes that we know the size of the payload before sending it :) which from exp may not always be the case.
I think its the RECEIVER of the content that should validate the size against the headers. :)

there are very specific conditions when to send this header.
https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2

If we consider request payloads "inputs" its the receiving system that should validated those 🤔

@LoveSharmaDEV
Copy link

Hi everyone !!!!!
Good Day!!
Its my first time contributing to an open source project.
Wanna know if this issue is still being worked upon.
And any insights for this issue would be very helpful.

@74ankur09
Copy link

Hey everyone,
I am looking forward to contribute in this , so can anyone help me on how to start with contributing as I have just started the open source contribution .
So please share the steps of how to start contributing here .

@sidwebworks
Copy link
Contributor

Hey @ronag is this issue still available for someone to take? I noticed some previously closed PRs due to some broken tests.

Can I take this one and get this resolved?

@mcollina
Copy link
Member

Go for it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are suitable for first-time contributors. http Issues or PRs related to the http subsystem.
Projects
None yet
10 participants