Encode Content-Disposition filenames according to RFC 2231

Closes #30134.

6 files changed, 74 insertions, 4 deletions

diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index 2e627ef118..664a53a778 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -127,7 +127,7 @@ class ActiveStorage::Blob < ActiveRecord::Base
   # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And
   # it allows permanent URLs that redirect to the +service_url+ to be cached in the view.
   def service_url(expires_in: 5.minutes, disposition: :inline)
-    service.url key, expires_in: expires_in, disposition: "#{disposition}; filename=\"#{filename}\"", filename: filename, content_type: content_type
+    service.url key, expires_in: expires_in, disposition: "#{disposition}; #{filename.parameters}", filename: filename, content_type: content_type
   end
 
   # Returns a URL that can be used to directly upload a file for this blob on the service. This URL is intended to be
diff --git a/activestorage/app/models/active_storage/filename.rb b/activestorage/app/models/active_storage/filename.rb
index df21078718..c2ad5c844c 100644
--- a/activestorage/app/models/active_storage/filename.rb
+++ b/activestorage/app/models/active_storage/filename.rb
@@ -34,6 +34,10 @@ class ActiveStorage::Filename
     @filename.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-")
   end
 
+  def parameters
+    Parameters.new self
+  end
+
   # Returns the sanitized version of the filename.
   def to_s
     sanitized.to_s
diff --git a/activestorage/app/models/active_storage/filename/parameters.rb b/activestorage/app/models/active_storage/filename/parameters.rb
new file mode 100644
index 0000000000..f8f54ba8eb
--- /dev/null
+++ b/activestorage/app/models/active_storage/filename/parameters.rb
@@ -0,0 +1,34 @@
+class ActiveStorage::Filename::Parameters
+  attr_reader :filename
+
+  def initialize(filename)
+    @filename = filename
+  end
+
+  def combined
+    "#{ascii}; #{utf8}"
+  end
+
+  TRADITIONAL_ESCAPED_CHAR = /[^ A-Za-z0-9!#$+.^_`|~-]/
+
+  def ascii
+    'filename="' + percent_escape(I18n.transliterate(filename.sanitized), TRADITIONAL_ESCAPED_CHAR) + '"'
+  end
+
+  RFC_5987_ESCAPED_CHAR = /[^A-Za-z0-9!#$&+.^_`|~-]/
+
+  def utf8
+    "filename*=UTF-8''" + percent_escape(filename.sanitized, RFC_5987_ESCAPED_CHAR)
+  end
+
+  def to_s
+    combined
+  end
+
+  private
+    def percent_escape(string, pattern)
+      string.gsub(pattern) do |char|
+        char.bytes.map { |byte| "%%%02X" % byte }.join
+      end
+    end
+end
diff --git a/activestorage/test/controllers/disk_controller_test.rb b/activestorage/test/controllers/disk_controller_test.rb
index eacb9bfc11..940dbf5918 100644
--- a/activestorage/test/controllers/disk_controller_test.rb
+++ b/activestorage/test/controllers/disk_controller_test.rb
@@ -8,7 +8,7 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
     blob = create_blob
 
     get blob.service_url
-    assert_equal "inline; filename=\"hello.txt\"", @response.headers["Content-Disposition"]
+    assert_equal "inline; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", @response.headers["Content-Disposition"]
     assert_equal "text/plain", @response.headers["Content-Type"]
   end
 
@@ -16,7 +16,7 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
     blob = create_blob
 
     get blob.service_url(disposition: :attachment)
-    assert_equal "attachment; filename=\"hello.txt\"", @response.headers["Content-Disposition"]
+    assert_equal "attachment; filename=\"hello.txt\"; filename*=UTF-8''hello.txt", @response.headers["Content-Disposition"]
     assert_equal "text/plain", @response.headers["Content-Type"]
   end
 
diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
index 03bf72c0de..6e815997ba 100644
--- a/activestorage/test/models/blob_test.rb
+++ b/activestorage/test/models/blob_test.rb
@@ -50,7 +50,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
 
   private
     def expected_url_for(blob, disposition: :inline)
-      query_string = { content_type: blob.content_type, disposition: "#{disposition}; filename=\"#{blob.filename}\"" }.to_param
+      query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{blob.filename.parameters}" }.to_param
       "/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{blob.filename}?#{query_string}"
     end
 end
diff --git a/activestorage/test/models/filename/parameters_test.rb b/activestorage/test/models/filename/parameters_test.rb
new file mode 100644
index 0000000000..431be00639
--- /dev/null
+++ b/activestorage/test/models/filename/parameters_test.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class ActiveStorage::Filename::ParametersTest < ActiveSupport::TestCase
+  test "parameterizing a Latin filename" do
+    filename = ActiveStorage::Filename.new("racecar.jpg")
+
+    assert_equal %(filename="racecar.jpg"), filename.parameters.ascii
+    assert_equal "filename*=UTF-8''racecar.jpg", filename.parameters.utf8
+    assert_equal "#{filename.parameters.ascii}; #{filename.parameters.utf8}", filename.parameters.combined
+    assert_equal filename.parameters.combined, filename.parameters.to_s
+  end
+
+  test "parameterizing a Latin filename with accented characters" do
+    filename = ActiveStorage::Filename.new("råcëçâr.jpg")
+
+    assert_equal %(filename="racecar.jpg"), filename.parameters.ascii
+    assert_equal "filename*=UTF-8''r%C3%A5c%C3%AB%C3%A7%C3%A2r.jpg", filename.parameters.utf8
+    assert_equal "#{filename.parameters.ascii}; #{filename.parameters.utf8}", filename.parameters.combined
+    assert_equal filename.parameters.combined, filename.parameters.to_s
+  end
+
+  test "parameterizing a non-Latin filename" do
+    filename = ActiveStorage::Filename.new("автомобиль.jpg")
+
+    assert_equal %(filename="%3F%3F%3F%3F%3F%3F%3F%3F%3F%3F.jpg"), filename.parameters.ascii
+    assert_equal "filename*=UTF-8''%D0%B0%D0%B2%D1%82%D0%BE%D0%BC%D0%BE%D0%B1%D0%B8%D0%BB%D1%8C.jpg", filename.parameters.utf8
+    assert_equal "#{filename.parameters.ascii}; #{filename.parameters.utf8}", filename.parameters.combined
+    assert_equal filename.parameters.combined, filename.parameters.to_s
+  end
+end