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 Sinatra::RequiredParams implementation #1431

Merged

Conversation

arpitchauhan
Copy link
Contributor

@arpitchauhan arpitchauhan commented May 28, 2018

Currently, when required_params is called in such a way that it expects a key in the params to be a hash, it fails with error

undefined method has_key?

when it is not indeed a hash in the params in a request.

Now, it's been ensured that the key can respond to has_key? before has_key? is called on it.

So,

required_params :p1, :p2 => [:p3]

now works properly even if :p2 turns out not to point to a hash, e. g. if the params hash is

{ p1: 2, p2: 3 }

it will properly halt with status 400.

A unit test has also been added to required_params_spec.rb. It failed before the fix, but now passes.

Ensures that required_params doesn't fail
abruptly when a key in params it expected
to be a hash turns out to not be.

It fails currently.
When required_params is called in such a way
that it expects a key in the params to be a hash,
it fails with error

undefined method has_key?

when it is not indeed a hash in the params in a
request.

Now, it's been ensured that the key can respond to
has_key? before has_key? is called on it.

So,

required_params :p1, :p2 => [:p3]

now works properly even if :p2 turns out
not to point to a hash, e. g. if the params hash is

{
  p1: 2,
  p2: 3
}

it will properly halt with status 400.
@arpitchauhan arpitchauhan mentioned this pull request May 29, 2018
@namusyaka namusyaka added this to the v2.0.2 milestone May 29, 2018
Copy link
Member

@namusyaka namusyaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks

@namusyaka namusyaka merged commit 17c28f2 into sinatra:master Jun 2, 2018
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 23, 2018
## 2.0.4 / 2018-09-15

* Don't blow up when passing frozen string to `send_file` disposition [#1137](sinatra/sinatra#1137) by Andrew Selder

* Fix ubygems LoadError [#1436](sinatra/sinatra#1436) by Pavel Rosick�«ò

* Unescape regex captures [#1446](sinatra/sinatra#1446) by Jordan Owens

* Slight performance improvements for IndifferentHash [#1427](sinatra/sinatra#1427) by Mike Pastore

* Improve development support and documentation and source code by Will Yang, Jake Craige, Grey Baker and Guilherme Goettems Schneider

## 2.0.3 / 2018-06-09

* Fix the backports gem regression [#1442](sinatra/sinatra#1442) by Marc-Andr�«± Lafortune

## 2.0.2 / 2018-06-05

* Escape invalid query parameters [#1432](sinatra/sinatra#1432) by Kunpei Sakai
  * The patch fixes [CVE-2018-11627](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-11627).

* Fix undefined method error for `Sinatra::RequiredParams` with hash key [#1431](sinatra/sinatra#1431) by Arpit Chauhan

* Add xml content-types to valid html_types for Rack::Protection [#1413](sinatra/sinatra#1413) by Reenan Arbitrario

* Encode route parameters using :default_encoding setting [#1412](sinatra/sinatra#1412) by Brian m. Carlson

* Fix unpredictable behaviour from Sinatra::ConfigFile [#1244](sinatra/sinatra#1244) by John Hope

* Add Sinatra::IndifferentHash#slice [#1405](sinatra/sinatra#1405) by Shota Iguchi

* Remove status code 205 from drop body response [#1398](sinatra/sinatra#1398) by Shota Iguchi

* Ignore empty captures from params [#1390](sinatra/sinatra#1390) by Shota Iguchi

* Improve development support and documentation and source code by Zp Yuan, Andreas Finger, Olle Jonsson, Shota Iguchi, Nikita Bulai and Joshua O'Brien
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants