diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 6b00ab3b2..4bea52fca 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -19,12 +19,20 @@ require "digest" require "fileutils" +require "tmpdir" require "zip" class Attachment < ApplicationRecord include Redmine::SafeAttributes belongs_to :container, :polymorphic => true belongs_to :author, :class_name => "User" + MARKDOWNIZED_PREVIEW_IMAGE_PREFIX = 'markdownized_preview_image_' + # Other attachments of this attachment are not images of the preview. + scope :named_as_markdownized_preview_image, lambda { + where("#{table_name}.filename LIKE ?", "#{sanitize_sql_like(MARKDOWNIZED_PREVIEW_IMAGE_PREFIX)}%") + } + has_many :markdownized_preview_images, lambda {named_as_markdownized_preview_image.order(:id)}, + :class_name => 'Attachment', :as => :container, :dependent => :destroy, :inverse_of => :container validates_presence_of :filename, :author validates_length_of :filename, :maximum => 255 @@ -92,6 +100,8 @@ class Attachment < ApplicationRecord File.join(storage_path, 'derived_cache', 'markdownized_previews') end + MARKDOWNIZED_PREVIEW_IMAGE_REFERENCE = /(?<=src="|\]\()#{MARKDOWNIZED_PREVIEW_IMAGE_PREFIX}\d+\.[a-z]+\b/ + before_create :set_content_type, :files_to_final_location before_update :reset_content_type, :if => :filename_changed? after_commit :delete_from_disk, :on => :destroy @@ -247,6 +257,20 @@ class Attachment < ApplicationRecord end end + # The images of a markdownized preview are visible with the previewed + # attachment. They are generated, so they can't be changed separately. + def attachments_visible?(user=User.current) + visible?(user) + end + + def attachments_editable?(user=User.current) + false + end + + def attachments_deletable?(user=User.current) + false + end + def image? !!(self.filename =~ /\.(avif|bmp|gif|jpg|jpe|jpeg|png|webp)$/i) end @@ -303,6 +327,7 @@ class Attachment < ApplicationRecord Dir.glob(File.join(markdownized_previews_storage_path, "*.md")).each do |file| File.delete file end + where(:container_type => name).named_as_markdownized_preview_image.find_each(&:destroy) end def is_text? @@ -358,9 +383,10 @@ class Attachment < ApplicationRecord return nil unless markdownized_previewable? target = markdownized_preview_cache_path - if Redmine::Markdownizer.convert(diskfile, target) - File.read(target, :mode => "rb") - end + content = File.read(target, :mode => "rb") if File.exist?(target) + return content if content && markdownized_preview_images_complete?(content) + + generate_markdownized_preview(target) rescue => e if logger logger.error( @@ -600,6 +626,38 @@ class Attachment < ApplicationRecord # anymore, thats why this is caught and ignored as well. end + # The cached preview is shared by the attachments with the same file, but + # each of them has its own images: another one may have created the preview + def markdownized_preview_images_complete?(content) + filenames = content.scan(MARKDOWNIZED_PREVIEW_IMAGE_REFERENCE).uniq + (filenames - markdownized_preview_images.map(&:filename)).empty? + end + + def generate_markdownized_preview(target) + Dir.mktmpdir('markdownized-preview') do |directory| + next unless Redmine::Markdownizer.convert(diskfile, target, directory) + + content = File.read(target, :mode => "rb") + # The lock is not held during the conversion, which can take a long time. + # Another request may have stored the images in the meantime: they are + # kept because pages may already display them. + with_lock do + next if markdownized_preview_images_complete?(content) + + markdownized_preview_images.destroy_all + Dir.children(directory).sort.each do |filename| + File.open(File.join(directory, filename), 'rb') do |file| + # Validations are skipped because these are not uploaded files: + # the size and extension limits are not meant for them + image = markdownized_preview_images.build(:file => file, :filename => filename, :author => author) + image.save(:validate => false) + end + end + end + content + end + end + # Physically deletes the file from the file system def delete_from_disk! FileUtils.rm_f(diskfile) if disk_filename.present? diff --git a/app/views/attachments/markdownized.html.erb b/app/views/attachments/markdownized.html.erb index 3a158e827..ca211f13c 100644 --- a/app/views/attachments/markdownized.html.erb +++ b/app/views/attachments/markdownized.html.erb @@ -1,4 +1,5 @@ <%= render :layout => 'layouts/file' do %> <%= render :partial => 'common/markup', - :locals => {:markup_text_formatting => 'common_mark', :markup_text => @content} %> + :locals => {:markup_text_formatting => 'common_mark', :markup_text => @content, + :attachments => @attachment.markdownized_preview_images.to_a} %> <% end %> diff --git a/app/views/common/_markup.html.erb b/app/views/common/_markup.html.erb index 14786a87c..71507b7c7 100644 --- a/app/views/common/_markup.html.erb +++ b/app/views/common/_markup.html.erb @@ -1,3 +1,3 @@
- <%= Redmine::WikiFormatting.to_html(markup_text_formatting, Redmine::CodesetUtil.to_utf8_by_setting(markup_text)).html_safe %> + <%= Redmine::WikiFormatting.to_html(markup_text_formatting, Redmine::CodesetUtil.to_utf8_by_setting(markup_text), :attachments => local_assigns[:attachments], :view => self, :only_path => true).html_safe %>
diff --git a/extra/pandoc_filter/markdownized_preview_images.lua b/extra/pandoc_filter/markdownized_preview_images.lua new file mode 100644 index 000000000..2e08a469a --- /dev/null +++ b/extra/pandoc_filter/markdownized_preview_images.lua @@ -0,0 +1,35 @@ +-- Writes the images embedded in the document to the current directory, with +-- names that do not depend on the document. Images that are not embedded in +-- the document are never fetched, and formats that browsers cannot display +-- are replaced with their description. +local extensions = { + ["image/avif"] = "avif", + ["image/bmp"] = "bmp", + ["image/gif"] = "gif", + ["image/jpeg"] = "jpg", + ["image/png"] = "png", + ["image/webp"] = "webp" +} +local filenames = {} +local count = 0 + +function Image(el) + local mime, contents = pandoc.mediabag.lookup(el.src) + local extension = extensions[mime] + + if not extension then + local description = pandoc.utils.stringify(el.caption) + return pandoc.Str("[" .. (description ~= "" and description or el.src:match("[^/]*$")) .. "]") + end + + if not filenames[el.src] then + count = count + 1 + filenames[el.src] = "markdownized_preview_image_" .. count .. "." .. extension + local file = assert(io.open(filenames[el.src], "wb")) + file:write(contents) + file:close() + end + + el.src = filenames[el.src] + return el +end diff --git a/lib/redmine/markdownizer.rb b/lib/redmine/markdownizer.rb index 9bca4db8a..dc62f93a6 100644 --- a/lib/redmine/markdownizer.rb +++ b/lib/redmine/markdownizer.rb @@ -30,14 +30,14 @@ module Redmine PREVIEW_GENERATION_TIMEOUT = Redmine::Configuration['markdownized_preview_generation_timeout'].to_i MAX_SOURCE_SIZE = Redmine::Configuration['markdownized_preview_max_source_size'].to_i MAX_OUTPUT_SIZE = Redmine::Configuration['markdownized_preview_max_output_size'].to_i + IMAGES_FILTER = Rails.root.join('extra', 'pandoc_filter', 'markdownized_preview_images.lua').to_s.freeze def self.supports?(filename) markdownizable_extensions.include?(File.extname(filename.to_s).downcase) end - def self.convert(source, target) + def self.convert(source, target, media_directory) return nil unless available? - return target if File.exist?(target) if File.size(source) > MAX_SOURCE_SIZE logger.warn("Markdownized preview generation skipped because source file is too large (#{File.size(source)} bytes): #{source}") @@ -46,13 +46,13 @@ module Redmine directory = File.dirname(target) FileUtils.mkdir_p(directory) - args = [COMMAND, source, "-t", "gfm"] + args = [COMMAND, File.expand_path(source), "-t", "gfm", *image_options] pid = nil output = Tempfile.new('markdownized-preview') begin Timeout.timeout(PREVIEW_GENERATION_TIMEOUT) do - pid = Process.spawn(*args, out: output.path) + pid = Process.spawn(*args, chdir: media_directory, out: output.path) _, status = Process.wait2(pid) unless status.success? logger.error("Markdownized preview generation failed (#{status.exitstatus}):\nCommand: #{args.shelljoin}") @@ -80,6 +80,14 @@ module Redmine output&.unlink end + def self.images_available? + available? && (@pandoc_version <=> [2, 15]) >= 0 + end + + def self.image_options + images_available? ? ["--sandbox", "--quiet", "--lua-filter=#{IMAGES_FILTER}"] : [] + end + def self.available? return @available if defined?(@available) diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb index 8ac686ee0..6279fe640 100644 --- a/test/functional/attachments_controller_test.rb +++ b/test/functional/attachments_controller_test.rb @@ -316,6 +316,54 @@ class AttachmentsControllerTest < Redmine::ControllerTest assert_select '.nodata', :count => 0 end + def test_show_msword_should_display_images_of_the_document + skip unless Redmine::Markdownizer.images_available? + + set_tmp_attachments_directory + a = Attachment.create!( + :container => Issue.find(1), + :file => uploaded_test_file( + 'msword.docx', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' + ), + :author => User.find(1) + ) + + get(:show, :params => {:id => a.id}) + + assert_response :success + image = a.markdownized_preview_images.first + assert_select "div.filecontent.wiki img[src=?]", download_named_attachment_path(image, image.filename) + + get(:download, :params => {:id => image.id}) + assert_response :success + assert_equal 'image/png', @response.media_type + end + + def test_download_of_markdownized_preview_image_should_require_access_to_the_attachment + skip unless Redmine::Markdownizer.images_available? + + set_tmp_attachments_directory + a = Attachment.create!( + :container => Issue.find(4), + :file => uploaded_test_file( + 'msword.docx', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' + ), + :author => User.find(1) + ) + a.markdownized_preview_content + image = a.markdownized_preview_images.first + + @request.session[:user_id] = nil + get(:download, :params => {:id => image.id}) + assert_response :found + + @request.session[:user_id] = 2 + get(:download, :params => {:id => image.id}) + assert_response :success + end + def test_show_libreoffice_writer skip unless Redmine::Markdownizer.available? diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb index 8ad4b5973..96899c6ef 100644 --- a/test/unit/attachment_test.rb +++ b/test/unit/attachment_test.rb @@ -692,6 +692,201 @@ class AttachmentTest < ActiveSupport::TestCase assert_not File.exist?(preview) end + def docx_attachment + Attachment.create!( + :container => Issue.find(1), + :file => uploaded_test_file( + "msword.docx", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document" + ), + :author => User.find(1) + ) + end + private :docx_attachment + + def test_markdownized_preview_content_should_store_images_as_attachments + skip unless Redmine::Markdownizer.images_available? + + attachment = docx_attachment + content = attachment.markdownized_preview_content + + assert_include 'src="markdownized_preview_image_1.png"', content + assert_equal attachment.markdownized_preview_cache_path, + File.join(Attachment.markdownized_previews_storage_path, "#{attachment.digest}_#{attachment.filesize}.md") + assert File.exist?(attachment.markdownized_preview_cache_path) + image = attachment.reload.markdownized_preview_images.first + assert_equal 'markdownized_preview_image_1.png', image.filename + assert_equal 'image/png', image.content_type + assert_equal attachment, image.container + assert_equal attachment.project, image.project + assert image.readable? + end + + def test_markdownized_preview_content_should_use_the_cache + skip unless Redmine::Markdownizer.images_available? + + attachment = docx_attachment + content = attachment.markdownized_preview_content + + Redmine::Markdownizer.expects(:convert).never + assert_equal content, attachment.reload.markdownized_preview_content + assert_equal 1, attachment.markdownized_preview_images.count + end + + def test_markdownized_preview_content_should_ignore_image_filenames_in_the_text + attachment = docx_attachment + FileUtils.mkdir_p(Attachment.markdownized_previews_storage_path) + File.binwrite(attachment.markdownized_preview_cache_path, 'The image is markdownized_preview_image_7.png') + + Redmine::Markdownizer.expects(:convert).never + assert_include 'The image is', attachment.markdownized_preview_content + end + + def test_markdownized_preview_content_should_generate_missing_images + attachment = docx_attachment + FileUtils.mkdir_p(Attachment.markdownized_previews_storage_path) + Redmine::Markdownizer.expects(:convert).twice.returns(nil) + + ['![](markdownized_preview_image_1.png "Picture 1")', ''].each do |content| + File.binwrite(attachment.markdownized_preview_cache_path, content) + assert_nil attachment.markdownized_preview_content + end + end + + def test_markdownized_preview_content_should_keep_images_stored_by_a_concurrent_request + skip unless Redmine::Markdownizer.images_available? + + attachment = docx_attachment + attachment.markdownized_preview_content + image = attachment.reload.markdownized_preview_images.first + + # The other request found no cached preview before this one stored it + Attachment.find(attachment.id).send(:generate_markdownized_preview, attachment.markdownized_preview_cache_path) + assert_equal [image], attachment.reload.markdownized_preview_images.to_a + end + + def test_markdownized_preview_content_should_not_convert_in_a_transaction + skip unless Redmine::Markdownizer.available? + + attachment = docx_attachment + transactions = Attachment.lease_connection.open_transactions + transactions_during_conversion = nil + Redmine::Markdownizer.expects(:convert).with do + transactions_during_conversion = Attachment.lease_connection.open_transactions + true + end.returns(nil) + + assert_nil attachment.markdownized_preview_content + assert_equal transactions, transactions_during_conversion + end + + def test_markdownized_preview_content_should_store_images_for_each_attachment_with_the_same_file + skip unless Redmine::Markdownizer.images_available? + + attachment = docx_attachment + attachment.markdownized_preview_content + copy = attachment.copy(:container => Issue.find(2)) + copy.save! + assert File.exist?(copy.markdownized_preview_cache_path) + + assert_include 'markdownized_preview_image_1.png', copy.markdownized_preview_content + image = copy.reload.markdownized_preview_images.first + assert_equal 'markdownized_preview_image_1.png', image.filename + + attachment.destroy + assert image.reload.readable? + end + + def test_markdownizer_should_export_images_only_with_the_sandbox + skip unless Redmine::Markdownizer.available? + + version = Redmine::Markdownizer.instance_variable_get(:@pandoc_version) + begin + Redmine::Markdownizer.instance_variable_set(:@pandoc_version, [2, 14, 2]) + assert_not Redmine::Markdownizer.images_available? + assert_equal [], Redmine::Markdownizer.image_options + Redmine::Markdownizer.instance_variable_set(:@pandoc_version, [2, 15]) + assert Redmine::Markdownizer.images_available? + assert_equal ['--sandbox', '--quiet', "--lua-filter=#{Redmine::Markdownizer::IMAGES_FILTER}"], Redmine::Markdownizer.image_options + ensure + Redmine::Markdownizer.instance_variable_set(:@pandoc_version, version) + end + end + + def test_destroy_should_destroy_markdownized_preview_images + skip unless Redmine::Markdownizer.images_available? + + attachment = docx_attachment + attachment.markdownized_preview_content + image = attachment.reload.markdownized_preview_images.first + diskfile = image.diskfile + + assert_difference 'Attachment.count', -2 do + attachment.destroy + end + assert_not File.exist?(diskfile) + end + + def test_clear_markdownized_previews_should_destroy_only_preview_images + skip unless Redmine::Markdownizer.images_available? + + attachment = docx_attachment + attachment.markdownized_preview_content + image = attachment.reload.markdownized_preview_images.first + other = Attachment.create!( + :container => attachment, + :file => uploaded_test_file("testfile.txt", "text/plain"), + :filename => 'other.txt', + :author => User.find(1) + ) + + Attachment.clear_markdownized_previews + + assert_not File.exist?(attachment.markdownized_preview_cache_path) + assert_not Attachment.exists?(image.id) + assert Attachment.exists?(attachment.id) + assert Attachment.exists?(other.id) + + assert_include 'markdownized_preview_image_1.png', attachment.reload.markdownized_preview_content + assert_equal ['markdownized_preview_image_1.png'], attachment.reload.markdownized_preview_images.map(&:filename) + assert Attachment.exists?(other.id) + end + + def test_markdownized_preview_images_should_have_the_permissions_of_the_attachment + skip unless Redmine::Markdownizer.images_available? + + attachment = docx_attachment + attachment.markdownized_preview_content + image = attachment.reload.markdownized_preview_images.first + + assert image.visible?(User.find(2)) + assert_equal attachment.visible?(User.anonymous), image.visible?(User.anonymous) + assert_not image.editable?(User.find(1)) + assert_not image.deletable?(User.find(1)) + + Issue.find(1).update_column(:is_private, true) + attachment.reload.container.reload + assert_not image.reload.visible?(User.find(7)) + end + + def test_markdownized_preview_content_should_not_read_files_referenced_by_a_document + skip unless Redmine::Markdownizer.images_available? + + attachment = Attachment.create!( + :container => Issue.find(1), + :file => uploaded_test_file( + "libreoffice-writer-linked-image.odt", + "application/vnd.oasis.opendocument.text" + ), + :author => User.find(1) + ) + + content = attachment.markdownized_preview_content + assert_include 'hostname', content + assert_not_include 'src=', content + assert_equal 0, attachment.reload.markdownized_preview_images.count + end + if convert_installed? def test_thumbnail_should_generate_the_thumbnail set_fixtures_attachments_directory