ArgumentError raised when parameter is encoded incorrectly

[MrBananaLord]

Hiya, I am not sure what to do with this one :)

The ArgumentError is raised for requests with incorrectly encoded params. That is kind of good and kind of bad at the same time. It's good that it's caught. It's bad because it's really hard to handle this. Catching ArgumentError could be really risky and additionally it happens in the middleware.

In vanilla rails, this is handled by raising ActionDispatch::InvalidParameterError that later results in ActionController::BadRequest. I am not sure if we want to couple this to rails, but maybe it'd be worth wrapping it with descriptive error that could be handled down the line? (I am also not sure how could we allow for handling it)

The spec to reproduce is spec/middlewares/request_validation_spec.rb:

  context 'when parameter is incorrectly encoded' do
    it 'returns 400' do
      get '/pets?limit=%E0%A4%A'
      expect(last_response.status).to eq 400 # not necessarily 400, that's just an assumption!
    end
  end

Result:

Failures:

  1) OpenapiFirst::Middlewares::RequestValidation when parameter is incorrectly encoded returns 400
     Failure/Error: OpenapiParameters::Query.new(operation.query_parameters).unpack(request.env[Rack::QUERY_STRING]) || {}
     
     ArgumentError:
       invalid %-encoding (%E0%A4%A)
     # ./lib/openapi_first/runtime_request.rb:87:in 'OpenapiFirst::RuntimeRequest#query'
     # ./lib/openapi_first/request_validation/validator.rb:53:in 'OpenapiFirst::RequestValidation::Validator#validate_query_params!'
     # ./lib/openapi_first/request_validation/validator.rb:35:in 'OpenapiFirst::RequestValidation::Validator#validate_parameters!'
     # ./lib/openapi_first/request_validation/validator.rb:17:in 'block in OpenapiFirst::RequestValidation::Validator#validate'
     # ./lib/openapi_first/request_validation/validator.rb:15:in 'Kernel#catch'
     # ./lib/openapi_first/request_validation/validator.rb:15:in 'OpenapiFirst::RequestValidation::Validator#validate'
     # ./lib/openapi_first/runtime_request.rb:125:in 'OpenapiFirst::RuntimeRequest#validate'
     # ./lib/openapi_first/middlewares/request_validation.rb:32:in 'OpenapiFirst::Middlewares::RequestValidation#call'
     # ./spec/middlewares/request_validation_spec.rb:45:in 'block (3 levels) in <top (required)>'

In my app, with removed silencers, the top of the stack is:

vendor/bundle/ruby/3.4.0/gems/uri-1.0.3/lib/uri/common.rb:425:in `URI._decode_uri_component': invalid %-encoding (%)

    raise ArgumentError, "invalid %-encoding (#{str})" if /%(?!\h\h)/.match?(str)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (ArgumentError)

    from vendor/bundle/ruby/3.4.0/gems/uri-1.0.3/lib/uri/common.rb:396:in `URI.decode_www_form_component'
    from vendor/bundle/ruby/3.4.0/gems/rack-3.1.16/lib/rack/query_parser.rb:235:in `Rack::QueryParser#unescape'
    from vendor/bundle/ruby/3.4.0/gems/rack-3.1.16/lib/rack/query_parser.rb:78:in `Array#map!'
    from vendor/bundle/ruby/3.4.0/gems/rack-3.1.16/lib/rack/query_parser.rb:78:in `block in Rack::QueryParser#parse_query'
    from vendor/bundle/ruby/3.4.0/gems/rack-3.1.16/lib/rack/query_parser.rb:76:in `Array#each'
    from vendor/bundle/ruby/3.4.0/gems/rack-3.1.16/lib/rack/query_parser.rb:76:in `Rack::QueryParser#parse_query'
    from vendor/bundle/ruby/3.4.0/gems/rack-3.1.16/lib/rack/utils.rb:103:in `Rack::Utils.parse_query'
    from vendor/bundle/ruby/3.4.0/gems/openapi_parameters-0.5.0/lib/openapi_parameters/query.rb:17:in `OpenapiParameters::Query#unpack'
    from vendor/bundle/ruby/3.4.0/gems/openapi_first-2.7.3/lib/openapi_first/request_parser.rb:30:in `OpenapiFirst::RequestParser#parse'
    from vendor/bundle/ruby/3.4.0/gems/openapi_first-2.7.3/lib/openapi_first/request.rb:50:in `block in OpenapiFirst::Request#validate'
    from vendor/bundle/ruby/3.4.0/gems/openapi_first-2.7.3/lib/openapi_first/request.rb:49:in `Kernel#catch'
    from vendor/bundle/ruby/3.4.0/gems/openapi_first-2.7.3/lib/openapi_first/request.rb:49:in `OpenapiFirst::Request#validate'
    from vendor/bundle/ruby/3.4.0/gems/openapi_first-2.7.3/lib/openapi_first/definition.rb:70:in `OpenapiFirst::Definition#validate_request'
    from vendor/bundle/ruby/3.4.0/gems/openapi_first-2.7.3/lib/openapi_first/middlewares/request_validation.rb:38:in `OpenapiFirst::Middlewares::RequestValidation#call'

[ahx]

Hi @MrBananaLord. Thanks for the report.

Here is what I found out about this:
The ArgumentError exception is raised by URI.decode_www_form_component (source), when calling Rack::Utils.parse_request, which is called by openapi_parameters. openapi_parameters uses Rack::Utils.parse_request and Rack::Utils.parse_nested_request, but only the latter raises a specific exception (Rack::Utils::InvalidParameterError).

Here is an idea how I will attempt to solve this:

1. Change openapi_paramters to raise Rack::Utils::InvalidParameterError when parsing the parameter
2. Handle Rack::Utils::InvalidParameterError (or the included Rack::BadRequest) in openapi_first and return a 400 (or raise an exception)

Does that sound right to you?

Thanks again for the report.

[ahx]

This should be fixed in version 2.7.4, which has just been released.

Does this work for you, @MrBananaLord ?

[MrBananaLord]

Wow! So fast :D

Looks great, thank you very much!

[edit] ah, I just tested it, works like charm :)