Skip to content
This repository has been archived by the owner on Nov 6, 2022. It is now read-only.

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

Open
KjellSchubert opened this issue Apr 22, 2015 · 1 comment

Comments

@KjellSchubert
Copy link
Contributor

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
Copy link
Member

jasnell commented Oct 26, 2015

@indutny ... thoughts on this one?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants