Skip to content

Commit

Permalink
Make Rack::Request#{[],[]=} warn even in non-verbose mode
Browse files Browse the repository at this point in the history
Additionally, use the :uplevel keyword to warn, so it includes
the caller information in the warning output (similar to rb_warn).
This only works on Ruby 2.5+, so output will look a little odd on
Ruby 2.3/2.4, but I don't think it warrants separate handling in
older versions.

Fixes rack#1277
  • Loading branch information
jeremyevans committed Jul 14, 2020
1 parent 5791ef6 commit 058fa08
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file. For info on
- Removed options from `Rack::Builder.parse_file` and `Rack::Builder.load_file`. ([#1663](https://github.com/rack/rack/pull/1663), [@ioquatix](https://github.com/ioquatix))
- HMAC argument for `Rack::Session::Cookie` doesn't accept a class constant anymore, but only a string recognized by OpenSSL (e.g. `"SHA256"`) or compatible instance (e.g. `OpenSSL::Digest.new("SHA256")`) ([#1676](https://github.com/rack/rack/pull/1676), [@bdewater](https://github.com/bdewater))
- `Rack::HTTP_VERSION` has been removed and the `HTTP_VERSION` env setting is no longer set in the CGI and Webrick handlers . ([#970](https://github.com/rack/rack/issues/970), [@jeremyevans](https://github.com/jeremyevans))
- `Rack::Request#[]` and `#[]=` now warn even in non-verbose mode. ([#1277](https://github.com/rack/rack/issues/1277), [@jeremyevans](https://github.com/jeremyevans))

### Fixed

Expand Down
8 changes: 2 additions & 6 deletions lib/rack/request.rb
Expand Up @@ -530,9 +530,7 @@ def trusted_proxy?(ip)

# shortcut for <tt>request.params[key]</tt>
def [](key)
if $VERBOSE
warn("Request#[] is deprecated and will be removed in a future version of Rack. Please use request.params[] instead")
end
warn("Request#[] is deprecated and will be removed in a future version of Rack. Please use request.params[] instead", uplevel: 1)

params[key.to_s]
end
Expand All @@ -541,9 +539,7 @@ def [](key)
#
# Note that modifications will not be persisted in the env. Use update_param or delete_param if you want to destructively modify params.
def []=(key, value)
if $VERBOSE
warn("Request#[]= is deprecated and will be removed in a future version of Rack. Please use request.params[]= instead")
end
warn("Request#[]= is deprecated and will be removed in a future version of Rack. Please use request.params[]= instead", uplevel: 1)

params[key.to_s] = value
end
Expand Down
44 changes: 28 additions & 16 deletions test/spec_request.rb
Expand Up @@ -523,19 +523,21 @@ def initialize(*)
it "get value by key from params with #[]" do
req = make_request \
Rack::MockRequest.env_for("?foo=quux")
req['foo'].must_equal 'quux'
req[:foo].must_equal 'quux'
assert_output(nil, /deprecated/) do
req['foo'].must_equal 'quux'
req[:foo].must_equal 'quux'
end

next if self.class == TestProxyRequest
verbose = $VERBOSE
warn_arg = nil
req.define_singleton_method(:warn) do |arg|
warn_arg = arg
req.define_singleton_method(:warn) do |*args|
warn_arg = args
end
begin
$VERBOSE = true
req['foo'].must_equal 'quux'
warn_arg.must_equal "Request#[] is deprecated and will be removed in a future version of Rack. Please use request.params[] instead"
warn_arg.must_equal ["Request#[] is deprecated and will be removed in a future version of Rack. Please use request.params[] instead", {uplevel: 1}]
ensure
$VERBOSE = verbose
end
Expand All @@ -544,33 +546,43 @@ def initialize(*)
it "set value to key on params with #[]=" do
req = make_request \
Rack::MockRequest.env_for("?foo=duh")
req['foo'].must_equal 'duh'
req[:foo].must_equal 'duh'
assert_output(nil, /deprecated/) do
req['foo'].must_equal 'duh'
req[:foo].must_equal 'duh'
end
req.params.must_equal 'foo' => 'duh'

if req.delegate?
skip "delegate requests don't cache params, so mutations have no impact"
end

req['foo'] = 'bar'
assert_output(nil, /deprecated/) do
req['foo'] = 'bar'
end
req.params.must_equal 'foo' => 'bar'
req['foo'].must_equal 'bar'
req[:foo].must_equal 'bar'
assert_output(nil, /deprecated/) do
req['foo'].must_equal 'bar'
req[:foo].must_equal 'bar'
end

req[:foo] = 'jaz'
assert_output(nil, /deprecated/) do
req[:foo] = 'jaz'
end
req.params.must_equal 'foo' => 'jaz'
req['foo'].must_equal 'jaz'
req[:foo].must_equal 'jaz'
assert_output(nil, /deprecated/) do
req['foo'].must_equal 'jaz'
req[:foo].must_equal 'jaz'
end

verbose = $VERBOSE
warn_arg = nil
req.define_singleton_method(:warn) do |arg|
warn_arg = arg
req.define_singleton_method(:warn) do |*args|
warn_arg = args
end
begin
$VERBOSE = true
req['foo'] = 'quux'
warn_arg.must_equal "Request#[]= is deprecated and will be removed in a future version of Rack. Please use request.params[]= instead"
warn_arg.must_equal ["Request#[]= is deprecated and will be removed in a future version of Rack. Please use request.params[]= instead", {uplevel: 1}]
req.params['foo'].must_equal 'quux'
ensure
$VERBOSE = verbose
Expand Down

0 comments on commit 058fa08

Please sign in to comment.