Rack 3 / json_parser behavior change, not sure whether this is a bug

[benalavi]

In Rack 2 the below code works: Roda's json_parser parses the params and the post prints them out. In Rack 3 it breaks and the params are not parsed if any middleware before Roda attempts to read the params via a Rack::Request object.

The issue seems to be that in Rack 3 attempting to read the params will set env["rack.request.form_hash"] = {}, where in Rack 2 it would not. json_parser then checks to see if rack.request.form_hash is truthy, and if so does not attempt to parse the params via JSON.

This broke for us upgrading to Rack 3, but I'm not sure whether this should be considered an issue w/ Roda or expected behavior.

require "roda"

class Auth
  def initialize(app)
    @app = app
  end

  def call(env)
    request = Rack::Request.new env
    # calling request.params ends up here:
    # https://github.com/rack/rack/blob/v3.1.8/lib/rack/request.rb#L545, which
    # sets RACK_REQUEST_FORM_HASH = {}
    puts "request params: #{request.params.inspect}"

    @app.call env
  end
end

class API < Roda
  plugin :json_parser

  route do |r|
    r.post do
      # Rack::Request.new.params sets env["rack.request.form_hash"], so
      # json_parser does not attempt to parse the params again:
      # https://github.com/jeremyevans/roda/blob/master/lib/roda/plugins/json_parser.rb#L56
      r.params.inspect
    end
  end
end

run(
  Rack::Builder.new do
    use Auth
    run API
  end
)

bundle exec puma
curl -XPOST -d '{"foo":1}' -H 'Content-Type: application/json' localhost:9292

[jeremyevans]

Thank you very much for the report. The main reason that rack.request.form_hash was already respected is to handle non-json input without significant performance loss. That worked fine in Rack 2, but as you point out, it's no longer safe in Rack 3. Since Rack 3 sets rack.request.form_hash to {} for JSON data without attempting to read the body, Roda's behavior should be improved. I'll fix this shortly.

[jeremyevans]

Fixed in eb7843c, is it possible for you to test your app with the master branch to confirm this fixes the issue for you?

[benalavi]

Thanks @jeremyevans! The fix on master does indeed fix the issue for us w/ Rack 3 👍

[jeremyevans]

Great! Thank you for confirming.