From 294fd239a71aab805877790f0a92ee3c72e67d79 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Tue, 14 Jul 2020 11:02:48 -0700 Subject: [PATCH] Make Rack::Request#{[],[]=} warn even in non-verbose mode 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 #1277 --- CHANGELOG.md | 1 + lib/rack/request.rb | 8 ++------ test/spec_request.rb | 44 ++++++++++++++++++++++++++++---------------- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 427ec7974..9e377de69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/rack/request.rb b/lib/rack/request.rb index a69bc8e08..74377ede3 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -530,9 +530,7 @@ def trusted_proxy?(ip) # shortcut for request.params[key] 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 @@ -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 diff --git a/test/spec_request.rb b/test/spec_request.rb index 4ce82e9a6..c948947e1 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -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 @@ -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