From a008c688ab661089a152c016c75d468bdeabe20b 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 - Fix style comments - Applied the Tute Costa refactor to URI Adapter. - Added entry to the NEWS file. - Removed editor tracking file - Fix test cases --- NEWS | 1 + lib/paperclip/io_adapters/uri_adapter.rb | 42 ++++++++++----- .../http_url_proxy_adapter_spec.rb | 15 +++--- .../paperclip/io_adapters/uri_adapter_spec.rb | 52 ++++++++++++++++--- 4 files changed, 85 insertions(+), 25 deletions(-) diff --git a/NEWS b/NEWS index b80f327e4..bf7241ef1 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ master: +* (port from 4.3) Improvement: the `URI adapter` now uses the content-disposition header to name the downloaded file. 5.0.0 (2016-07-01): diff --git a/lib/paperclip/io_adapters/uri_adapter.rb b/lib/paperclip/io_adapters/uri_adapter.rb index 2156468b1..51d2e1192 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,25 +11,41 @@ def initialize(target) @tempfile = copy_to_tempfile(@content) end - attr_writer :content_type - private - def download_content - options = { read_timeout: Paperclip.options[:read_timeout] }.compact + def cache_current_values + self.content_type = content_type_from_content || "text/html" - open(@target, **options) + 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 + options = { read_timeout: Paperclip.options[:read_timeout] }.compact + + open(@target, **options) 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 001a27556..d8d95f7cd 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 @@ -101,7 +104,7 @@ context "a url with special characters in the filename" do it "returns a encoded filename" do Paperclip::HttpUrlProxyAdapter.any_instance.stubs(:download_content). - returns(StringIO.new("x")) + returns(@open_return) url = "https://github.com/thoughtbot/paperclip-öäü字´½♥زÈ.png" subject = Paperclip.io_adapters.for(url) filename = "paperclip-%C3%B6%C3%A4%C3%BC%E5%AD%97%C2%B4%C2%BD%E2%99%A5"\ diff --git a/spec/paperclip/io_adapters/uri_adapter_spec.rb b/spec/paperclip/io_adapters/uri_adapter_spec.rb index 288dc8327..86810acb8 100644 --- a/spec/paperclip/io_adapters/uri_adapter_spec.rb +++ b/spec/paperclip/io_adapters/uri_adapter_spec.rb @@ -1,11 +1,20 @@ 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) + 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) + 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 +65,12 @@ 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")) + Paperclip::UriAdapter.any_instance. + stubs(:download_content).returns(@open_return) + @uri = URI.parse("http://thoughtbot.com") @subject = Paperclip.io_adapters.for(@uri) end @@ -73,7 +86,9 @@ context "a url with query params" do before do - Paperclip::UriAdapter.any_instance.stubs(:download_content).returns(StringIO.new("xxx")) + Paperclip::UriAdapter.any_instance. + stubs(:download_content).returns(@open_return) + @uri = URI.parse("https://github.com/thoughtbot/paperclip?file=test") @subject = Paperclip.io_adapters.for(@uri) end @@ -83,9 +98,32 @@ 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 + Paperclip::UriAdapter.any_instance. + stubs(:download_content).returns(@open_return) + + @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")) + Paperclip::UriAdapter.any_instance. + stubs(:download_content).returns(@open_return) + @uri = URI.parse("https://github.com/thoughtbot/paper:clip.jpg") @subject = Paperclip.io_adapters.for(@uri) end @@ -101,7 +139,7 @@ describe "#download_content" do before do - Paperclip::UriAdapter.any_instance.stubs(:open).returns(StringIO.new("xxx")) + Paperclip::UriAdapter.any_instance.stubs(:open).returns(@open_return) @uri = URI.parse("https://github.com/thoughtbot/paper:clip.jpg") @subject = Paperclip.io_adapters.for(@uri) end