From 173b40aa70221d7af42d5ea38126de23443f48fb Mon Sep 17 00:00:00 2001 From: Zachary Herr Date: Mon, 2 Jul 2018 09:57:15 -0500 Subject: [PATCH] Fix multipart filename for ActionDispatch::Http::UploadedFile TempFile --- Changelog.md | 4 ++++ lib/httparty/request/body.rb | 6 +++++- spec/fixtures/unknown_file_type | 1 + spec/httparty/request/body_spec.rb | 31 ++++++++++++++++++++++-------- spec/httparty_spec.rb | 2 +- spec/spec_helper.rb | 2 ++ 6 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 spec/fixtures/unknown_file_type diff --git a/Changelog.md b/Changelog.md index ebbb629c..e2b44134 100644 --- a/Changelog.md +++ b/Changelog.md @@ -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) diff --git a/lib/httparty/request/body.rb b/lib/httparty/request/body.rb index 5bdb7635..27e70e36 100644 --- a/lib/httparty/request/body.rb +++ b/lib/httparty/request/body.rb @@ -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" @@ -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 diff --git a/spec/fixtures/unknown_file_type b/spec/fixtures/unknown_file_type new file mode 100644 index 00000000..be108529 --- /dev/null +++ b/spec/fixtures/unknown_file_type @@ -0,0 +1 @@ +??? diff --git a/spec/httparty/request/body_spec.rb b/spec/httparty/request/body_spec.rb index 1addb1ce..430a8ae7 100644 --- a/spec/httparty/request/body_spec.rb +++ b/spec/httparty/request/body_spec.rb @@ -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 } @@ -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" \ @@ -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 diff --git a/spec/httparty_spec.rb b/spec/httparty_spec.rb index 09e9d403..71d174f6 100644 --- a/spec/httparty_spec.rb +++ b/spec/httparty_spec.rb @@ -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 } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d8622a46..8c941feb 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -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