Skip to content

MethodOverride middleware silently eats invalid parameters. #2009

@ioquatix

Description

@ioquatix

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"
    end

The 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
end

Output:

["/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=>{}}

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions