Conversation
Earlopain
left a comment
There was a problem hiding this comment.
I guess this is fine. But, users could end up with cgi in their gemfile one way or another in which case the version check is more predictable.
lib/rack/mock_response.rb
Outdated
| cookie_bits = cookie_filling.split(';') | ||
| cookie_attributes = Hash.new | ||
| cookie_attributes.store('value', Array(cookie_bits[0].strip)) | ||
| cookie_attributes.store('value', cookie_bits[0].strip) |
There was a problem hiding this comment.
This is needed. rack doesn't test against ruby-dev or previews so it doesn't show but here are the failures:
Finished in 3.234368s, 340.0974 runs/s, 1473.8583 assertions/s.
1) Failure:
Rack::MockResponse#test_0007_parses cookies giving max-age precedence over expires [test/spec_mock_response.rb:120]:
Expected: "expires_and_max-age_test"
Actual: "e"
2) Failure:
Rack::MockResponse#test_0004_provides access to session cookies [test/spec_mock_response.rb:87]:
Expected: "session_test"
Actual: "s"
3) Failure:
Rack::MockResponse#test_0006_provides access to persistent cookies set with expires [test/spec_mock_response.rb:109]:
Expected: "persistent_with_expires_test"
Actual: "p"
4) Failure:
Rack::MockResponse#test_0009_parses cookie headers with equals sign at the end [test/spec_mock_response.rb:138]:
--- expected
+++ actual
@@ -1 +1 @@
-"_somebase64encodedstringwithequalsatthened="
+"_"
5) Failure:
Rack::MockResponse#test_0005_provides access to persistent cookies set with max-age [test/spec_mock_response.rb:98]:
Expected: "persistent_test"
Actual: "p"
6) Failure:
Rack::MockResponse#test_0012_parses multiple set-cookie headers provided as hash with array value [test/spec_mock_response.rb:155]:
Expected: "awesome"
Actual: "a"
7) Failure:
Rack::MockResponse#test_0008_provides access to secure cookies [test/spec_mock_response.rb:128]:
Expected: "secure_test"
Actual: "s"There was a problem hiding this comment.
Agreed, did not mean to commit this change.
jeremyevans
left a comment
There was a problem hiding this comment.
This allows a Ruby 3.5 user to opt-in to the cgi/cookie support by installing the gem. It seems a little at-odds with #2332, in that this increases the cases where cgi/cookie is used, while #2332 eliminates the cgi/cookie usage completely. However, I guess that makes sense, as this allows full backwards compatibility if the cgi gem is manually installed, which is probably what we want in a stable branch.
eb2fe0d to
42c1386
Compare
|
@Earlopain What is the name you prefer to use on the changelog? Or is it "earlopain" or "Earl O'Pain"? |
31eecaf to
38b69c4
Compare
|
It's good like this, I don't care about capitalization, both is good. Thanks for handling the releases! |
I would prefer to feature detect
CGI::Cookierather than version check.If we approve this, I want to backport to 2.2 and 3.0.
cc @Earlopain
This is a follow up to #2327.