Skip to content

Commit

Permalink
correctly set multipart param encoding
Browse files Browse the repository at this point in the history
  • Loading branch information
tenderlove committed Dec 4, 2013
1 parent fab6bcf commit 5c7f9c6
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 0 deletions.
33 changes: 33 additions & 0 deletions lib/rack/multipart/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ def parse
end

get_data(filename, body, content_type, name, head) do |data|
tag_multipart_encoding(filename, content_type, name, data)

Utils.normalize_params(@params, name, data)
end

Expand Down Expand Up @@ -165,9 +167,40 @@ def scrub_filename(filename)
filename.encode!(:invalid => :replace, :undef => :replace)
end
end

CHARSET = "charset"
TEXT_PLAIN = "text/plain"

def tag_multipart_encoding(filename, content_type, name, body)
name.force_encoding Encoding::UTF_8

return if filename

encoding = Encoding::UTF_8

if content_type
list = content_type.split(';')
type_subtype = list.first
type_subtype.strip!
if TEXT_PLAIN == type_subtype
rest = list.drop 1
rest.each do |param|
k,v = param.split('=', 2)
k.strip!
v.strip!
encoding = Encoding.find v if k == CHARSET
end
end
end

name.force_encoding encoding
body.force_encoding encoding
end
else
def scrub_filename(filename)
end
def tag_multipart_encoding(filename, content_type, name, body)
end
end

def get_data(filename, body, content_type, name, head)
Expand Down
44 changes: 44 additions & 0 deletions test/spec_multipart.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,42 @@ def multipart_file(name)
params["text"].should.equal "contents"
end

should "set US_ASCII encoding based on charset" do
env = Rack::MockRequest.env_for("/", multipart_fixture(:content_type_and_no_filename))
params = Rack::Multipart.parse_multipart(env)
params["text"].encoding.should.equal Encoding::US_ASCII

# I'm not 100% sure if making the param name encoding match the
# Content-Type charset is the right thing to do. We should revisit this.
params.keys.each do |key|
key.encoding.should.equal Encoding::US_ASCII
end
end

should "set BINARY encoding on things without content type" do
env = Rack::MockRequest.env_for("/", multipart_fixture(:none))
params = Rack::Multipart.parse_multipart(env)
params["submit-name"].encoding.should.equal Encoding::UTF_8
end

should "set UTF8 encoding on names of things without content type" do
env = Rack::MockRequest.env_for("/", multipart_fixture(:none))
params = Rack::Multipart.parse_multipart(env)
params.keys.each do |key|
key.encoding.should.equal Encoding::UTF_8
end
end

should "default text to UTF8" do
env = Rack::MockRequest.env_for("/", multipart_fixture(:text))
params = Rack::Multipart.parse_multipart(env)
params['submit-name'].encoding.should.equal Encoding::UTF_8
params['submit-name-with-content'].encoding.should.equal Encoding::UTF_8
params.keys.each do |key|
key.encoding.should.equal Encoding::UTF_8
end
end

should "raise RangeError if the key space is exhausted" do
env = Rack::MockRequest.env_for("/", multipart_fixture(:content_type_and_no_filename))

Expand Down Expand Up @@ -196,6 +232,14 @@ def rd.length
params["files"].size.should.equal 252
end

should "parse multipart/mixed" do
env = Rack::MockRequest.env_for("/", multipart_fixture(:mixed_files))
params = Rack::Utils::Multipart.parse_multipart(env)
params["foo"].should.equal "bar"
params["files"].should.be.instance_of String
params["files"].size.should.equal 252
end

should "parse IE multipart upload and clean up filename" do
env = Rack::MockRequest.env_for("/", multipart_fixture(:ie))
params = Rack::Multipart.parse_multipart(env)
Expand Down

1 comment on commit 5c7f9c6

@spastorino
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit broke the build for MRI 1.8.7 https://travis-ci.org/rack/rack/jobs/14898576

Please sign in to comment.