The uri io adapter should use the content-disposition filename (#2250)

- 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

 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):
 

lib/paperclip/io_adapters/uri_adapter.rb

@@ -2,6 +2,8 @@ require 'open-uri'
 
 module Paperclip
   class UriAdapter < AbstractAdapter
+    attr_writer :content_type
+
     def initialize(target)
       @target = target
       @content = download_content
@@ -9,25 +11,41 @@ module Paperclip
       @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)

spec/paperclip/io_adapters/http_url_proxy_adapter_spec.rb

 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 @@ describe Paperclip::HttpUrlProxyAdapter do
 
   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 @@ describe Paperclip::HttpUrlProxyAdapter do
 
   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 @@ describe Paperclip::HttpUrlProxyAdapter do
   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"\

spec/paperclip/io_adapters/uri_adapter_spec.rb

 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 @@ describe Paperclip::UriAdapter do
   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 @@ describe Paperclip::UriAdapter do
 
   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 @@ describe Paperclip::UriAdapter do
     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 Paperclip::UriAdapter do
 
   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