-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Closed
Labels
Description
See also #2006. cc @byroot @jeremyevans
The current implementation of method_override_param effectively eats invalid parameters.
def method_override_param(req)
req.POST[METHOD_OVERRIDE_PARAM_KEY] if req.form_data? || req.parseable_data?
rescue Utils::InvalidParameterError, Utils::ParameterTypeError
req.get_header(RACK_ERRORS).puts "Invalid or incomplete POST params"
rescue EOFError
req.get_header(RACK_ERRORS).puts "Bad request content body"
endThe first time this is called, an exception is raised and caught. However, the params is cached, and so no follow-up call to params doesn't appear to fail with the same error. This can change the behaviour of web applications which expect Rack::Request#POST or #params to raise an exception on invalid input.
I'm not sure what the ideal behaviour should be. Maybe we need to cache the exception and raise it in subsequent calls?
Example reproduction:
#!/usr/bin/env ruby
require 'stringio'
require 'rack/request'
env = {
'REQUEST_METHOD' => 'POST',
'PATH_INFO' => '/foo',
'rack.input' => StringIO.new('invalid=bar&invalid[foo]=bar'),
'HTTP_CONTENT_TYPE' => "application/x-www-form-urlencoded",
}
2.times do
begin
pp params: Rack::Request.new(env).POST
rescue => error
pp error: error
end
endOutput:
["/home/samuel/.gem/ruby/3.2.0/gems/rack-3.0.3/lib/rack/query_parser.rb:79:in `block in parse_nested_query'",
"/home/samuel/.gem/ruby/3.2.0/gems/rack-3.0.3/lib/rack/query_parser.rb:76:in `each'",
"/home/samuel/.gem/ruby/3.2.0/gems/rack-3.0.3/lib/rack/query_parser.rb:76:in `parse_nested_query'",
"/home/samuel/.gem/ruby/3.2.0/gems/rack-3.0.3/lib/rack/request.rb:648:in `parse_query'",
"/home/samuel/.gem/ruby/3.2.0/gems/rack-3.0.3/lib/rack/request.rb:512:in `POST'",
"/home/samuel/Desktop/request.rb:15:in `block in <main>'",
"/home/samuel/Desktop/request.rb:13:in `times'",
"/home/samuel/Desktop/request.rb:13:in `<main>'"]
{:error=>
#<Rack::QueryParser::ParameterTypeError: expected Hash (got String) for param `invalid'>}
{:params=>{}}
Reactions are currently unavailable