Skip to content

Fix ParseHttpMessage failing to store >2 repeatable headers #657

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

Merged
merged 2 commits into from
Oct 11, 2022

Conversation

malcolmseyd
Copy link
Contributor

ParseHttpMessage had a bug where HttpMessage.xheaders would not exceed a capacity of 1 inside of ParseHttpMessage. This resulted in only 2 repeatable headers being stored, the first in HttpMessage.headers, and the second in the 1-capacity HttpMessage.xheaders.

This pull request implements a unit test to detect this bug and a fix that passes the unit test. Now, when xheaders has 0 capacity, it's set to 1, and when it's full, the capacity gets doubled.

Also, clang-format rearranged the headers in the test file.

Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting this. Looks like I forgot the += in c2 = c2 >> 1 . Very well executed by the way in the two commit style, creating a failing test case, and then the fix.

Welcome as a new contributor. We need to go through one quick hurdle before we can proceed. Could you please email Justine Tunney [email protected] and say that you intend to assign her the copyright to the changes you contribute to Cosmopolitan? Please send the email under your legal name rather than anonymously. Please use a real email account associated with your identity. See CONTRIBUTING.md for further details. We make the ask because it helps us ship the tiniest most legally permissive prim and proper binaries. You also only need to do it one time, so any future changes you send us can be quickly merged.

Thanks!

@malcolmseyd
Copy link
Contributor Author

Email sent. I'm glad that I can contribute to this great project!

@jart jart merged commit 84b9b8e into jart:master Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants