Skip to content
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

Added support for HTTP/2 Cookie Headers #198

Merged
merged 2 commits into from
Aug 3, 2024

Conversation

peelle
Copy link
Contributor

@peelle peelle commented Aug 2, 2024

Hi! I am really enjoying learning Cro. Thank you!

Missing Feature: HTTP/2 requests can send multiple Cookie headers. Cro::HTTP::Request will only check the first. This makes sense for HTTP/1.1 since it only allows one.

Solution: I checked the http-version string on unpack, to choose which behavior to use.

Motivation: I was trying to implement sessions and the request object kept losing my cookie, but not the CSRF cookie. After a lot of learning, I realized that browsers were sending multiple Cookie headers via HTTP/2. I saw this behavior with both Chrome and Safari. When I set the Cro server to only accept 1.1, the browsers would only send 1 Cookie header, and the request object could parse it without issue.

Relevant RFC Section

Thanks!

Copy link
Member

@patrickbkr patrickbkr left a comment

Choose a reason for hiding this comment

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

Oh! Good catch, research and implementation. Thank you!

@patrickbkr
Copy link
Member

We are currently searching for people to help developing Cro further. We can use help in any area. (The most pressing need is people willing to review PRs, given we have a "one review required to merge a PR" rule.) Would you want to join?

@patrickbkr patrickbkr merged commit ee938bb into croservices:master Aug 3, 2024
2 checks passed
@peelle
Copy link
Contributor Author

peelle commented Aug 5, 2024

Thank you for the kind words. Yes, I'd love to join and help out.

@patrickbkr
Copy link
Member

patrickbkr commented Aug 6, 2024 via email

@peelle
Copy link
Contributor Author

peelle commented Aug 6, 2024

I would enjoy doing PR reviews and fixing/updating tests first while I'm still getting comfortable with Cro. I don't have any strong desires currently. I can probably be most useful with the WebApp and OpenAPI sections. That aligns with my Perl work experience. I have some interest in learning more about web sockets, but that area is still very new to me.

@patrickbkr
Copy link
Member

patrickbkr commented Aug 6, 2024 via email

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