-
Notifications
You must be signed in to change notification settings - Fork 41
adds rfc compliant http1 parser #132
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
Open
jonfriesen
wants to merge
16
commits into
main
Choose a base branch
from
jon/eng-647-add-rfc-compliant-http1-parser
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44fc1dd
to
9b7b848
Compare
9b7b848
to
35b986a
Compare
tylerflint
reviewed
Sep 17, 2025
tylerflint
reviewed
Sep 17, 2025
ab6e99c
to
143bd7f
Compare
f23af5c
to
f2f3fe5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Our original goal of the http1 parser was to offload as much as possible to the Go standard library. Unfortunately, this was rather difficult to do because the std lib http package is designed around being a consumer or producer of http messages. This means it does makes some changes to wire traffic that work for it. While most of these changes were fine, a few were problematic when it came to a out-of-band observability use case. It also became difficult to handle traffic that was unhealthy, which we still wanted for observability purposes but had to hack around to get pass validity checks within the standard library.
This PR introduces a new HTTP1.0 and HTTP1.1 parser. I designed it following the previous parsers high level streaming operations, but reimplementing the internals using small pieces of the Go http package (eg. Headers struct, and chunk reader implementation), but left the rest of it to my own devices. I focused pretty heavily on satisfying the RFC 9112 spec.
Along with this implementation, I wanted to be a bit more confident in this change as well as future changes to the parser and add some more thorough unit testing. I wrote a handful of tests for the different specific internal tests. Then wrote up some detail specs on how to use the tools and had a combination of Claude 4.1 and Gemini 2.5 help me build a list of important tests and RFC spec related tests. Claude 3.5 Sonnet help with implementation of these tests.