From f2bab8b5f4599fe9c4788a9981cfdb497fe8a10d Mon Sep 17 00:00:00 2001
From: MAEDA Go <maeda@farend.jp>
Date: Mon, 31 Aug 2026 13:22:49 +0900
Subject: [PATCH] Send PDF attachments as a download from the download action.

---
 app/controllers/attachments_controller.rb     |  7 +++++--
 app/views/attachments/pdf.html.erb            |  2 +-
 .../functional/attachments_controller_test.rb | 19 ++++++++++++++++---
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
index 92cf7b760..b146b16e9 100644
--- a/app/controllers/attachments_controller.rb
+++ b/app/controllers/attachments_controller.rb
@@ -79,7 +79,6 @@ class AttachmentsController < ApplicationController
     end

     if stale?(:etag => @attachment.digest, :template => false)
-      # PDFs are sent inline
       send_file @attachment.diskfile, :filename => filename_for_content_disposition(@attachment.filename),
                                       :type => detect_content_type(@attachment),
                                       :disposition => disposition(@attachment)
@@ -313,8 +312,12 @@ class AttachmentsController < ApplicationController
     content_type
   end

+  # Attachments are always sent as a download, except when an inline
+  # disposition is explicitly requested for a PDF compatible file, as done by
+  # the PDF preview. Other content types must not be sent inline to prevent
+  # them from being rendered by the browser (e.g. XSS with HTML or SVG files)
   def disposition(attachment)
-    if detect_content_type(attachment) == 'application/pdf'
+    if params[:disposition] == 'inline' && detect_content_type(attachment) == 'application/pdf'
       'inline'
     else
       'attachment'
diff --git a/app/views/attachments/pdf.html.erb b/app/views/attachments/pdf.html.erb
index 441e6155c..6125fda90 100644
--- a/app/views/attachments/pdf.html.erb
+++ b/app/views/attachments/pdf.html.erb
@@ -1,4 +1,4 @@
 <%= render :layout => 'layouts/file' do %>
   <%= render :partial => 'common/pdf',
-             :locals => {:path => download_named_attachment_path(@attachment, @attachment.filename)} %>
+             :locals => {:path => download_named_attachment_path(@attachment, @attachment.filename, :disposition => 'inline')} %>
 <% end %>
diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb
index 4050dcaa9..76b61fb74 100644
--- a/test/functional/attachments_controller_test.rb
+++ b/test/functional/attachments_controller_test.rb
@@ -262,7 +262,7 @@ class AttachmentsControllerTest < Redmine::ControllerTest
     assert_response :success
     assert_equal 'text/html', @response.media_type

-    path = download_named_attachment_path(attachments(:attachments_023), attachments(:attachments_023).filename)
+    path = download_named_attachment_path(attachments(:attachments_023), attachments(:attachments_023).filename, :disposition => 'inline')
     assert_select ".filecontent.pdf object[data='#{path}']"
     assert_select '.nodata', :text => 'No preview available'
   end
@@ -423,7 +423,7 @@ class AttachmentsControllerTest < Redmine::ControllerTest
     assert_select 'div.filecontent.pdf object[type=?]', 'application/pdf'
   end

-  def test_download_pdf_compatible_illustrator_file_should_be_sent_inline_as_pdf
+  def test_download_pdf_compatible_illustrator_file_should_be_sent_as_a_download
     set_tmp_attachments_directory
     attachment = Attachment.create!(
       :file => mock_file_with_options(
@@ -436,11 +436,24 @@ class AttachmentsControllerTest < Redmine::ControllerTest

     get(:download, :params => {:id => attachment.id})
     assert_response :success
-    # Sent as PDF so that browsers display it inline
+    assert_equal 'application/pdf', @response.media_type
+    assert_match %r{\Aattachment}, @response.headers['Content-Disposition']
+  end
+
+  def test_download_pdf_file_with_inline_disposition_param_should_be_sent_inline
+    @request.session[:user_id] = 2
+    get(:download, :params => {:id => 23, :disposition => 'inline'})
+    assert_response :success
     assert_equal 'application/pdf', @response.media_type
     assert_match %r{\Ainline}, @response.headers['Content-Disposition']
   end

+  def test_download_non_pdf_file_with_inline_disposition_param_should_be_sent_as_a_download
+    get(:download, :params => {:id => 4, :disposition => 'inline'})
+    assert_response :success
+    assert_match %r{\Aattachment}, @response.headers['Content-Disposition']
+  end
+
   def test_download_version_file_with_issue_tracking_disabled
     Project.find(1).disable_module! :issue_tracking
     get(:download, :params => {:id => 9})
--
2.55.0