Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Grape allowing invalid headers to be set #2511

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* [#2506](https://github.com/ruby-grape/grape/pull/2506): Fix fetch_formatter api_format - [@ericproulx](https://github.com/ericproulx).
* [#2507](https://github.com/ruby-grape/grape/pull/2507): Fix type: Set with values - [@nikolai-b](https://github.com/nikolai-b).
* [#2510](https://github.com/ruby-grape/grape/pull/2510): Fix ContractScope's validator inheritance - [@ericproulx](https://github.com/ericproulx).
* [#2511](https://github.com/ruby-grape/grape/pull/2511): Convert all header values to strings - [@SlakrHakr](https://github.com/SlakrHakr).
* Your contribution here.

### 2.2.0 (2024-09-14)
Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
- [Using dry-validation or dry-schema](#using-dry-validation-or-dry-schema)
- [Headers](#headers)
- [Request](#request)
- [Header Value Types](#header-value-types)
- [Header Case Handling](#header-case-handling)
- [Response](#response)
- [Routes](#routes)
Expand Down Expand Up @@ -2210,6 +2211,10 @@ get do
end
```

#### Header Value Types

All header values are converted to strings to conform with the [rack specification](https://github.com/rack/rack/blob/main/SPEC.rdoc#the-headers-).

#### Header Case Handling

The above example may have been requested as follows:
Expand Down
5 changes: 5 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ Upgrading Grape

### Upgrading to >= 2.3.0

### Header values forced into string with warning in log
Before 2.3.0 header values retained their type (e.g.: int, boolean, etc.)
This functionality has been altered to log a warning message about that a value is not a string and then force the value into a string for the rest of the request.
See [#2511](https://github.com/ruby-grape/grape/pull/2511) for more information.

### `content_type` vs `api.format` inside API

Before 2.3.0, `content_type` had priority over `env['api.format']` when set in an API, which was incorrect. The priority has been flipped and `env['api.format']` will be checked first.
Expand Down
7 changes: 6 additions & 1 deletion lib/grape/dsl/headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ module Headers
# 4. Delete a specifc header key-value pair
def header(key = nil, val = nil)
if key
val ? header[key.to_s] = val : header.delete(key.to_s)
if val
warn "Header value for '#{key}' is not a string. Converting to string." unless val.is_a?(String)
header[key.to_s] = val.to_s
else
header.delete(key.to_s)
end
else
@header ||= Grape::Util::Header.new
end
Expand Down
13 changes: 13 additions & 0 deletions spec/grape/dsl/headers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,17 @@
end
end
end

context 'when non-string headers are set' do
describe '#header' do
it 'converts non-string header values to strings' do
subject.header('integer key', 123)
expect(subject.header['integer key']).to eq '123'
end

it 'emits a warning if the header value is not a string' do
expect { subject.header('integer key', 123) }.to output("Header value for 'integer key' is not a string. Converting to string.\n").to_stderr
end
end
end
end
2 changes: 1 addition & 1 deletion spec/grape/dsl/inside_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def initialize
it 'set the correct headers' do
expect(subject.header).to match(
Rack::CACHE_CONTROL => 'cache',
Rack::CONTENT_LENGTH => 123,
Rack::CONTENT_LENGTH => '123',
Grape::Http::Headers::TRANSFER_ENCODING => 'base64'
)
end
Expand Down