From 342c912dacccea1b36c5b9af4a4e322950a348fd Mon Sep 17 00:00:00 2001 From: Eric Date: Thu, 4 Jan 2024 21:54:14 +0100 Subject: [PATCH 1/6] Fix #2403 --- .github/workflows/edge.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/edge.yml b/.github/workflows/edge.yml index c617da14e1..cfa629b0b8 100644 --- a/.github/workflows/edge.yml +++ b/.github/workflows/edge.yml @@ -7,7 +7,12 @@ jobs: fail-fast: false matrix: ruby: ['2.7', '3.0', '3.1', '3.2', '3.3', ruby-head, truffleruby-head, jruby-head] - gemfile: [rails_edge, rack_edge, rack_3_0] + gemfile: [rails_edge, rack_edge] + exclude: + - ruby: '2.7' + gemfile: rails_edge + - ruby: '3.0' + gemfile: rails_edge runs-on: ubuntu-latest continue-on-error: true env: From 9983343c42daa2602a84b9ddd273d314a20b1209 Mon Sep 17 00:00:00 2001 From: Eric Date: Fri, 5 Jan 2024 22:30:39 +0100 Subject: [PATCH 2/6] Fix #2404 Replace last_response.headers[Rack::CONTENT_TYPE] by last_response.content_type Replace last_response.headers['Location'] by last_response.content_type Replace last_response.headers[Rack::CONTENT_LENGTH] by last_response.content_type --- spec/grape/api_spec.rb | 38 ++++++------ spec/grape/endpoint_spec.rb | 8 +-- spec/grape/entity_spec.rb | 6 +- spec/grape/middleware/base_spec.rb | 4 +- spec/support/basic_auth_encode_helpers.rb | 2 + spec/support/chunked_response.rb | 74 +++++++++++++++++++++++ 6 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 spec/support/chunked_response.rb diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 243c6cb9ed..c195c081ca 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -688,7 +688,7 @@ class DummyFormatClass 'example' end put '/example' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eql 'text/plain' + expect(last_response.content_type).to eql 'text/plain' end describe 'adds an OPTIONS route that' do @@ -1195,7 +1195,7 @@ class DummyFormatClass it 'sets content type for txt format' do get '/foo' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain') + expect(last_response.content_type).to eq('text/plain') end it 'does not set Cache-Control' do @@ -1205,22 +1205,22 @@ class DummyFormatClass it 'sets content type for xml' do get '/foo.xml' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('application/xml') + expect(last_response.content_type).to eq('application/xml') end it 'sets content type for json' do get '/foo.json' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('application/json') + expect(last_response.content_type).to eq('application/json') end it 'sets content type for serializable hash format' do get '/foo.serializable_hash' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('application/json') + expect(last_response.content_type).to eq('application/json') end it 'sets content type for binary format' do get '/foo.binary' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('application/octet-stream') + expect(last_response.content_type).to eq('application/octet-stream') end it 'returns raw data when content type binary' do @@ -1229,7 +1229,7 @@ class DummyFormatClass subject.format :binary subject.get('/binary_file') { File.binread(image_filename) } get '/binary_file' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('application/octet-stream') + expect(last_response.content_type).to eq('application/octet-stream') expect(last_response.body).to eq(file) end @@ -1241,8 +1241,8 @@ class DummyFormatClass subject.get('/file') { stream test_file } get '/file' - expect(last_response.headers[Rack::CONTENT_LENGTH]).to eq('25') - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain') + expect(last_response.content_length).to eq(25) + expect(last_response.content_type).to eq('text/plain') expect(last_response.body).to eq(file_content) end @@ -1252,12 +1252,12 @@ class DummyFormatClass blk.yield ' file content' end - subject.use Rack::Chunked + subject.use defined?(Rack::Chunked) ? Rack::Chunked : ChunkedResponse subject.get('/stream') { stream test_stream } get '/stream', {}, 'HTTP_VERSION' => 'HTTP/1.1', 'SERVER_PROTOCOL' => 'HTTP/1.1' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain') - expect(last_response.headers[Rack::CONTENT_LENGTH]).to be_nil + expect(last_response.content_type).to eq('text/plain') + expect(last_response.content_length).to be_nil expect(last_response.headers[Rack::CACHE_CONTROL]).to eq('no-cache') expect(last_response.headers[Grape::Http::Headers::TRANSFER_ENCODING]).to eq('chunked') @@ -1267,7 +1267,7 @@ class DummyFormatClass it 'sets content type for error' do subject.get('/error') { error!('error in plain text', 500) } get '/error' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eql 'text/plain' + expect(last_response.content_type).to eql 'text/plain' end it 'sets content type for json error' do @@ -1275,7 +1275,7 @@ class DummyFormatClass subject.get('/error') { error!('error in json', 500) } get '/error.json' expect(last_response.status).to be 500 - expect(last_response.headers[Rack::CONTENT_TYPE]).to eql 'application/json' + expect(last_response.content_type).to eql 'application/json' end it 'sets content type for xml error' do @@ -1283,7 +1283,7 @@ class DummyFormatClass subject.get('/error') { error!('error in xml', 500) } get '/error' expect(last_response.status).to be 500 - expect(last_response.headers[Rack::CONTENT_TYPE]).to eql 'application/xml' + expect(last_response.content_type).to eql 'application/xml' end it 'includes extension in format' do @@ -1313,12 +1313,12 @@ class DummyFormatClass it 'sets content type' do get '/custom.custom' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eql 'application/custom' + expect(last_response.content_type).to eql 'application/custom' end it 'sets content type for error' do get '/error.custom' - expect(last_response.headers[Rack::CONTENT_TYPE]).to eql 'application/custom' + expect(last_response.content_type).to eql 'application/custom' end end @@ -1338,7 +1338,7 @@ class DummyFormatClass image_filename = 'grape.png' post url, file: Rack::Test::UploadedFile.new(image_filename, 'image/png', true) expect(last_response.status).to eq(201) - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('image/png') + expect(last_response.content_type).to eq('image/png') expect(last_response.headers['Content-Disposition']).to eq("attachment; filename*=UTF-8''grape.png") File.open(image_filename, 'rb') do |io| expect(last_response.body).to eq io.read @@ -1350,7 +1350,7 @@ class DummyFormatClass filename = __FILE__ post '/attachment.rb', file: Rack::Test::UploadedFile.new(filename, 'application/x-ruby', true) expect(last_response.status).to eq(201) - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('application/x-ruby') + expect(last_response.content_type).to eq('application/x-ruby') expect(last_response.headers['Content-Disposition']).to eq("attachment; filename*=UTF-8''api_spec.rb") File.open(filename, 'rb') do |io| expect(last_response.body).to eq io.read diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 94d0443c37..77dd116c00 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -490,7 +490,7 @@ def app end it 'responses with given content type in headers' do - expect(last_response.headers[Rack::CONTENT_TYPE]).to eq 'application/json; charset=utf-8' + expect(last_response.content_type).to eq 'application/json; charset=utf-8' end end @@ -650,7 +650,7 @@ def app end get '/hey' expect(last_response.status).to eq 302 - expect(last_response.headers['Location']).to eq '/ha' + expect(last_response.location).to eq '/ha' expect(last_response.body).to eq 'This resource has been moved temporarily to /ha.' end @@ -660,7 +660,7 @@ def app end post '/hey', {}, 'HTTP_VERSION' => 'HTTP/1.1' expect(last_response.status).to eq 303 - expect(last_response.headers['Location']).to eq '/ha' + expect(last_response.location).to eq '/ha' expect(last_response.body).to eq 'An alternate resource is located at /ha.' end @@ -670,7 +670,7 @@ def app end get '/hey' expect(last_response.status).to eq 301 - expect(last_response.headers['Location']).to eq '/ha' + expect(last_response.location).to eq '/ha' expect(last_response.body).to eq 'This resource has been moved permanently to /ha.' end diff --git a/spec/grape/entity_spec.rb b/spec/grape/entity_spec.rb index 425edf0031..7eaa0fae87 100644 --- a/spec/grape/entity_spec.rb +++ b/spec/grape/entity_spec.rb @@ -236,7 +236,7 @@ def initialize(args) end get '/example' expect(last_response.status).to eq(200) - expect(last_response.headers['Content-type']).to eq('application/xml') + expect(last_response.content_type).to eq('application/xml') expect(last_response.body).to eq <<~XML @@ -266,7 +266,7 @@ def initialize(args) end get '/example' expect(last_response.status).to eq(200) - expect(last_response.headers['Content-type']).to eq('application/json') + expect(last_response.content_type).to eq('application/json') expect(last_response.body).to eq('{"example":{"name":"johnnyiller"}}') end @@ -298,7 +298,7 @@ def initialize(args) get '/example?callback=abcDef' expect(last_response.status).to eq(200) - expect(last_response.headers['Content-type']).to eq('application/javascript') + expect(last_response.content_type).to eq('application/javascript') expect(last_response.body).to include 'abcDef({"example":{"name":"johnnyiller"}})' end diff --git a/spec/grape/middleware/base_spec.rb b/spec/grape/middleware/base_spec.rb index cbba974e1a..3be95be1f5 100644 --- a/spec/grape/middleware/base_spec.rb +++ b/spec/grape/middleware/base_spec.rb @@ -93,7 +93,7 @@ end it 'header' do - expect(subject.response.header).to have_key(:abc) + expect(subject.response.headers).to have_key(:abc) end it 'returns the memoized Rack::Response instance' do @@ -115,7 +115,7 @@ end it 'header' do - expect(subject.response.header).to have_key(:abc) + expect(subject.response.headers).to have_key(:abc) end it 'returns the memoized Rack::Response instance' do diff --git a/spec/support/basic_auth_encode_helpers.rb b/spec/support/basic_auth_encode_helpers.rb index 78e21e6c80..00c3c61497 100644 --- a/spec/support/basic_auth_encode_helpers.rb +++ b/spec/support/basic_auth_encode_helpers.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'base64' + module Spec module Support module Helpers diff --git a/spec/support/chunked_response.rb b/spec/support/chunked_response.rb new file mode 100644 index 0000000000..8cc71dc3b5 --- /dev/null +++ b/spec/support/chunked_response.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +# this is a copy of Rack::Chunked which has been removed in rack > 3.0 + +class ChunkedResponse + class Body + TERM = "\r\n" + TAIL = "0#{TERM}" + + # Store the response body to be chunked. + def initialize(body) + @body = body + end + + # For each element yielded by the response body, yield + # the element in chunked encoding. + def each(&block) + term = TERM + @body.each do |chunk| + size = chunk.bytesize + next if size == 0 + + yield [size.to_s(16), term, chunk.b, term].join + end + yield TAIL + yield_trailers(&block) + yield term + end + + # Close the response body if the response body supports it. + def close + @body.close if @body.respond_to?(:close) + end + + private + + # Do nothing as this class does not support trailer headers. + def yield_trailers + end + end + + class TrailerBody < Body + private + + # Yield strings for each trailer header. + def yield_trailers + @body.trailers.each_pair do |k, v| + yield "#{k}: #{v}\r\n" + end + end + end + + def initialize(app) + @app = app + end + + def call(env) + status, headers, body = response = @app.call(env) + + if !Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.key?(status.to_i) && + !headers[Rack::CONTENT_LENGTH] && + !headers[Rack::TRANSFER_ENCODING] + + headers[Rack::TRANSFER_ENCODING] = 'chunked' + if headers['trailer'] + response[2] = TrailerBody.new(body) + else + response[2] = Body.new(body) + end + end + + response + end +end From 63c69100d9f0d803f9ba52d09f47d0d0880f207e Mon Sep 17 00:00:00 2001 From: Eric Date: Fri, 5 Jan 2024 22:31:35 +0100 Subject: [PATCH 3/6] Add CHANGELOG.md entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43ff91251f..b7b048c1d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ * [#2373](https://github.com/ruby-grape/grape/pull/2373): Fix markdown files for following 1-line format - [@jcagarcia](https://github.com/jcagarcia). * [#2382](https://github.com/ruby-grape/grape/pull/2382): Fix values validator for params wrapped in `with` block - [@numbata](https://github.com/numbata). * [#2387](https://github.com/ruby-grape/grape/pull/2387): Fix rubygems version within workflows - [@ericproulx](https://github.com/ericproulx). +* [#2405](https://github.com/ruby-grape/grape/pull/2405): Fix edge workflows - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 2.0.0 (2023/11/11) From cbe85b1bbfbc760875746bc8b067dc4af8624913 Mon Sep 17 00:00:00 2001 From: Eric Date: Fri, 5 Jan 2024 22:34:13 +0100 Subject: [PATCH 4/6] Fix rubocop --- spec/support/chunked_response.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/spec/support/chunked_response.rb b/spec/support/chunked_response.rb index 8cc71dc3b5..41defe87e6 100644 --- a/spec/support/chunked_response.rb +++ b/spec/support/chunked_response.rb @@ -35,8 +35,7 @@ def close private # Do nothing as this class does not support trailer headers. - def yield_trailers - end + def yield_trailers; end end class TrailerBody < Body @@ -58,15 +57,15 @@ def call(env) status, headers, body = response = @app.call(env) if !Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.key?(status.to_i) && - !headers[Rack::CONTENT_LENGTH] && - !headers[Rack::TRANSFER_ENCODING] + !headers[Rack::CONTENT_LENGTH] && + !headers[Rack::TRANSFER_ENCODING] headers[Rack::TRANSFER_ENCODING] = 'chunked' - if headers['trailer'] - response[2] = TrailerBody.new(body) - else - response[2] = Body.new(body) - end + response[2] = if headers['trailer'] + TrailerBody.new(body) + else + Body.new(body) + end end response From 26eb421729df51bd81e3ff3b2d77812e549b6376 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Fri, 5 Jan 2024 22:38:19 +0100 Subject: [PATCH 5/6] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7b048c1d2..333f3112b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ * [#2373](https://github.com/ruby-grape/grape/pull/2373): Fix markdown files for following 1-line format - [@jcagarcia](https://github.com/jcagarcia). * [#2382](https://github.com/ruby-grape/grape/pull/2382): Fix values validator for params wrapped in `with` block - [@numbata](https://github.com/numbata). * [#2387](https://github.com/ruby-grape/grape/pull/2387): Fix rubygems version within workflows - [@ericproulx](https://github.com/ericproulx). -* [#2405](https://github.com/ruby-grape/grape/pull/2405): Fix edge workflows - [@ericproulx](https://github.com/ericproulx). +* [#2405](https://github.com/ruby-grape/grape/pull/2405): Fix edge workflow - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 2.0.0 (2023/11/11) From 75c242342bc6a8aa2e05b896a0e242b49cf3dc34 Mon Sep 17 00:00:00 2001 From: Eric Date: Sat, 6 Jan 2024 21:46:31 +0100 Subject: [PATCH 6/6] Remove Rack::Chunked deprecation --- spec/grape/api_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index be02ae30aa..97c2dbff07 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1252,7 +1252,7 @@ class DummyFormatClass blk.yield ' file content' end - subject.use defined?(Rack::Chunked) ? Rack::Chunked : ChunkedResponse + subject.use Gem::Version.new(Rack.release) < Gem::Version.new('3') ? Rack::Chunked : ChunkedResponse subject.get('/stream') { stream test_stream } get '/stream', {}, 'HTTP_VERSION' => 'HTTP/1.1', 'SERVER_PROTOCOL' => 'HTTP/1.1'