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

Avoid TypeError when params contain a key without a value on Ruby < 2.4 #1517

Merged
merged 1 commit into from Feb 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
@@ -1,3 +1,7 @@
## Unreleased

* Avoid `TypeError` when params contain a key without a value on Ruby < 2.4 [#1516](https://github.com/sinatra/sinatra/pull/1516) by Samuel Giddins

Copy link
Member

Choose a reason for hiding this comment

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

You don't need to update CHANGELOG.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The contributing.md said it made it easier to do releases when PRs included changelog entries 😉

Copy link
Member

@namusyaka namusyaka Feb 3, 2019

Choose a reason for hiding this comment

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

The changelog entry will automatically be generated by the release script: #1515
contributing.md will also be updated at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/sinatra/sinatra/blob/master/CONTRIBUTING.md#have-a-patch #4 says to update the changelog, I was just following the instructions 🤷

Copy link
Member

Choose a reason for hiding this comment

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

@segiddins Yes, I apologize if confusing.
But as I said earlier, the contents of contributing.md will be updated soon.
Do you want to keep your own changelog entry so much?
If not, please remove the entry from changelog.

Copy link
Contributor

@junaruga junaruga Feb 11, 2019

Choose a reason for hiding this comment

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

This PR is not #1516 but #1517. That confused me a little :)

[#1516](https://github.com/sinatra/sinatra/pull/1516)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agh, I was trying to figure out which PR it would be before opening it

## 2.0.5 / 2018-12-22

* Avoid FrozenError when params contains frozen value [#1506](https://github.com/sinatra/sinatra/pull/1506) by Kunpei Sakai
Expand Down
2 changes: 1 addition & 1 deletion lib/sinatra/base.rb
Expand Up @@ -1089,7 +1089,7 @@ def invoke

# Dispatch a request with error handling.
def dispatch!
@params.merge!(@request.params).each { |key, val| @params[key] = force_encoding(val.dup) }
@params.merge!(@request.params).each { |key, val| @params[key] = val && force_encoding(val.dup) }

invoke do
static! if settings.static? && (request.get? || request.head?)
Expand Down
11 changes: 11 additions & 0 deletions test/routing_test.rb
Expand Up @@ -311,6 +311,17 @@ class RoutingTest < Minitest::Test
assert_equal 'well, alright', body
end

it "handles params without a value" do
mock_app {
get '/' do
assert_nil params.fetch('foo')
"Given: #{params.keys.sort.join(',')}"
end
}
get '/?foo'
assert_equal 'Given: foo', body
end

it "merges named params and query string params in params" do
mock_app {
get '/:foo' do
Expand Down