search

nodejs / http-parser

http request/response parser for c
MIT License
6.35k stars 1.54k forks source link

Always skip body for 1xx, 204, and 304 responses #464

Open azaretsky opened 5 years ago

azaretsky commented 5 years ago

https://tools.ietf.org/html/rfc7230#section-3.3.3: ... any response with a 1xx (Informational), 204 (No Content), or 304 (Not Modified) status code is always terminated by the first empty line after the header fields, regardless of the header fields present in the message, and thus cannot contain a message body.

Fixes: https://github.com/nodejs/http-parser/issues/251

azaretsky commented 5 years ago

It's quite hard to tell if this behavior change will affect anyone since it seems that these responses are rarely encountered in the wild if at all: 1xx, 204, and 304 responses without a declared message body length are already correctly handled by the parser, while 1xx and 204 with a message body are not legitimate according to the spec, and real servers often do not include content-length or transfer-encoidng in 304 responses (at least IIS, nginx and apache httpd don't).

So there's a great chance that this change is harmless but also purely cosmetic and no-one actually needs it.

bnoordhuis commented 5 years ago

cc @indutny - perhaps you can weigh in?

indutny commented 5 years ago

I think it might be too hard to propagate description this change to the consumers of http-parser. In order to properly do it, we'll have to do major release.

@nodejs/http : do we consider major release a possibility, or would we prefer to stick with llhttp and do majority of changes there instead?

azaretsky commented 5 years ago

Just for the record: I will try to keep this branch rebased on top of the current master in case anyone wants to use it before the next major release.