Skip to content

Commit

Permalink
Set response headers based on Rack version (#2355)
Browse files Browse the repository at this point in the history
* Set response headers based on Rack version
* Use Rack.release instead of Rack::RELEASE
* Bumped to 1.9.0 and updated UPGRADING.md
* Added .rack3? method
* Removed headers_helper
* Updated README and UPGRADING
  • Loading branch information
schinery authored Oct 24, 2023
1 parent 51b081c commit 2f62365
Show file tree
Hide file tree
Showing 21 changed files with 187 additions and 82 deletions.
10 changes: 10 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ jobs:
if: ${{ matrix.gemfile == 'multi_xml' }}
run: bundle exec rspec spec/integration/multi_xml

- name: Run tests (spec/integration/rack/v2)
# rack_2_0.gemfile is equals to Gemfile
if: ${{ matrix.gemfile == 'rack_2_0' }}
run: bundle exec rspec spec/integration/rack/v2

- name: Run tests (spec/integration/rack/v3)
# rack_2_0.gemfile is equals to Gemfile
if: ${{ matrix.gemfile == 'rack_3_0' }}
run: bundle exec rspec spec/integration/rack/v3

- name: Coveralls
uses: coverallsapp/github-action@master
with:
Expand Down
2 changes: 2 additions & 0 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@ RSpec/FilePath:
- 'spec/integration/eager_load/eager_load_spec.rb'
- 'spec/integration/multi_json/json_spec.rb'
- 'spec/integration/multi_xml/xml_spec.rb'
- 'spec/integration/rack/v2/headers_spec.rb'
- 'spec/integration/rack/v3/headers_spec.rb'

# Offense count: 12
# Configuration parameters: Max.
Expand Down
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
### 1.8.1 (Next)
### 1.9.0 (Next)

#### Features

* [#2353](https://github.com/ruby-grape/grape/pull/2353): Added Rails 7.1 support - [@ericproulx](https://github.com/ericproulx).
* [#2355](https://github.com/ruby-grape/grape/pull/2355): Set response headers based on Rack version - [@schinery](https://github.com/schinery).
* [#2360](https://github.com/ruby-grape/grape/pull/2360): Reduce gem size by removing specs - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.

#### Fixes

* Your contribution here.
Expand Down
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ content negotiation, versioning and much more.

## Stable Release

You're reading the documentation for the next release of Grape, which should be **1.8.1**.
You're reading the documentation for the next release of Grape, which should be **1.9.0**.
Please read [UPGRADING](UPGRADING.md) when upgrading from a previous version.
The current stable release is [1.8.0](https://github.com/ruby-grape/grape/blob/v1.8.0/README.md).

Expand Down Expand Up @@ -2130,8 +2130,9 @@ curl -H "secret_PassWord: swordfish" ...

The header name will have been normalized for you.

- In the `header` helper names will be coerced into a capitalized kebab case.
- In the `env` collection they appear in all uppercase, in snake case, and prefixed with 'HTTP_'.
- In the `header` helper names will be coerced into a downcased kebab case as `secret-password` if using Rack 3.
- In the `header` helper names will be coerced into a capitalized kebab case as `Secret-PassWord` if using Rack < 3.
- In the `env` collection they appear in all uppercase, in snake case, and prefixed with 'HTTP_' as `HTTP_SECRET_PASSWORD`

The header name will have been normalized per HTTP standards defined in [RFC2616 Section 4.2](https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2) regardless of what is being sent by a client.

Expand Down
29 changes: 29 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,35 @@
Upgrading Grape
===============

### Upgrading to >= 1.9.0

#### Headers

As per [rack/rack#1592](https://github.com/rack/rack/issues/1592) Rack 3.0 is enforcing the HTTP/2 semantics, and thus treats all headers as lowercase. Starting with Grape 1.9.0, headers will be cased based on what version of Rack you are using.

Given this request:

```shell
curl -H "Content-Type: application/json" -H "Secret-Password: foo" ...
```

If you are using Rack 3 in your application then the headers will be set to:

```ruby
{ "content-type" => "application/json", "secret-password" => "foo"}
```

This means if you are checking for header values in your application, you would need to change your code to use downcased keys.

```ruby
get do
# This would use headers['Secret-Password'] in Rack < 3
error!('Unauthorized', 401) unless headers['secret-password'] == 'swordfish'
end
```

See [#2355](https://github.com/ruby-grape/grape/pull/2355) for more information.

### Upgrading to >= 1.7.0

#### Exceptions renaming
Expand Down
4 changes: 4 additions & 0 deletions lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ def self.deprecator
@deprecator ||= ActiveSupport::Deprecation.new('2.0', 'Grape')
end

def self.rack3?
Gem::Version.new(::Rack.release) >= Gem::Version.new('3')
end

eager_autoload do
autoload :API
autoload :Endpoint
Expand Down
8 changes: 4 additions & 4 deletions lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def redirect(url, permanent: false, body: nil, **_options)
status 302
body_message ||= "This resource has been moved temporarily to #{url}."
end
header 'Location', url
header Grape::Http::Headers::LOCATION, url
content_type 'text/plain'
body body_message
end
Expand Down Expand Up @@ -328,9 +328,9 @@ def sendfile(value = nil)
def stream(value = nil)
return if value.nil? && @stream.nil?

header 'Content-Length', nil
header 'Transfer-Encoding', nil
header 'Cache-Control', 'no-cache' # Skips ETag generation (reading the response up front)
header Grape::Http::Headers::CONTENT_LENGTH, nil
header Grape::Http::Headers::TRANSFER_ENCODING, nil
header Grape::Http::Headers::CACHE_CONTROL, 'no-cache' # Skips ETag generation (reading the response up front)
if value.is_a?(String)
file_body = Grape::ServeStream::FileBody.new(value)
@stream = Grape::ServeStream::StreamResponse.new(file_body)
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def run
if (allowed_methods = env[Grape::Env::GRAPE_ALLOWED_METHODS])
raise Grape::Exceptions::MethodNotAllowed.new(header.merge('Allow' => allowed_methods)) unless options?

header 'Allow', allowed_methods
header Grape::Http::Headers::ALLOW, allowed_methods
response_object = ''
status 204
else
Expand Down
20 changes: 18 additions & 2 deletions lib/grape/http/headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,24 @@ module Headers
PATH_INFO = 'PATH_INFO'
REQUEST_METHOD = 'REQUEST_METHOD'
QUERY_STRING = 'QUERY_STRING'
CONTENT_TYPE = 'Content-Type'

if Grape.rack3?
ALLOW = 'allow'
CACHE_CONTROL = 'cache-control'
CONTENT_LENGTH = 'content-length'
CONTENT_TYPE = 'content-type'
LOCATION = 'location'
TRANSFER_ENCODING = 'transfer-encoding'
X_CASCADE = 'x-cascade'
else
ALLOW = 'Allow'
CACHE_CONTROL = 'Cache-Control'
CONTENT_LENGTH = 'Content-Length'
CONTENT_TYPE = 'Content-Type'
LOCATION = 'Location'
TRANSFER_ENCODING = 'Transfer-Encoding'
X_CASCADE = 'X-Cascade'
end

GET = 'GET'
POST = 'POST'
Expand All @@ -24,7 +41,6 @@ module Headers
SUPPORTED_METHODS_WITHOUT_OPTIONS = Grape::Util::LazyObject.new { [GET, POST, PUT, PATCH, DELETE, HEAD].freeze }

HTTP_ACCEPT_VERSION = 'HTTP_ACCEPT_VERSION'
X_CASCADE = 'X-Cascade'
HTTP_TRANSFER_ENCODING = 'HTTP_TRANSFER_ENCODING'
HTTP_ACCEPT = 'HTTP_ACCEPT'

Expand Down
10 changes: 8 additions & 2 deletions lib/grape/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,14 @@ def build_headers
end
end

def transform_header(header)
-header[5..].split('_').each(&:capitalize!).join('-')
if Grape.rack3?
def transform_header(header)
-header[5..].tr('_', '-').downcase
end
else
def transform_header(header)
-header[5..].split('_').map(&:capitalize).join('-')
end
end
end
end
2 changes: 1 addition & 1 deletion lib/grape/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

module Grape
# The current version of Grape.
VERSION = '1.8.1'
VERSION = '1.9.0'
end
12 changes: 9 additions & 3 deletions spec/grape/api/custom_validations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,19 @@ def validate(request)
return unless request.params.key? @attrs.first
# check if admin flag is set to true
return unless @option

# check if user is admin or not
# as an example get a token from request and check if it's admin or not
raise Grape::Exceptions::Validation.new(params: @attrs, message: 'Can not set Admin only field.') unless request.headers['X-Access-Token'] == 'admin'
raise Grape::Exceptions::Validation.new(params: @attrs, message: 'Can not set Admin only field.') unless request.headers[access_header] == 'admin'
end

def access_header
Grape.rack3? ? 'x-access-token' : 'X-Access-Token'
end
end
end
let(:app) { Rack::Builder.new(subject) }
let(:x_access_token_header) { Grape.rack3? ? 'x-access-token' : 'X-Access-Token' }

before do
described_class.register_validator('admin', admin_validator)
Expand Down Expand Up @@ -197,14 +203,14 @@ def validate(request)
end

it 'does not fail when we send admin fields and we are admin' do
header 'X-Access-Token', 'admin'
header x_access_token_header, 'admin'
get '/', admin_field: 'tester', non_admin_field: 'toaster', admin_false_field: 'test'
expect(last_response.status).to eq 200
expect(last_response.body).to eq 'bacon'
end

it 'fails when we send admin fields and we are not admin' do
header 'X-Access-Token', 'user'
header x_access_token_header, 'user'
get '/', admin_field: 'tester', non_admin_field: 'toaster', admin_false_field: 'test'
expect(last_response.status).to eq 400
expect(last_response.body).to include 'Can not set Admin only field.'
Expand Down
Loading

0 comments on commit 2f62365

Please sign in to comment.