nodejs / http-parser

http request/response parser for c
MIT License
6.32k stars 1.53k forks source link

http_parser should invoke the begin message callback on first byte, regardless of whether or not its valid #238

Open KjellSchubert opened 9 years ago

KjellSchubert commented 9 years ago

I'm creating this issue on behalf of the Proxygen team, to gauge the odds of the corresponding Proxygen fork's changeset being accepted back into http_parser, quoting Proxygen task description:

Title: http_parser should invoke the begin message callback on first byte, regardless of whether or not its valid Summary: RIght now, http_parser calls the on_message callback only if the first byte of the message is valid. If any subsequent byte is invalid, it will call on_message_error. This is a bit weird, as it means that there's asymmetric behavior depending on which byte contains the error -- one cannot depend on on_message always being called before on_message_error.

(I dont think there is an actual on_message_error callback, where it says on_message_error we mean the parser returns nparsed != recved). The current http_parser behavior for calling parse("ZZZZ") is: the returned nparsed will be 0 (since a 'Z' doesnt start a valid HTTP request or response), on_message_begin will not be called. The Proxygen fork's behavior is: nparsed will also be 0, but on_message_begin will actually be called.

@indutny : what are the odds of this change being accepted by your or the http_parser team in general? I'm asking this before I bother to create the pull request for this change, just in case the odds are slim to none. The change is not terribly complex, it adds 3 states s_pre_start_req_or_res s_pre_start_res s_pre_start_req plus a dozen lines of code plus a few tests.

jasnell commented 8 years ago

@indutny ... thoughts on this one?