Skip to content

Commit

Permalink
Validators bad encoding (#2524)
Browse files Browse the repository at this point in the history
* Use scrub when possible
Add specs for bad encoding.

* Fix rubocop
Add CHANGELOG.md entry
  • Loading branch information
ericproulx authored Jan 20, 2025
1 parent 926c28d commit 5ce44de
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* [#2510](https://github.com/ruby-grape/grape/pull/2510): Fix ContractScope's validator inheritance - [@ericproulx](https://github.com/ericproulx).
* [#2521](https://github.com/ruby-grape/grape/pull/2521): Fixed typo in README - [@datpmt](https://github.com/datpmt).
* [#2525](https://github.com/ruby-grape/grape/pull/2525): Require logger before active_support - [@ericproulx](https://github.com/ericproulx).
* [#2524](https://github.com/ruby-grape/grape/pull/2524): Fix validators bad encoding - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.

### 2.2.0 (2024-09-14)
Expand Down
4 changes: 3 additions & 1 deletion lib/grape/middleware/versioner/accept_version_header.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ module Versioner
# route.
class AcceptVersionHeader < Base
def before
potential_version = env[Grape::Http::Headers::HTTP_ACCEPT_VERSION]&.strip
potential_version = env[Grape::Http::Headers::HTTP_ACCEPT_VERSION]
potential_version = potential_version.scrub unless potential_version.nil?

not_acceptable!('Accept-Version header must be set.') if strict? && potential_version.blank?

return if potential_version.blank?
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/validations/validators/allow_blank_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def validate_param!(attr_name, params)
return if (options_key?(:value) ? @option[:value] : @option) || !params.is_a?(Hash)

value = params[attr_name]
value = value.strip if value.respond_to?(:strip)
value = value.scrub if value.respond_to?(:scrub)

return if value == false || value.present?

Expand Down
2 changes: 1 addition & 1 deletion lib/grape/validations/validators/regexp_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Validators
class RegexpValidator < Base
def validate_param!(attr_name, params)
return unless params.respond_to?(:key?) && params.key?(attr_name)
return if Array.wrap(params[attr_name]).all? { |param| param.nil? || param.to_s.match?((options_key?(:value) ? @option[:value] : @option)) }
return if Array.wrap(params[attr_name]).all? { |param| param.nil? || param.to_s.scrub.match?((options_key?(:value) ? @option[:value] : @option)) }

raise Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: message(:regexp))
end
Expand Down
2 changes: 2 additions & 0 deletions lib/grape/validations/validators/values_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ def validate_param!(attr_name, params)

return if val.nil? && !required_for_root_scope?

val = val.scrub if val.respond_to?(:scrub)

# don't forget that +false.blank?+ is true
return if val != false && val.blank? && @allow_blank

Expand Down
12 changes: 12 additions & 0 deletions spec/grape/middleware/versioner/accept_version_header_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@
}
end

describe '#bad encoding' do
before do
@options[:versions] = %w[v1]
end

it 'does not raise an error' do
expect do
subject.call(Grape::Http::Headers::HTTP_ACCEPT_VERSION => "\x80")
end.to throw_symbol(:error, status: 406, headers: { Grape::Http::Headers::X_CASCADE => 'pass' }, message: 'The requested version is not supported.')
end
end

context 'api.version' do
before do
@options[:versions] = ['v1']
Expand Down
19 changes: 19 additions & 0 deletions spec/grape/validations/validators/allow_blank_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,25 @@
end
end

describe 'bad encoding' do
let(:app) do
Class.new(Grape::API) do
default_format :json

params do
requires :name, type: String, allow_blank: false
end
get '/bad_encoding'
end
end

context 'when value has bad encoding' do
it 'does not raise an error' do
expect { get('/bad_encoding', { name: "Hello \x80" }) }.not_to raise_error
end
end
end

context 'invalid input' do
it 'refuses empty string' do
get '/disallow_blank', name: ''
Expand Down
19 changes: 19 additions & 0 deletions spec/grape/validations/validators/regexp_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,25 @@
end
end

describe '#bad encoding' do
let(:app) do
Class.new(Grape::API) do
default_format :json

params do
requires :name, regexp: { value: /^[a-z]+$/ }
end
get '/bad_encoding'
end
end

context 'when value as bad encoding' do
it 'does not raise an error' do
expect { get '/bad_encoding', name: "Hello \x80" }.not_to raise_error
end
end
end

context 'custom validation message' do
context 'with invalid input' do
it 'refuses inapppopriate' do
Expand Down
19 changes: 19 additions & 0 deletions spec/grape/validations/validators/values_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,25 @@ def default_excepts
stub_const('ValuesModel', values_model)
end

describe '#bad encoding' do
let(:app) do
Class.new(Grape::API) do
default_format :json

params do
requires :type, type: String, values: %w[a b]
end
get '/bad_encoding'
end
end

context 'when value as bad encoding' do
it 'does not raise an error' do
expect { get '/bad_encoding', type: "Hello \x80" }.not_to raise_error
end
end
end

context 'with a custom validation message' do
it 'allows a valid value for a parameter' do
get('/custom_message', type: 'valid-type1')
Expand Down

0 comments on commit 5ce44de

Please sign in to comment.