Skip to content

Commit

Permalink
When parsing a multi-part POST, retain original pairs (#2088)
Browse files Browse the repository at this point in the history
The input is not guaranteed to be rewindable, and the default parameter
expansion loses details of the parameter names.

For form-encoded data, we (already) retain the full string: it contains
only simple key/value pairs, and will be of a manageable size.

Multipart data may contain uploaded files, so we don't wish to retain
the full input string -- that's why we dropped the rewindability
requirement on the input stream. Instead, we retain an array of
two-element arrays representing the "raw" key-value pairs as we parsed
the stream. This minimizes additional memory use, because the values are
the same objects we use in the params hash, while still allowing an
interested consumer to apply their own logic to parameter name
interpretation.
  • Loading branch information
matthewd committed Jul 17, 2023
1 parent 8f5c885 commit 3855d1d
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
1 change: 1 addition & 0 deletions lib/rack/constants.rb
Expand Up @@ -54,6 +54,7 @@ module Rack
RACK_RESPONSE_FINISHED = 'rack.response_finished'
RACK_REQUEST_FORM_INPUT = 'rack.request.form_input'
RACK_REQUEST_FORM_HASH = 'rack.request.form_hash'
RACK_REQUEST_FORM_PAIRS = 'rack.request.form_pairs'
RACK_REQUEST_FORM_VARS = 'rack.request.form_vars'
RACK_REQUEST_FORM_ERROR = 'rack.request.form_error'
RACK_REQUEST_COOKIE_HASH = 'rack.request.cookie_hash'
Expand Down
25 changes: 25 additions & 0 deletions lib/rack/multipart.rb
Expand Up @@ -19,6 +19,31 @@ class MissingInputError < StandardError
include BadRequest
end

# Accumulator for multipart form data, conforming to the QueryParser API.
# In future, the Parser could return the pair list directly, but that would
# change its API.
class ParamList # :nodoc:
def self.make_params
new
end

def self.normalize_params(params, key, value)
params << [key, value]
end

def initialize
@pairs = []
end

def <<(pair)
@pairs << pair
end

def to_params_hash
@pairs
end
end

class << self
def parse_multipart(env, params = Rack::Utils.default_query_parser)
unless io = env[RACK_INPUT]
Expand Down
15 changes: 14 additions & 1 deletion lib/rack/request.rb
Expand Up @@ -516,7 +516,10 @@ def POST
set_header RACK_REQUEST_FORM_INPUT, nil
set_header(RACK_REQUEST_FORM_HASH, {})
elsif form_data? || parseable_data?
unless set_header(RACK_REQUEST_FORM_HASH, parse_multipart)
if pairs = Rack::Multipart.parse_multipart(env, Rack::Multipart::ParamList)
set_header RACK_REQUEST_FORM_PAIRS, pairs
set_header RACK_REQUEST_FORM_HASH, expand_param_pairs(pairs)
else
form_vars = get_header(RACK_INPUT).read

# Fix for Safari Ajax postings that always append \0
Expand Down Expand Up @@ -672,6 +675,16 @@ def parse_multipart
Rack::Multipart.extract_multipart(self, query_parser)
end

def expand_param_pairs(pairs, query_parser = query_parser())
params = query_parser.make_params

pairs.each do |k, v|
query_parser.normalize_params(params, k, v)
end

params.to_params_hash
end

def split_header(value)
value ? value.strip.split(/[,\s]+/) : []
end
Expand Down
2 changes: 2 additions & 0 deletions test/spec_request.rb
Expand Up @@ -1359,6 +1359,8 @@ def initialize(*)
f[:filename].must_equal "dj.jpg"
f.must_include :tempfile
f[:tempfile].size.must_equal 76

req.env['rack.request.form_pairs'].must_equal [["reply", "yes"], ["fileupload", f]]
end

it "MultipartPartLimitError when request has too many multipart file parts if limit set" do
Expand Down

0 comments on commit 3855d1d

Please sign in to comment.