Add the content type for external URLs

app/models/attachment.rb

@@ -93,7 +93,9 @@ class Attachment < ApplicationRecord
   # specifically when using S3 for attachments. In the case of S3 the file name for the downloaded
   # file will still be correct as it's part of the URL before the query.
   def external_url_options(expires_in: nil)
-    { content_disposition: content_disposition(include_filename: false), expires_in: }
+    { content_disposition: content_disposition(include_filename: false),
+      content_type: served_content_type,
+      expires_in: }
   end
 
   def external_storage?
@@ -119,11 +121,24 @@ class Attachment < ApplicationRecord
     end
   end
 
+  # Returns the Content-Type to use when serving this file inline in a browser.
+  # Text files are normalised to text/plain (prevents script execution) with an
+  # explicit charset. Non-inlineable files get application/octet-stream so the
+  # browser is forced to download them.
+  def served_content_type
+    if is_text?
+      "text/plain; charset=#{charset.presence || Setting.attachment_default_charset}"
+    elsif inlineable?
+      content_type
+    else
+      "application/octet-stream"
+    end
+  end
+
   # Returns the content type to use when serving the file to a browser.
   # For text files, ensures a charset is always present so browsers don't
-  # fall back to ISO-8859-1.
-  # We use a configurable fallback (default utf-8) so that administrators
-  # can control content types for previously uploaded attachments
+  # fall back to ISO-8859-1. Preserves the real MIME subtype (e.g. text/x-ruby)
+  # unlike served_content_type which normalises to text/plain for security.
   def serving_content_type
     return content_type unless is_text?
 

app/uploaders/fog_file_uploader.rb

@@ -74,6 +74,7 @@ class FogFileUploader < CarrierWave::Uploader::Base
   #
   # @param options [Hash] Options hash.
   # @option options [String] :content_disposition Pass this content disposition to S3 so that it serves the file with it.
+  # @option options [String] :content_type Pass this content type to S3 so that it serves the file with it.
   # @option options [DateTime] :expires_at Date at which the link should expire (default: now + 5 minutes)
   # @option options [ActiveSupport::Duration] :expires_in Duration in which the link should expire.
   #
@@ -82,6 +83,7 @@ class FogFileUploader < CarrierWave::Uploader::Base
     url_options = {}
 
     set_content_disposition!(url_options, options:)
+    set_content_type!(url_options, options:)
     set_expires_at!(url_options, options:)
 
     remote_file.url url_options
@@ -103,15 +105,17 @@ class FogFileUploader < CarrierWave::Uploader::Base
   private
 
   def set_content_disposition!(url_options, options:)
-    if options[:content_disposition].present?
-      url_options[:query] = {
-        # Passing this option to S3 will make it serve the file with the
-        # respective content disposition. Without it no content disposition
-        # header is sent. This only works for S3 but we don't support
-        # anything else anyway (see carrierwave.rb).
-        "response-content-disposition" => options[:content_disposition]
-      }
-    end
+    return if options[:content_disposition].blank?
+
+    (url_options[:query] ||= {})["response-content-disposition"] = options[:content_disposition]
+  end
+
+  def set_content_type!(url_options, options:)
+    return if options[:content_type].blank?
+
+    # Like the content disposition above, this makes S3 serve the file with the
+    # given Content-Type, overriding the stored object type.
+    (url_options[:query] ||= {})["response-content-type"] = options[:content_type]
   end
 
   def set_expires_at!(url_options, options:)

lib/api/helpers/attachment_renderer.rb

@@ -100,17 +100,7 @@ module API
       end
 
       def attachment_content_type(attachment)
-        if attachment.is_text?
-          # Even if the text mime type might differ, always output plain text
-          # so this doesn't get interpreted as e.g., a script or html file
-          charset = attachment.charset.presence || Setting.attachment_default_charset
-          "text/plain; charset=#{charset}"
-        elsif attachment.inlineable?
-          attachment.content_type
-        else
-          # For security reasons, mark all non-inlinable files as an octet-stream first
-          "application/octet-stream"
-        end
+        attachment.served_content_type
       end
 
       def set_cache_headers

spec/models/attachment_spec.rb

@@ -320,6 +320,11 @@ RSpec.describe Attachment do
       it_behaves_like "it uses content disposition inline" do
         let(:attachment) { text_attachment }
       end
+
+      it "includes response-content-type with text/plain and the detected charset in the S3 URL" do
+        url = text_attachment.external_url.to_s
+        expect(url).to include "response-content-type=text%2Fplain%3B%20charset%3D"
+      end
     end
 
     describe "for a video file" do
@@ -351,6 +356,10 @@ RSpec.describe Attachment do
         expect(binary_attachment.content_disposition).to eq "attachment; filename=textfile.txt.gz"
         expect(binary_attachment.external_url.to_s).to include "response-content-disposition=attachment"
       end
+
+      it "includes response-content-type application/octet-stream in the S3 URL" do
+        expect(binary_attachment.external_url.to_s).to include "response-content-type=application%2Foctet-stream"
+      end
     end
   end