From 98e0c346a614217bba65c01b3ba77e7951d83b2f Mon Sep 17 00:00:00 2001 From: Jonathan Garay Date: Wed, 11 May 2016 10:57:18 -0500 Subject: [PATCH] The uri io adapter should use the content-disposition filename The uri io adapter now seeks for the content-disposition header if this is pressent the value filename is taken instead of the last path segment for the resource file name. [fixes #2210] --- NEWS | 1 + lib/paperclip/io_adapters/uri_adapter.rb | 41 ++++++++++++++----- .../http_url_proxy_adapter_spec.rb | 13 +++--- .../paperclip/io_adapters/uri_adapter_spec.rb | 37 ++++++++++++++--- 4 files changed, 70 insertions(+), 22 deletions(-) diff --git a/NEWS b/NEWS index ff2c53171..d881e1c45 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,4 @@ +* Improvement: the `URI adapter` now uses the content-disposition header to name the downloaded file. * Improvement: Add `fog_options` configuration to send options to fog when storing files. 4.3.6 (3/13/2016): diff --git a/lib/paperclip/io_adapters/uri_adapter.rb b/lib/paperclip/io_adapters/uri_adapter.rb index c1a5fbd71..368096ac6 100644 --- a/lib/paperclip/io_adapters/uri_adapter.rb +++ b/lib/paperclip/io_adapters/uri_adapter.rb @@ -2,6 +2,8 @@ module Paperclip class UriAdapter < AbstractAdapter + attr_writer :content_type + def initialize(target) @target = target @content = download_content @@ -9,23 +11,40 @@ def initialize(target) @tempfile = copy_to_tempfile(@content) end - attr_writer :content_type - private - def download_content - open(@target) + def cache_current_values + self.content_type = content_type_from_content || "text/html" + + self.original_filename = filename_from_content_disposition || + filename_from_path || + default_filename + @size = @content.size end - def cache_current_values - @original_filename = @target.path.split("/").last - @original_filename ||= "index.html" - self.original_filename = @original_filename.strip + def content_type_from_content + if @content.respond_to?(:content_type) + @content.content_type + end + end - @content_type = @content.content_type if @content.respond_to?(:content_type) - @content_type ||= "text/html" + def filename_from_content_disposition + if @content.meta.has_key?("content-disposition") + @content.meta["content-disposition"]. + match(/filename="([^"]*)"/)[1] + end + end - @size = @content.size + def filename_from_path + @target.path.split("/").last + end + + def default_filename + "index.html" + end + + def download_content + open(@target) end def copy_to_tempfile(src) diff --git a/spec/paperclip/io_adapters/http_url_proxy_adapter_spec.rb b/spec/paperclip/io_adapters/http_url_proxy_adapter_spec.rb index c36ce5b47..9d01bcebb 100644 --- a/spec/paperclip/io_adapters/http_url_proxy_adapter_spec.rb +++ b/spec/paperclip/io_adapters/http_url_proxy_adapter_spec.rb @@ -1,11 +1,16 @@ require 'spec_helper' describe Paperclip::HttpUrlProxyAdapter do + before do + @open_return = StringIO.new("xxx") + @open_return.stubs(:content_type).returns("image/png") + @open_return.stubs(:meta).returns({}) + Paperclip::HttpUrlProxyAdapter.any_instance. + stubs(:download_content).returns(@open_return) + end + context "a new instance" do before do - @open_return = StringIO.new("xxx") - @open_return.stubs(:content_type).returns("image/png") - Paperclip::HttpUrlProxyAdapter.any_instance.stubs(:download_content).returns(@open_return) @url = "http://thoughtbot.com/images/thoughtbot-logo.png" @subject = Paperclip.io_adapters.for(@url) end @@ -60,7 +65,6 @@ context "a url with query params" do before do - Paperclip::HttpUrlProxyAdapter.any_instance.stubs(:download_content).returns(StringIO.new("x")) @url = "https://github.com/thoughtbot/paperclip?file=test" @subject = Paperclip.io_adapters.for(@url) end @@ -76,7 +80,6 @@ context "a url with restricted characters in the filename" do before do - Paperclip::HttpUrlProxyAdapter.any_instance.stubs(:download_content).returns(StringIO.new("x")) @url = "https://github.com/thoughtbot/paper:clip.jpg" @subject = Paperclip.io_adapters.for(@url) end diff --git a/spec/paperclip/io_adapters/uri_adapter_spec.rb b/spec/paperclip/io_adapters/uri_adapter_spec.rb index e0ab71367..65b8658f6 100644 --- a/spec/paperclip/io_adapters/uri_adapter_spec.rb +++ b/spec/paperclip/io_adapters/uri_adapter_spec.rb @@ -1,11 +1,19 @@ require 'spec_helper' describe Paperclip::UriAdapter do + let(:content_type) { "image/png" } + let(:meta) { {} } + + before do + @open_return = StringIO.new("xxx") + @open_return.stubs(:content_type).returns(content_type) + @open_return.stubs(:meta).returns(meta) + Paperclip::UriAdapter.any_instance. + stubs(:download_content).returns(@open_return) + end + context "a new instance" do before do - @open_return = StringIO.new("xxx") - @open_return.stubs(:content_type).returns("image/png") - Paperclip::UriAdapter.any_instance.stubs(:download_content).returns(@open_return) @uri = URI.parse("http://thoughtbot.com/images/thoughtbot-logo.png") @subject = Paperclip.io_adapters.for(@uri) end @@ -56,8 +64,9 @@ end context "a directory index url" do + let(:content_type) { "text/html" } + before do - Paperclip::UriAdapter.any_instance.stubs(:download_content).returns(StringIO.new("xxx")) @uri = URI.parse("http://thoughtbot.com") @subject = Paperclip.io_adapters.for(@uri) end @@ -73,7 +82,6 @@ context "a url with query params" do before do - Paperclip::UriAdapter.any_instance.stubs(:download_content).returns(StringIO.new("xxx")) @uri = URI.parse("https://github.com/thoughtbot/paperclip?file=test") @subject = Paperclip.io_adapters.for(@uri) end @@ -83,9 +91,26 @@ end end + context "a url with content disposition headers" do + let(:file_name) { "test_document.pdf" } + let(:meta) do + { + "content-disposition" => "attachment; filename=\"#{file_name}\";", + } + end + + before do + @uri = URI.parse("https://github.com/thoughtbot/paperclip?file=test") + @subject = Paperclip.io_adapters.for(@uri) + end + + it "returns a file name" do + assert_equal file_name, @subject.original_filename + end + end + context "a url with restricted characters in the filename" do before do - Paperclip::UriAdapter.any_instance.stubs(:download_content).returns(StringIO.new("xxx")) @uri = URI.parse("https://github.com/thoughtbot/paper:clip.jpg") @subject = Paperclip.io_adapters.for(@uri) end