Browse files

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
1 parent 85ebe9d commit e367efc2a44d8f137aad35facd9d4f61749204af @netmask netmask committed with tute Jul 23, 2016
View
1 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):
View
42 lib/paperclip/io_adapters/uri_adapter.rb
@@ -2,32 +2,50 @@
module Paperclip
class UriAdapter < AbstractAdapter
+ attr_writer :content_type
+
def initialize(target)
@target = target
@content = download_content
cache_current_values
@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)
View
15 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"\
View
52 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

0 comments on commit e367efc

Please sign in to comment.