From c3922841c560c3342040989229b46c93d67f2493 Mon Sep 17 00:00:00 2001 From: Lee DeBoom <31224301+SlakrHakr@users.noreply.github.com> Date: Mon, 4 Nov 2024 17:25:36 -0600 Subject: [PATCH 1/8] Fix Grape allowing invalid headers to be set Fixes #2334 Ensure all header values are strings according to the Rack spec. * Convert header values to strings using `to_s` in the `header` method in `lib/grape/dsl/headers.rb`. * Emit a warning if the header value is not a string in the `header` method in `lib/grape/dsl/headers.rb`. * Add tests in `spec/grape/dsl/headers_spec.rb` to verify that non-string header values are converted to strings and warnings are emitted. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/ruby-grape/grape/issues/2334?shareId=XXXX-XXXX-XXXX-XXXX). --- lib/grape/dsl/headers.rb | 9 ++++++++- spec/grape/dsl/headers_spec.rb | 13 +++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/grape/dsl/headers.rb b/lib/grape/dsl/headers.rb index a02bdd588e..710edaa9ee 100644 --- a/lib/grape/dsl/headers.rb +++ b/lib/grape/dsl/headers.rb @@ -10,7 +10,14 @@ 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 + unless val.is_a?(String) + warn "Header value for '#{key}' is not a string. Converting to string." + end + header[key.to_s] = val.to_s + else + header.delete(key.to_s) + end else @header ||= Grape::Util::Header.new end diff --git a/spec/grape/dsl/headers_spec.rb b/spec/grape/dsl/headers_spec.rb index 1502176bd9..042e94e776 100644 --- a/spec/grape/dsl/headers_spec.rb +++ b/spec/grape/dsl/headers_spec.rb @@ -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 From 7f5e53aef29c360ff77bf7790981e01c8a5270fa Mon Sep 17 00:00:00 2001 From: Lee DeBoom Date: Thu, 14 Nov 2024 18:31:52 -0600 Subject: [PATCH 2/8] appease rubocop --- lib/grape/dsl/headers.rb | 4 +--- lib/grape/endpoint.rb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/grape/dsl/headers.rb b/lib/grape/dsl/headers.rb index 710edaa9ee..1d39c9ead8 100644 --- a/lib/grape/dsl/headers.rb +++ b/lib/grape/dsl/headers.rb @@ -11,9 +11,7 @@ module Headers def header(key = nil, val = nil) if key if val - unless val.is_a?(String) - warn "Header value for '#{key}' is not a string. Converting to string." - end + 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) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 6bd72c23c9..9705084725 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -417,4 +417,4 @@ def build_helpers Module.new { helpers.each { |mod_to_include| include mod_to_include } } end end -end +end \ No newline at end of file From 6ffceb59a94c58f52d97cb736baa103782d4f509 Mon Sep 17 00:00:00 2001 From: Lee DeBoom Date: Thu, 14 Nov 2024 18:31:59 -0600 Subject: [PATCH 3/8] fix test --- spec/grape/dsl/inside_route_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index ea8d6d9875..5ce3ec4d3e 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -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 From 159a2df6848d980b5210339601b3c58f5f19f86e Mon Sep 17 00:00:00 2001 From: Lee DeBoom Date: Thu, 14 Nov 2024 17:33:16 -0800 Subject: [PATCH 4/8] Update endpoint.rb --- lib/grape/endpoint.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 9705084725..6bd72c23c9 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -417,4 +417,4 @@ def build_helpers Module.new { helpers.each { |mod_to_include| include mod_to_include } } end end -end \ No newline at end of file +end From d18cdfc4103c8341342b5ac7a6b5c8d7e93d071c Mon Sep 17 00:00:00 2001 From: Lee DeBoom Date: Thu, 14 Nov 2024 17:33:55 -0800 Subject: [PATCH 5/8] Update endpoint.rb From d3ed93aa4ae84f5a9515dcc6c28b63d0f614ce70 Mon Sep 17 00:00:00 2001 From: Lee DeBoom Date: Thu, 14 Nov 2024 19:52:41 -0600 Subject: [PATCH 6/8] add documentation --- CHANGELOG.md | 1 + README.md | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a15293fb1..68557cc729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/README.md b/README.md index 0c6c56aff4..60b0845a35 100644 --- a/README.md +++ b/README.md @@ -2210,6 +2210,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: From fa8eb12d9e2810ddcaf78c1876be92f2d2e2ace4 Mon Sep 17 00:00:00 2001 From: Lee DeBoom Date: Fri, 15 Nov 2024 16:27:53 -0600 Subject: [PATCH 7/8] add section to table of contents --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 60b0845a35..a567192cd8 100644 --- a/README.md +++ b/README.md @@ -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) From 761a7e6b551cb1aa17f65fca0090ec84936a1a4a Mon Sep 17 00:00:00 2001 From: Lee DeBoom Date: Fri, 15 Nov 2024 16:34:13 -0600 Subject: [PATCH 8/8] added info to upgrading doc --- UPGRADING.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/UPGRADING.md b/UPGRADING.md index 0c6c54f6ed..1be08036ae 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -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.