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

Support Rack 3 #124

Merged
merged 9 commits into from
Nov 12, 2024
Merged

Support Rack 3 #124

merged 9 commits into from
Nov 12, 2024

Conversation

danielmorrison
Copy link
Contributor

@danielmorrison danielmorrison commented Aug 29, 2024

Update Service to properly support Rack 3.

Looks like this is all we need to do.

  • Rewinding the body is no longer required. Only rewind if it is rewindable. Apps can add Rack::RewindableInput::Middleware if they need rewindable input.
  • Response headers must all be lower case. Use Rack::Headers as recommended (when available) to help ensure headers are lower case.
  • Updated example code for Rack 3.
  • Added example_rack2 that locks to Rack 2, but uses the code from the regular example.

Closes #111

Headers must all be lowercase. See: https://github.com/rack/rack/blob/main/UPGRADE-GUIDE.md#response-headers-must-be-lower-case

Leverage Rack::Headers to help enforce this, falling back to a normal Hash in Rack 2.
Rack 3 no longer requires us to rewind the body. Rather than drop it, we still rewind if we can.
It references the hello_world code from the regular example to show that there are no changes needed.
Keeps them consistent under Rack 2 and 3.
@Maxoplata
Copy link

@arthurnn our team could use this update as well. Hoping it gets merged in!

@arthurnn
Copy link
Owner

thanks for working on this. this is great. I would be nice to have CI running on rack 2 and 3 too (like we have a matrix for faraday)

@danielmorrison
Copy link
Contributor Author

@arthurnn added.

@bteng22
Copy link

bteng22 commented Nov 8, 2024

👋 @arthurnn Our team is facing a vulnerability issue that could benefit from this update as well! Anything else needed to get this over the finish line?

@arthurnn arthurnn merged commit fe455c6 into arthurnn:main Nov 12, 2024
@arthurnn
Copy link
Owner

thanks for working on this

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.

Rack 3 errors - rack_request.body.rewind, example app
4 participants