Support rack 3 cookies

[JoeDupuis]

This is a cherry pick of e2ef83b

Description

Rack 3 switched to arrays for cookies. Puma 6 already supports it (e2ef83b).

The problem if we don't backport this to Puma 5 is that a whole lot of people are about to hit this error.

I built a minimal repro of the issue here.

Since Rails 6.1, Rails has gem "puma", "~> 5.0" as the default web server.

This means that anyone who started a Rails app since 6.1 will hit the issue if they run bundle update after Rails 7.1 releases as Rails main removed the upper bound for rack

If they run bundle update rails there is not much we can do as it won't update Puma, but if we can mitigate the issue for anyone running bundle update this could solve a lot headache.

I doubt this is the only incompatibility with Rack 3. It might be too much to cherry pick everything, so I made this PR as an alternative. This alternative PR would prevent Puma 5 from starting if Rack 3 is loaded. I didn't open it on this repo (lives in my fork), I'll switch it if this approach is preferred.

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
• My pull request is 100 lines added/removed or less so that it can be easily reviewed.
• If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
• If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
• I have updated the documentation accordingly.
• All new and existing tests passed, including Rubocop.

[dentarg]

if they run bundle update after Rails 7.1 releases as Rails main removed the upper bound for rack

Wouldn't it be better if such Rails release set the requirement for Puma to 6? Cc @ioquatix @rafaelfranca (I guess Rails doesn't have a hard requirement on Puma? 🤔 Just new installations that get something in their Gemfiles?)

[ioquatix]

I don't fully understand the issue. Is it because Puma 5 is allowing Rails 3 without actually fully supporting it?

[dentarg]

Puma has no dependency on Rack

[JoeDupuis]

Wouldn't it be better if such Rails release set the requirement for Puma to 6? Cc @ioquatix @rafaelfranca (I guess Rails doesn't have a hard requirement on Puma? thinking Just new installations that get something in their Gemfiles?)

Exactly, Rails doesn't require Puma, but Puma is the default when generating a new app. So, every app generated from Rails 6.1 and after won't benefit from changing the Puma version in rails.

I was searching for the best place to put a check that would mitigate the issue with minimal impact.
I thought of putting a check in Rails that would check for the presence of both Puma 5 and Rack 3 and refuse to boot if both are loaded (similar to my alternative pr), but it doesn't feel like the right place.
Rails often runs without Puma, while Puma rarely run without Rack (I might be wrong here?)

I don't fully understand the issue. Is it because Puma 5 is allowing Rails 3 without actually fully supporting it?

Assuming Rails -> Rack: Yes, but adding a gemspec restriction would not work because bundle update would just update to the current Puma 5.6.* that doesn't have the gemspec restriction. The users would still hit the issue. Plus, it would introduce a dependency on Rack in Puma, which we don't want.

I feel like Puma is the best place for the check, but it is really a Rails issue. If you think a check in Rails is preferable, I can open an issue or a PR on the Rails repo.
If you also think Puma is the best place for the check, I can fix one of my PRs (they are failing tests).

[MSP-Greg]

@JoeDupuis

I spent a bit working on this. Getting CI to pass will be non-trivial. Part if the issue is loading any part of Rack affects other tests. Or, when running the test files in isolation (or one at a time), they all pass, but loading them all in the same process causes failures.

I feel like Puma is the best place for the check, but it is really a Rails issue.

I agree with both. I'm not going to pretend I'm familiar with every framework that can use Puma, but any framework using Puma and Rack may face similar issues. Hence, Puma is probably the best place for the check...

It's too bad Bundler/RubyGems doesn't allow for 'constraints' on gems that aren't dependencies, but I suspect that could be very messy...

[JoeDupuis]

I spent a bit working on this. Getting CI to pass will be non-trivial.

Preventing the app from booting is probably preferable then.

I opened a PR doing that here. I had to close the original one, (I could not change the base repo).

If everyone agree that Puma is the best place for a check and we should prevent booting instead of supporting Rack 3 on Puma 5, I'll close this PR and continue working on the other one.

[JoeDupuis]

Part if the issue is loading any part of Rack affects other tests.

Oh shoot. I missed that. It means I can't just require 'rack' in the other PR. I changed it to require 'rack/rack/version' Hopefully that would work 😮

[dentarg]

Yes, preventing booting (or print warning?) is probably the best we can do for Puma 5 – I don't think it is resonable to get Puma 5 compatible with Rack 3