FIX: Correctly encode non-ASCII filenames in HTTP header

Backport of fix from Rails 6: 890485cfce

lib/file_store/s3_store.rb

@@ -113,7 +113,12 @@ module FileStore
     def url_for(upload, force_download: false)
       if upload.private? || force_download
         opts = { expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS }
-        opts[:response_content_disposition] = "attachment; filename=\"#{upload.original_filename}\"" if force_download
+
+        if force_download
+          opts[:response_content_disposition] = ActionDispatch::Http::ContentDisposition.format(
+            disposition: "attachment", filename: upload.original_filename
+          )
+        end
 
         obj = @s3_helper.object(get_upload_key(upload))
         url = obj.presigned_url(:get, opts)

lib/freedom_patches/rails6.rb

@@ -9,3 +9,102 @@ if ! defined? ActionView::Base.with_view_paths
     end
   end
 end
+
+# backport of https://github.com/rails/rails/commit/890485cfce4c361c03a41ec23b0ba187007818cc
+if !defined? ActionDispatch::Http::ContentDisposition
+  module ActionDispatch
+    module Http
+      class ContentDisposition
+        def self.format(disposition:, filename:)
+          new(disposition: disposition, filename: filename).to_s
+        end
+
+        attr_reader :disposition, :filename
+
+        def initialize(disposition:, filename:)
+          @disposition = disposition
+          @filename = filename
+        end
+
+        TRADITIONAL_ESCAPED_CHAR = /[^ A-Za-z0-9!#$+.^_`|~-]/
+
+        def ascii_filename
+          'filename="' + percent_escape(I18n.transliterate(filename), TRADITIONAL_ESCAPED_CHAR) + '"'
+        end
+
+        RFC_5987_ESCAPED_CHAR = /[^A-Za-z0-9!#$&+.^_`|~-]/
+
+        def utf8_filename
+          "filename*=UTF-8''" + percent_escape(filename, RFC_5987_ESCAPED_CHAR)
+        end
+
+        def to_s
+          if filename
+            "#{disposition}; #{ascii_filename}; #{utf8_filename}"
+          else
+            "#{disposition}"
+          end
+        end
+
+        private
+        def percent_escape(string, pattern)
+          string.gsub(pattern) do |char|
+            char.bytes.map { |byte| "%%%02X" % byte }.join
+          end
+        end
+      end
+    end
+  end
+
+  module ActionController
+    module DataStreaming
+      private
+      def send_file_headers!(options)
+        type_provided = options.has_key?(:type)
+
+        content_type = options.fetch(:type, DEFAULT_SEND_FILE_TYPE)
+        self.content_type = content_type
+        response.sending_file = true
+
+        raise ArgumentError, ":type option required" if content_type.nil?
+
+        if content_type.is_a?(Symbol)
+          extension = Mime[content_type]
+          raise ArgumentError, "Unknown MIME type #{options[:type]}" unless extension
+          self.content_type = extension
+        else
+          if !type_provided && options[:filename]
+            # If type wasn't provided, try guessing from file extension.
+            content_type = Mime::Type.lookup_by_extension(File.extname(options[:filename]).downcase.delete(".")) || content_type
+          end
+          self.content_type = content_type
+        end
+
+        disposition = options.fetch(:disposition, DEFAULT_SEND_FILE_DISPOSITION)
+        if disposition
+          headers["Content-Disposition"] = ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: options[:filename])
+        end
+
+        headers["Content-Transfer-Encoding"] = "binary"
+
+        # Fix a problem with IE 6.0 on opening downloaded files:
+        # If Cache-Control: no-cache is set (which Rails does by default),
+        # IE removes the file it just downloaded from its cache immediately
+        # after it displays the "open/save" dialog, which means that if you
+        # hit "open" the file isn't there anymore when the application that
+        # is called for handling the download is run, so let's workaround that
+        response.cache_control[:public] ||= false
+      end
+    end
+  end
+
+  module ActiveStorage
+    class Service
+      private
+      def content_disposition_with(type: "inline", filename:)
+        disposition = (type.to_s.presence_in(%w( attachment inline )) || "inline")
+        ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename.sanitized)
+      end
+    end
+  end
+end

spec/components/file_store/s3_store_spec.rb

@@ -413,7 +413,7 @@ describe FileStore::S3Store do
       s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object)
       opts = {
         expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS,
-        response_content_disposition: "attachment; filename=\"#{upload.original_filename}\""
+        response_content_disposition: %Q|attachment; filename="#{upload.original_filename}"; filename*=UTF-8''#{upload.original_filename}|
       }
 
       s3_object.expects(:presigned_url).with(:get, opts)

spec/requests/uploads_controller_spec.rb

@@ -261,8 +261,8 @@ describe UploadsController do
       get "/uploads/#{site}/#{upload.sha1}.#{upload.extension}"
       expect(response.status).to eq(200)
 
-      # rails 6 adds UTF-8 filename to disposition
-      expect(response.headers["Content-Disposition"]).to include("attachment; filename=\"logo.png\"")
+      expect(response.headers["Content-Disposition"])
+        .to eq(%Q|attachment; filename="logo.png"; filename*=UTF-8''logo.png|)
     end
 
     it "handles image without extension" do
@@ -271,7 +271,8 @@ describe UploadsController do
 
       get "/uploads/#{site}/#{upload.sha1}.json"
       expect(response.status).to eq(200)
-      expect(response.headers["Content-Disposition"]).to include("attachment; filename=\"image_no_extension.png\"")
+      expect(response.headers["Content-Disposition"])
+        .to eq(%Q|attachment; filename="image_no_extension.png"; filename*=UTF-8''image_no_extension.png|)
     end
 
     it "handles file without extension" do
@@ -280,7 +281,8 @@ describe UploadsController do
 
       get "/uploads/#{site}/#{upload.sha1}.json"
       expect(response.status).to eq(200)
-      expect(response.headers["Content-Disposition"]).to include("attachment; filename=\"not_an_image\"")
+      expect(response.headers["Content-Disposition"])
+        .to eq(%Q|attachment; filename="not_an_image"; filename*=UTF-8''not_an_image|)
     end
 
     context "prevent anons from downloading files" do