Skip to content

Commit

Permalink
Rack 3.1 fixes (#2449)
Browse files Browse the repository at this point in the history
* Add Grape::Http::Headers::TRANSFER_ENCODING
Fix spec regarding content-length on stream

* Add CHANGELOG.md
  • Loading branch information
ericproulx committed Jun 14, 2024
1 parent 848e97a commit 1b4f510
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
fail-fast: false
matrix:
ruby: ['2.7', '3.0', '3.1', '3.2', '3.3']
gemfile: [Gemfile, gemfiles/rack_2_0.gemfile, gemfiles/rack_3_0.gemfile, gemfiles/rails_6_0.gemfile, gemfiles/rails_6_1.gemfile, gemfiles/rails_7_0.gemfile, gemfiles/rails_7_1.gemfile]
gemfile: [Gemfile, gemfiles/rack_2_0.gemfile, gemfiles/rack_3_0.gemfile, gemfiles/rack_3_1.gemfile, gemfiles/rails_6_0.gemfile, gemfiles/rails_6_1.gemfile, gemfiles/rails_7_0.gemfile, gemfiles/rails_7_1.gemfile]
specs: ['spec --exclude-pattern=spec/integration/**/*_spec.rb']
include:
- ruby: '2.7'
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
* [#2378](https://github.com/ruby-grape/grape/pull/2378): Do not overwrite `route_param` with a regular one if they share same name - [@arg](https://github.com/arg).
* [#2444](https://github.com/ruby-grape/grape/pull/2444): Replace method_missing in endpoint - [@ericproulx](https://github.com/ericproulx).
* [#2441](https://github.com/ruby-grape/grape/pull/2441): Optimize memory alloc and retained - [@ericproulx](https://github.com/ericproulx).
* [#2449](https://github.com/ruby-grape/grape/pull/2449): Rack 3.1 fixes - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.

### 2.0.0 (2023/11/11)
Expand Down
5 changes: 5 additions & 0 deletions gemfiles/rack_3_1.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# frozen_string_literal: true

eval_gemfile '../Gemfile'

gem 'rack', '~> 3.1'
2 changes: 1 addition & 1 deletion lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def stream(value = nil)
return if value.nil? && @stream.nil?

header Rack::CONTENT_LENGTH, nil
header Rack::TRANSFER_ENCODING, nil
header Grape::Http::Headers::TRANSFER_ENCODING, nil
header Rack::CACHE_CONTROL, 'no-cache' # Skips ETag generation (reading the response up front)
if value.is_a?(String)
file_body = Grape::ServeStream::FileBody.new(value)
Expand Down
1 change: 1 addition & 0 deletions lib/grape/http/headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module Headers
ALLOW = 'Allow'
LOCATION = 'Location'
X_CASCADE = 'X-Cascade'
TRANSFER_ENCODING = 'Transfer-Encoding'

SUPPORTED_METHODS = [
Rack::GET,
Expand Down
2 changes: 1 addition & 1 deletion spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1252,7 +1252,7 @@ def to_txt
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[Rack::TRANSFER_ENCODING]).to eq('chunked')
expect(last_response.headers[Grape::Http::Headers::TRANSFER_ENCODING]).to eq('chunked')

expect(last_response.body).to eq("c\r\nThis is some\r\nd\r\n file content\r\n0\r\n\r\n")
end
Expand Down
12 changes: 6 additions & 6 deletions spec/grape/dsl/inside_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ def initialize
before do
subject.header Rack::CACHE_CONTROL, 'cache'
subject.header Rack::CONTENT_LENGTH, 123
subject.header Rack::TRANSFER_ENCODING, 'base64'
subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64'
end

it 'sends no deprecation warnings' do
Expand Down Expand Up @@ -277,7 +277,7 @@ def initialize
it 'does not change the Transfer-Encoding header' do
subject.sendfile file_path

expect(subject.header[Rack::TRANSFER_ENCODING]).to eq 'base64'
expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to eq 'base64'
end
end

Expand Down Expand Up @@ -308,7 +308,7 @@ def initialize
before do
subject.header Rack::CACHE_CONTROL, 'cache'
subject.header Rack::CONTENT_LENGTH, 123
subject.header Rack::TRANSFER_ENCODING, 'base64'
subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64'
end

it 'emits no deprecation warnings' do
Expand Down Expand Up @@ -344,7 +344,7 @@ def initialize
it 'sets Transfer-Encoding header to nil' do
subject.stream file_path

expect(subject.header[Rack::TRANSFER_ENCODING]).to be_nil
expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to be_nil
end
end

Expand All @@ -358,7 +358,7 @@ def initialize
before do
subject.header Rack::CACHE_CONTROL, 'cache'
subject.header Rack::CONTENT_LENGTH, 123
subject.header Rack::TRANSFER_ENCODING, 'base64'
subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64'
end

it 'emits no deprecation warnings' do
Expand Down Expand Up @@ -388,7 +388,7 @@ def initialize
it 'sets Transfer-Encoding header to nil' do
subject.stream stream_object

expect(subject.header[Rack::TRANSFER_ENCODING]).to be_nil
expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to be_nil
end
end

Expand Down
9 changes: 8 additions & 1 deletion spec/grape/middleware/formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -453,12 +453,19 @@ def to_xml
let(:env) do
{ Rack::PATH_INFO => '/somewhere', Grape::Http::Headers::HTTP_ACCEPT => 'application/json' }
end
let(:headers) do
if Gem::Version.new(Rack.release) < Gem::Version.new('3.1')
{ Rack::CONTENT_TYPE => 'application/json', Rack::CONTENT_LENGTH => body.bytesize.to_s }
else
{ Rack::CONTENT_TYPE => 'application/json' }
end
end

it 'returns a file response' do
expect(file).to receive(:each).and_yield(body)
r = Rack::MockResponse[*subject.call(env)]
expect(r).to be_successful
expect(r.headers).to eq({ Rack::CONTENT_TYPE => 'application/json', Rack::CONTENT_LENGTH => body.bytesize.to_s })
expect(r.headers).to eq(headers)
expect(r.body).to eq('data')
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/support/chunked_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ def call(env)

if !Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.key?(status.to_i) &&
!headers[Rack::CONTENT_LENGTH] &&
!headers[Rack::TRANSFER_ENCODING]
!headers[Grape::Http::Headers::TRANSFER_ENCODING]

headers[Rack::TRANSFER_ENCODING] = 'chunked'
headers[Grape::Http::Headers::TRANSFER_ENCODING] = 'chunked'
response[2] = if headers['trailer']
TrailerBody.new(body)
else
Expand Down

0 comments on commit 1b4f510

Please sign in to comment.