Skip to content

Commit

Permalink
Fix multipart filename for ActionDispatch::Http::UploadedFile TempFile
Browse files Browse the repository at this point in the history
  • Loading branch information
Zachary Herr committed Jul 2, 2018
1 parent da1b1ad commit 173b40a
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 10 deletions.
4 changes: 4 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## Unreleased

* [Fix mulipart uploads with ActionDispatch::Http::UploadedFile TempFile by using original_filename](https://github.com/jnunemaker/httparty/pull/598)

## 0.16.2

* [Support ActionDispatch::Http::UploadedFile again](https://github.com/jnunemaker/httparty/pull/585)
Expand Down
6 changes: 5 additions & 1 deletion lib/httparty/request/body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def generate_multipart
memo += %(Content-Disposition: form-data; name="#{key}")
# value.path is used to support ActionDispatch::Http::UploadedFile
# https://github.com/jnunemaker/httparty/pull/585
memo += %(; filename="#{File.basename(value.path)}") if file?(value)
memo += %(; filename="#{determine_file_name(value)}") if file?(value)
memo += "\r\n"
memo += "Content-Type: application/octet-stream\r\n" if file?(value)
memo += "\r\n"
Expand Down Expand Up @@ -73,6 +73,10 @@ def normalize_query(query)
end
end

def determine_file_name(object)
object.respond_to?(:original_filename) ? object.original_filename : File.basename(object.path)
end

attr_reader :params, :query_string_normalizer
end
end
Expand Down
1 change: 1 addition & 0 deletions spec/fixtures/unknown_file_type
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
???
31 changes: 23 additions & 8 deletions spec/httparty/request/body_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require_relative '../../spec_helper'
require 'tempfile'

RSpec.describe HTTParty::Request::Body do
describe HTTParty::Request::Body do
describe '#call' do
subject { described_class.new(params).call }

Expand All @@ -18,26 +18,30 @@

context 'when params has file' do
before do
allow(HTTParty::Request::MultipartBoundary).
to receive(:generate).and_return("------------------------c772861a5109d5ef")
allow(HTTParty::Request::MultipartBoundary)
.to receive(:generate).and_return("------------------------c772861a5109d5ef")
end

let(:file) { File.open('spec/fixtures/tiny.gif') }
let(:params) do
{
user: {
avatar: File.open('spec/fixtures/tiny.gif'),
avatar: file,
first_name: 'John',
last_name: 'Doe',
enabled: true
}
}
end
let(:expected_file_name) { 'tiny.gif' }
let(:expected_file_contents) { "GIF89a\u0001\u0000\u0001\u0000\u0000\xFF\u0000,\u0000\u0000\u0000\u0000\u0001\u0000\u0001\u0000\u0000\u0002\u0000;" }
let(:expected_content_type) { 'application/octet-stream' }
let(:multipart_params) do
"--------------------------c772861a5109d5ef\r\n" \
"Content-Disposition: form-data; name=\"user[avatar]\"; filename=\"tiny.gif\"\r\n" \
"Content-Type: application/octet-stream\r\n" \
"Content-Disposition: form-data; name=\"user[avatar]\"; filename=\"#{expected_file_name}\"\r\n" \
"Content-Type: #{expected_content_type}\r\n" \
"\r\n" \
"GIF89a\u0001\u0000\u0001\u0000\u0000\xFF\u0000,\u0000\u0000\u0000\u0000\u0001\u0000\u0001\u0000\u0000\u0002\u0000;\r\n" \
"#{expected_file_contents}\r\n" \
"--------------------------c772861a5109d5ef\r\n" \
"Content-Disposition: form-data; name=\"user[first_name]\"\r\n" \
"\r\n" \
Expand All @@ -54,6 +58,17 @@
end

it { is_expected.to eq multipart_params }

context 'file object responds to original_filename' do
let(:some_temp_file) { Tempfile.new('some_temp_file.gif') }
let(:expected_file_name) { "some_temp_file.gif" }
let(:expected_file_contents) { "Hello" }
let(:file) { double(:mocked_action_dispatch, path: some_temp_file.path, original_filename: 'some_temp_file.gif', read: expected_file_contents) }

before { some_temp_file.write('Hello') }

it { is_expected.to eq multipart_params }
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/httparty_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require File.expand_path(File.join(File.dirname(__FILE__), 'spec_helper'))

RSpec.describe HTTParty do
describe HTTParty do
before(:each) do
@klass = Class.new
@klass.instance_eval { include HTTParty }
Expand Down
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ def file_fixture(filename)
config.order = :random

Kernel.srand config.seed

config.expose_dsl_globally = true
end

RSpec::Matchers.define :use_ssl do
Expand Down

0 comments on commit 173b40a

Please sign in to comment.