Deprecate key_space_limit

It was determined that as this limit did not affect nested parameter hashes, it didn't actually prevent an attacker from using more than limited number of bytes for parameter keys, so this limit isn't actually doing anything useful. It is confusing people when it gets in the way of desired large parameter requests.

3 files changed, 31 insertions, 56 deletions

diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb
index f4de71cf..87902a1f 100644
--- a/test/spec_multipart.rb
+++ b/test/spec_multipart.rb
@@ -98,17 +98,6 @@ describe Rack::Multipart do
     params['user_sid'].encoding.must_equal Encoding::UTF_8
   end
 
-  it "raise RangeError if the key space is exhausted" do
-    env = Rack::MockRequest.env_for("/", multipart_fixture(:content_type_and_no_filename))
-
-    old, Rack::Utils.key_space_limit = Rack::Utils.key_space_limit, 1
-    begin
-      lambda { Rack::Multipart.parse_multipart(env) }.must_raise(RangeError)
-    ensure
-      Rack::Utils.key_space_limit = old
-    end
-  end
-
   it "parse multipart form webkit style" do
     env = Rack::MockRequest.env_for '/', multipart_fixture(:webkit)
     env['CONTENT_TYPE'] = "multipart/form-data; boundary=----WebKitFormBoundaryWLHCs9qmcJJoyjKR"
@@ -219,7 +208,7 @@ describe Rack::Multipart do
         @params = Hash.new{|h, k| h[k.to_s] if k.is_a?(Symbol)}
       end
     end
-    query_parser = Rack::QueryParser.new c, 65536, 100
+    query_parser = Rack::QueryParser.new c, 100
     env = Rack::MockRequest.env_for("/", multipart_fixture(:text))
     params = Rack::Multipart.parse_multipart(env, query_parser)
     params[:files][:type].must_equal "text/plain"
diff --git a/test/spec_request.rb b/test/spec_request.rb
index 111f4912..c2c71dc4 100644
--- a/test/spec_request.rb
+++ b/test/spec_request.rb
@@ -295,7 +295,7 @@ class RackRequestTest < Minitest::Spec
         @params = Hash.new{|h, k| h[k.to_s] if k.is_a?(Symbol)}
       end
     end
-    parser = Rack::QueryParser.new(c, 65536, 100)
+    parser = Rack::QueryParser.new(c, 100)
     c = Class.new(Rack::Request) do
       define_method(:query_parser) do
         parser
@@ -316,32 +316,6 @@ class RackRequestTest < Minitest::Spec
     req.params.must_equal "foo" => "bar", "quux" => "b;la;wun=duh"
   end
 
-  it "limit the keys from the GET query string" do
-    env = Rack::MockRequest.env_for("/?foo=bar")
-
-    old, Rack::Utils.key_space_limit = Rack::Utils.key_space_limit, 1
-    begin
-      req = make_request(env)
-      lambda { req.GET }.must_raise RangeError
-    ensure
-      Rack::Utils.key_space_limit = old
-    end
-  end
-
-  it "limit the key size per nested params hash" do
-    nested_query = Rack::MockRequest.env_for("/?foo%5Bbar%5D%5Bbaz%5D%5Bqux%5D=1")
-    plain_query  = Rack::MockRequest.env_for("/?foo_bar__baz__qux_=1")
-
-    old, Rack::Utils.key_space_limit = Rack::Utils.key_space_limit, 3
-    begin
-      exp = { "foo" => { "bar" => { "baz" => { "qux" => "1" } } } }
-      make_request(nested_query).GET.must_equal exp
-      lambda { make_request(plain_query).GET  }.must_raise RangeError
-    ensure
-      Rack::Utils.key_space_limit = old
-    end
-  end
-
   it "limit the allowed parameter depth when parsing parameters" do
     env = Rack::MockRequest.env_for("/?a#{'[a]' * 40}=b")
     req = make_request(env)
@@ -388,7 +362,7 @@ class RackRequestTest < Minitest::Spec
         @params = Hash.new{|h, k| h[k.to_s] if k.is_a?(Symbol)}
       end
     end
-    parser = Rack::QueryParser.new(c, 65536, 100)
+    parser = Rack::QueryParser.new(c, 100)
     c = Class.new(Rack::Request) do
       define_method(:query_parser) do
         parser
@@ -438,20 +412,6 @@ class RackRequestTest < Minitest::Spec
     req.params.must_equal "foo" => "bar", "quux" => "bla"
   end
 
-  it "limit the keys from the POST form data" do
-    env = Rack::MockRequest.env_for("",
-            "REQUEST_METHOD" => 'POST',
-            :input => "foo=bar&quux=bla")
-
-    old, Rack::Utils.key_space_limit = Rack::Utils.key_space_limit, 1
-    begin
-      req = make_request(env)
-      lambda { req.POST }.must_raise RangeError
-    ensure
-      Rack::Utils.key_space_limit = old
-    end
-  end
-
   it "parse POST data with explicit content type regardless of method" do
     req = make_request \
       Rack::MockRequest.env_for("/",
diff --git a/test/spec_utils.rb b/test/spec_utils.rb
index f753f85b..a2673927 100644
--- a/test/spec_utils.rb
+++ b/test/spec_utils.rb
@@ -114,7 +114,7 @@ describe Rack::Utils do
     ex = { "foo" => nil }
     ex["foo"] = ex
 
-    params = Rack::Utils::KeySpaceConstrainedParams.new(65536)
+    params = Rack::Utils::KeySpaceConstrainedParams.new
     params['foo'] = params
     params.to_params_hash.to_s.must_equal ex.to_s
   end
@@ -123,6 +123,32 @@ describe Rack::Utils do
     Rack::Utils.parse_nested_query(nil).must_equal({})
   end
 
+  it "should warn using deprecated Rack::Util.key_space_limit=" do
+    begin
+      warn_arg = nil
+      Rack::Utils.define_singleton_method(:warn) do |*args|
+        warn_arg = args.first
+      end
+      Rack::Utils.key_space_limit = 65536
+      warn_arg.must_equal("`Rack::Utils.key_space_limit=` is deprecated and no longer has an effect. It will be removed in a future version of Rack")
+    ensure
+      Rack::Utils.singleton_class.send(:remove_method, :warn)
+    end
+  end
+
+  it "should warn using deprecated Rack::Util.key_space_limit" do
+    begin
+      warn_arg = nil
+      Rack::Utils.define_singleton_method(:warn) do |*args|
+        warn_arg = args.first
+      end
+      Rack::Utils.key_space_limit
+      warn_arg.must_equal("`Rack::Utils.key_space_limit` is deprecated as this value no longer has an effect. It will be removed in a future version of Rack")
+    ensure
+      Rack::Utils.singleton_class.send(:remove_method, :warn)
+    end
+  end
+
   it "raise an exception if the params are too deep" do
     len = Rack::Utils.param_depth_limit
 
@@ -259,7 +285,7 @@ describe Rack::Utils do
           @params = Hash.new{|h, k| h[k.to_s] if k.is_a?(Symbol)}
         end
       end
-      Rack::Utils.default_query_parser = Rack::QueryParser.new(param_parser_class, 65536, 100)
+      Rack::Utils.default_query_parser = Rack::QueryParser.new(param_parser_class, 100)
       h1 = Rack::Utils.parse_query(",foo=bar;,", ";,")
       h1[:foo].must_equal "bar"
       h2 = Rack::Utils.parse_nested_query("x[y][][z]=1&x[y][][w]=2")