Skip to content

Commit

Permalink
Remove cookie-jar dependency for ruby 3.3 (#2395)
Browse files Browse the repository at this point in the history
* Remove cookie-jar dependency
Remove deleted and set max-age 0 for current cookies
Replace HTTP_COOKIE by set-cookie function
Replace Set-Cookie by Rack::SET_COOKIE
Update specs

* Remove delete_set_cookie_header and set_cookie_header since its in rack >= 3.0

* Add cookie_helper to manage different expires value.

* Fix rubocop

* Add cookiejar to Rack::MockResponse

* Add CHANGELOG.md entry

* Update CHANGELOG.md

* Fix CHANGELOG.md
  • Loading branch information
ericproulx authored Dec 31, 2023
1 parent 63a0416 commit 15c891c
Show file tree
Hide file tree
Showing 18 changed files with 86 additions and 52 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* [#2384](https://github.com/ruby-grape/grape/pull/2384): Allow to use `before/after/rescue_from` methods in any order when using `mount` - [@jcagarcia](https://github.com/jcagarcia).
* [#2390](https://github.com/ruby-grape/grape/pull/2390): Drop support for Ruby 2.6 and Rails 5 - [@ericproulx](https://github.com/ericproulx).
* [#2393](https://github.com/ruby-grape/grape/pull/2393): Optimize AttributeTranslator - [@ericproulx](https://github.com/ericproulx).
* [#2395](https://github.com/ruby-grape/grape/pull/2395): Set `max-age` to 0 when `cookies.delete` - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.

#### Fixes
Expand Down
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/multi_json.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/multi_xml.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rack_1_0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rack_2_0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rack_3_0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rack_edge.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rails_6_0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rails_6_1.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rails_7_0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rails_7_1.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
1 change: 0 additions & 1 deletion gemfiles/rails_edge.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group :development do
end

group :test do
gem 'cookiejar'
gem 'grape-entity', '~> 0.6', require: false
gem 'mime-types'
gem 'rack-jsonp', require: 'rack/jsonp'
Expand Down
3 changes: 2 additions & 1 deletion lib/grape/cookies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ def each(&block)
@cookies.each(&block)
end

# see https://github.com/rack/rack/blob/main/lib/rack/utils.rb#L338-L340
# rubocop:disable Layout/SpaceBeforeBrackets
def delete(name, **opts)
options = opts.merge(value: 'deleted', expires: Time.at(0))
options = opts.merge(max_age: '0', value: '', expires: Time.at(0))
self.[]=(name, options)
end
# rubocop:enable Layout/SpaceBeforeBrackets
Expand Down
1 change: 0 additions & 1 deletion spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

describe Grape::API do
subject do
puts described_class
Class.new(described_class)
end

Expand Down
65 changes: 28 additions & 37 deletions spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,37 +175,42 @@ def app

get('/get/cookies')

expect(Array(last_response.headers['Set-Cookie']).flat_map { |h| h.split("\n") }.sort).to eql [
'cookie3=symbol',
'cookie4=secret+code+here',
'my-awesome-cookie1=is+cool',
'my-awesome-cookie2=is+cool+too; domain=my.example.com; path=/; secure'
]
expect(last_response.cookie_jar).to contain_exactly(
{ 'name' => 'cookie3', 'value' => 'symbol' },
{ 'name' => 'cookie4', 'value' => 'secret code here' },
{ 'name' => 'my-awesome-cookie1', 'value' => 'is cool' },
{ 'name' => 'my-awesome-cookie2', 'value' => 'is cool too', 'domain' => 'my.example.com', 'path' => '/', 'secure' => true }
)
end

it 'sets browser cookies and does not set response cookies' do
set_cookie %w[username=mrplum sandbox=true]
subject.get('/username') do
cookies[:username]
end
get('/username', {}, 'HTTP_COOKIE' => 'username=mrplum; sandbox=true')

get '/username'
expect(last_response.body).to eq('mrplum')
expect(last_response.headers['Set-Cookie']).to be_nil
expect(last_response.cookie_jar).to be_empty
end

it 'sets and update browser cookies' do
set_cookie %w[username=user sandbox=false]
subject.get('/username') do
cookies[:sandbox] = true if cookies[:sandbox] == 'false'
cookies[:username] += '_test'
end
get('/username', {}, 'HTTP_COOKIE' => 'username=user; sandbox=false')

get '/username'
expect(last_response.body).to eq('user_test')
cookies = Array(last_response.headers['Set-Cookie']).flat_map { |h| h.split("\n") }
expect(cookies[0]).to match(/username=user_test/)
expect(cookies[1]).to match(/sandbox=true/)
expect(last_response.cookie_jar).to contain_exactly(
{ 'name' => 'sandbox', 'value' => 'true' },
{ 'name' => 'username', 'value' => 'user_test' }
)
end

it 'deletes cookie' do
set_cookie %w[delete_this_cookie=1 and_this=2]
subject.get('/test') do
sum = 0
cookies.each do |name, val|
Expand All @@ -214,22 +219,16 @@ def app
end
sum
end
get '/test', {}, 'HTTP_COOKIE' => 'delete_this_cookie=1; and_this=2'
get '/test'
expect(last_response.body).to eq('3')
cookies = Array(last_response.headers['Set-Cookie']).flat_map { |h| h.split("\n") }.to_h do |set_cookie|
cookie = CookieJar::Cookie.from_set_cookie 'http://localhost/test', set_cookie
[cookie.name, cookie]
end
expect(cookies.size).to eq(2)
%w[and_this delete_this_cookie].each do |cookie_name|
cookie = cookies[cookie_name]
expect(cookie).not_to be_nil
expect(cookie.value).to eq('deleted')
expect(cookie.expired?).to be true
end
expect(last_response.cookie_jar).to contain_exactly(
{ 'name' => 'and_this', 'value' => '', 'max-age' => 0, 'expires' => Time.at(0) },
{ 'name' => 'delete_this_cookie', 'value' => '', 'max-age' => 0, 'expires' => Time.at(0) }
)
end

it 'deletes cookies with path' do
set_cookie %w[delete_this_cookie=1 and_this=2]
subject.get('/test') do
sum = 0
cookies.each do |name, val|
Expand All @@ -238,20 +237,12 @@ def app
end
sum
end
get('/test', {}, 'HTTP_COOKIE' => 'delete_this_cookie=1; and_this=2')
get '/test'
expect(last_response.body).to eq('3')
cookies = Array(last_response.headers['Set-Cookie']).flat_map { |h| h.split("\n") }.to_h do |set_cookie|
cookie = CookieJar::Cookie.from_set_cookie 'http://localhost/test', set_cookie
[cookie.name, cookie]
end
expect(cookies.size).to eq(2)
%w[and_this delete_this_cookie].each do |cookie_name|
cookie = cookies[cookie_name]
expect(cookie).not_to be_nil
expect(cookie.value).to eq('deleted')
expect(cookie.path).to eq('/test')
expect(cookie.expired?).to be true
end
expect(last_response.cookie_jar).to contain_exactly(
{ 'name' => 'and_this', 'path' => '/test', 'value' => '', 'max-age' => 0, 'expires' => Time.at(0) },
{ 'name' => 'delete_this_cookie', 'path' => '/test', 'value' => '', 'max-age' => 0, 'expires' => Time.at(0) }
)
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/integration/rack/v3/headers_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

describe Grape::Http::Headers do
describe Grape::Http::Headers, if: Gem::Version.new(Rack.release) >= Gem::Version.new('3') do
it { expect(described_class::ALLOW).to eq('allow') }
it { expect(described_class::LOCATION).to eq('location') }
it { expect(described_class::TRANSFER_ENCODING).to eq('transfer-encoding') }
Expand Down
54 changes: 54 additions & 0 deletions spec/support/cookie_jar.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# frozen_string_literal: true

require 'uri'

module Rack
class MockResponse
def cookie_jar
@cookie_jar ||= Array(headers['Set-Cookie']).flat_map { |h| h.split("\n") }.map { |c| Cookie.new(c).to_h }
end

# https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie
class Cookie
attr_reader :attributes

def initialize(raw)
@attributes = raw.split(/;\s*/).flat_map.with_index do |attribute, i|
attribute, value = attribute.split('=', 2)
if i.zero?
[['name', attribute], ['value', unescape(value)]]
else
[[attribute.downcase, parse_value(attribute, value)]]
end
end.to_h.freeze
end

def to_h
@attributes.dup
end

def to_s
@attributes.to_s
end

private

def unescape(value)
URI.decode_www_form_component(value, Encoding::UTF_8)
end

def parse_value(attribute, value)
case attribute
when 'expires'
Time.parse(value)
when 'max-age'
value.to_i
when 'secure', 'httponly', 'partitioned'
true
else
unescape(value)
end
end
end
end
end

0 comments on commit 15c891c

Please sign in to comment.