Skip to content

Commit

Permalink
Make json_parser plugin handle case where Rack::Request#POST is calle…
Browse files Browse the repository at this point in the history
…d previously for env on Rack 3 (Fixes #372)

The Rack 1-2 behavior did not cache rack.request.form_hash values
for data it didn't understand.  That changed in Rack 3, to fix
a bug in Rack that was open since 2014.  This updates the code
to work with all versions of Rack.  It now never checks
rack.request.form_hash or sets rack.request.form_input.

This approach is slightly slower for r.POST when given non-JSON
input, because it now checks the content_type when previously
it did not. Unfortunately, I don't see a way to avoid that.  I
did make the check faster by switching from a regexp to using
String#include?. In most cases, r.POST performance doesn't matter,
as users typically use r.params, and that is cached via an
instance variable, so r.POST is generally only called once per
request.
  • Loading branch information
jeremyevans committed Dec 18, 2024
1 parent 7ddfb97 commit eb7843c
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 22 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
=== master

* Make json_parser plugin handle case where Rack::Request#POST is called previously for env on Rack 3 (jeremyevans) (#372)

* Fix strict_unused_block warnings when running specs on Ruby 3.4 (jeremyevans)

= 3.87.0 (2024-12-17)
Expand Down
39 changes: 20 additions & 19 deletions lib/roda/plugins/json_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,27 +53,28 @@ module RequestMethods
# parse the request body as JSON. Ignore an empty request body.
def POST
env = @env
if post_params = (env["roda.json_params"] || env["rack.request.form_hash"])
post_params
elsif (input = env["rack.input"]) && content_type =~ /json/
str = _read_json_input(input)
return super if str.empty?
begin
json_params = parse_json(str)
rescue
roda_class.opts[:json_parser_error_handler].call(self)
end
if post_params = env["roda.json_params"]
return post_params
end

unless (input = env["rack.input"]) && (content_type = self.content_type) && content_type.include?('json')
return super
end

str = _read_json_input(input)
return super if str.empty?
begin
json_params = parse_json(str)
rescue
roda_class.opts[:json_parser_error_handler].call(self)
end

wrap = roda_class.opts[:json_parser_wrap]
if wrap == :always || (wrap == :unless_hash && !json_params.is_a?(Hash))
json_params = {"_json"=>json_params}
end
env["roda.json_params"] = json_params
env["rack.request.form_input"] = input
json_params
else
super
wrap = roda_class.opts[:json_parser_wrap]
if wrap == :always || (wrap == :unless_hash && !json_params.is_a?(Hash))
json_params = {"_json"=>json_params}
end
env["roda.json_params"] = json_params
json_params
end

private
Expand Down
7 changes: 7 additions & 0 deletions spec/plugin/json_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
body('rack.input'=>rack_input('{"a":{"b":1}}'), 'CONTENT_TYPE'=>'text/json', 'REQUEST_METHOD'=>'POST').must_equal '1'
end

it "Handles Rack::Request#POST being called in advance" do
env = req_env('rack.input'=>rack_input('{"a":{"b":1}}'), 'CONTENT_TYPE'=>'text/json', 'REQUEST_METHOD'=>'POST')
r = Rack::Request.new(env)
r.POST
body(env).must_equal '1'
end

it "doesn't affect parsing of non-json content type" do
body('rack.input'=>rack_input('a[b]=1'), 'REQUEST_METHOD'=>'POST').must_equal '1'
end
Expand Down
10 changes: 7 additions & 3 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ def req(path='/', env={})
_req(@app, env)
end

def _req(app, env)
env = {"REQUEST_METHOD" => "GET", "PATH_INFO" => "/", "SCRIPT_NAME" => ""}.merge(env)
def req_env(env)
env = {"REQUEST_METHOD" => "GET", "PATH_INFO" => "/", "SCRIPT_NAME" => ""}.merge!(env)
if ENV['LINT']
env['SERVER_NAME'] ||= 'example.com'
env['SERVER_PROTOCOL'] ||= env['HTTP_VERSION'] || 'HTTP/1.0'
Expand All @@ -187,7 +187,11 @@ def _req(app, env)
env['rack.multiprocess'] = env['rack.multithread'] = env['rack.run_once'] = false
end
end
a = @app.call(env)
env
end

def _req(app, env)
a = @app.call(req_env(env))

if ENV['LINT']
orig = a[2]
Expand Down

0 comments on commit eb7843c

Please sign in to comment.