Better handling of the content-disposition header

Encountered an issue where the URI was returning header with
content-disposition where the filename value wasn't enclosed in the
double quotes. Turns out that this is a valid grammar according to
RFC6266. Also made the logic more robust to account for spaces and
uppercase letters.

NEWS

+master:
+
+* Improvement: Better handling of the content-disposition header. Now supports file name that is either
+  enclosed or not in double quotes and is case insensitive as per RC6266 grammar
+
 6.0.0 (2018-03-09):
 
 * Improvement: Depend only on `aws-sdk-s3` instead of `aws-sdk` (https://github.com/thoughtbot/paperclip/pull/2481)

lib/paperclip/io_adapters/uri_adapter.rb

@@ -34,9 +34,13 @@ module Paperclip
     end
 
     def filename_from_content_disposition
-      if @content.meta.key?("content-disposition")
-        matches = @content.meta["content-disposition"].match(/filename="([^"]*)"/)
-        matches[1].presence if matches
+      if @content.meta.key?("content-disposition") && @content.meta["content-disposition"].match(/filename/i)
+        # can include both filename and filename* values according to RCF6266. filename should come first
+        _, filename = @content.meta["content-disposition"].split(/filename\*?\s*=\s*/i)
+
+        # filename can be enclosed in quotes or not
+        matches = filename.match(/"(.*)"/)
+        matches ? matches[1] : filename.split(';')[0]
       end
     end
 

spec/paperclip/io_adapters/uri_adapter_spec.rb

@@ -105,21 +105,55 @@ describe Paperclip::UriAdapter do
 
   context "a url with content disposition headers" do
     let(:file_name) { "test_document.pdf" }
-    let(:meta) do
-      {
-        "content-disposition" => "attachment; filename=\"#{file_name}\";",
-      }
-    end
+    let(:filename_from_path) { "paperclip" }
 
     before do
       Paperclip::UriAdapter.any_instance.
         stubs(:download_content).returns(@open_return)
 
-      @uri = URI.parse("https://github.com/thoughtbot/paperclip?file=test")
+      @uri = URI.parse(
+        "https://github.com/thoughtbot/#{filename_from_path}?file=test")
+    end
+
+    it "returns file name from path" do
+      meta["content-disposition"] = "inline;"
+
       @subject = Paperclip.io_adapters.for(@uri)
+
+      assert_equal filename_from_path, @subject.original_filename
     end
 
-    it "returns a file name" do
+    it "returns a file name enclosed in double quotes" do
+      file_name = "john's test document.pdf"
+      meta["content-disposition"] = "attachment; filename=\"#{file_name}\";"
+
+      @subject = Paperclip.io_adapters.for(@uri)
+
+      assert_equal file_name, @subject.original_filename
+    end
+
+    it "returns a file name not enclosed in double quotes" do
+      meta["content-disposition"] = "ATTACHMENT; FILENAME=#{file_name};"
+
+      @subject = Paperclip.io_adapters.for(@uri)
+
+      assert_equal file_name, @subject.original_filename
+    end
+
+    it "does not crash when an empty filename is given" do
+      meta["content-disposition"] = "ATTACHMENT; FILENAME=\"\";"
+
+      @subject = Paperclip.io_adapters.for(@uri)
+
+      assert_equal "", @subject.original_filename
+    end
+
+    it "returns a file name ignoring RFC 5987 encoding" do
+      meta["content-disposition"] =
+        "attachment; filename=#{file_name}; filename* = utf-8''%e2%82%ac%20rates"
+
+      @subject = Paperclip.io_adapters.for(@uri)
+
       assert_equal file_name, @subject.original_filename
     end
 
@@ -127,6 +161,9 @@ describe Paperclip::UriAdapter do
       let(:file_name) { "test_document..pdf" }
 
       it "returns a file name" do
+        @uri = URI.parse(
+          "https://github.com/thoughtbot/#{file_name}?file=test")
+        @subject = Paperclip.io_adapters.for(@uri)
         assert_equal file_name, @subject.original_filename
       end
     end