From a987e5096e831e68945cac4a95b1b230e2f67968 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 16 Jan 2023 21:09:33 +1300 Subject: [PATCH 1/3] `Rack::Request#POST` should consistently raise errors. Cache errors that occur when invoking `Rack::Request#POST` so they can be raised again later. --- lib/rack/constants.rb | 1 + lib/rack/request.rb | 47 ++++++++++++++++++++++++++----------------- test/spec_request.rb | 16 +++++++++++++++ 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/lib/rack/constants.rb b/lib/rack/constants.rb index 6aa9281f6..13365935b 100644 --- a/lib/rack/constants.rb +++ b/lib/rack/constants.rb @@ -55,6 +55,7 @@ module Rack RACK_REQUEST_FORM_INPUT = 'rack.request.form_input' RACK_REQUEST_FORM_HASH = 'rack.request.form_hash' RACK_REQUEST_FORM_VARS = 'rack.request.form_vars' + RACK_REQUEST_FORM_ERROR = 'rack.request.form_error' RACK_REQUEST_COOKIE_HASH = 'rack.request.cookie_hash' RACK_REQUEST_COOKIE_STRING = 'rack.request.cookie_string' RACK_REQUEST_QUERY_HASH = 'rack.request.query_hash' diff --git a/lib/rack/request.rb b/lib/rack/request.rb index 6641b5fcd..f7bb37bdd 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -496,26 +496,35 @@ def GET # This method support both application/x-www-form-urlencoded and # multipart/form-data. def POST - if get_header(RACK_INPUT).nil? - raise "Missing rack.input" - elsif get_header(RACK_REQUEST_FORM_INPUT) == get_header(RACK_INPUT) - get_header(RACK_REQUEST_FORM_HASH) - elsif form_data? || parseable_data? - unless set_header(RACK_REQUEST_FORM_HASH, parse_multipart) - form_vars = get_header(RACK_INPUT).read - - # Fix for Safari Ajax postings that always append \0 - # form_vars.sub!(/\0\z/, '') # performance replacement: - form_vars.slice!(-1) if form_vars.end_with?("\0") - - set_header RACK_REQUEST_FORM_VARS, form_vars - set_header RACK_REQUEST_FORM_HASH, parse_query(form_vars, '&') + if error = get_header(RACK_REQUEST_FORM_ERROR) + raise error + end + + begin + if get_header(RACK_INPUT).nil? + raise "Missing rack.input" + elsif get_header(RACK_REQUEST_FORM_INPUT) == get_header(RACK_INPUT) + get_header(RACK_REQUEST_FORM_HASH) + elsif form_data? || parseable_data? + unless set_header(RACK_REQUEST_FORM_HASH, parse_multipart) + form_vars = get_header(RACK_INPUT).read + + # Fix for Safari Ajax postings that always append \0 + # form_vars.sub!(/\0\z/, '') # performance replacement: + form_vars.slice!(-1) if form_vars.end_with?("\0") + + set_header RACK_REQUEST_FORM_VARS, form_vars + set_header RACK_REQUEST_FORM_HASH, parse_query(form_vars, '&') + end + set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT) + get_header RACK_REQUEST_FORM_HASH + else + set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT) + set_header(RACK_REQUEST_FORM_HASH, {}) end - set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT) - get_header RACK_REQUEST_FORM_HASH - else - set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT) - set_header(RACK_REQUEST_FORM_HASH, {}) + rescue => error + set_header(RACK_REQUEST_FORM_ERROR, error) + raise end end diff --git a/test/spec_request.rb b/test/spec_request.rb index 750639deb..9fed5029a 100644 --- a/test/spec_request.rb +++ b/test/spec_request.rb @@ -1218,6 +1218,22 @@ def initialize(*) req.media_type_params['weird'].must_equal 'lol"' end + it "returns the same error for invalid post inputs" do + 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 + # The actual exception type here is unimportant - just that it fails. + assert_raises(Rack::Utils::ParameterTypeError) do + Rack::Request.new(env).POST + end + end + end + it "parse with junk before boundary" do # Adapted from RFC 1867. input = < Date: Mon, 16 Jan 2023 21:28:44 +1300 Subject: [PATCH 2/3] Don't throw exactly the same error - so we have the correct backtrace. --- lib/rack/request.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index f7bb37bdd..c28e33c19 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -497,7 +497,7 @@ def GET # multipart/form-data. def POST if error = get_header(RACK_REQUEST_FORM_ERROR) - raise error + raise error.class, error.message, cause: error end begin From 36977c21f1be93e2327f59cc0d22eb64f8447667 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 17 Jan 2023 10:45:49 +1300 Subject: [PATCH 3/3] Set `cause: error.cause`. --- lib/rack/request.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index c28e33c19..40922a219 100644 --- a/lib/rack/request.rb +++ b/lib/rack/request.rb @@ -497,7 +497,7 @@ def GET # multipart/form-data. def POST if error = get_header(RACK_REQUEST_FORM_ERROR) - raise error.class, error.message, cause: error + raise error.class, error.message, cause: error.cause end begin