Project

General

Profile

Defect #44358 » 0001b-Fix-broken-images-in-markdownized-Office-document-pr.patch

Florian Walchshofer, 2026-10-20 23:05

View differences:

app/controllers/attachments_controller.rb
20 20
class AttachmentsController < ApplicationController
21 21
  include ActionView::Helpers::NumberHelper
22 22

  
23
  before_action :find_attachment, :only => [:show, :download, :thumbnail, :update, :destroy]
23
  before_action :find_attachment, :only => [:show, :download, :thumbnail, :update, :destroy, :preview_media]
24 24
  before_action :find_container, :only => [:edit_all, :update_all, :download_all]
25 25
  before_action :find_downloadable_attachments, :only => :download_all
26 26
  before_action :find_editable_attachments, :only => [:edit_all, :update_all]
27
  before_action :file_readable, :read_authorize, :only => [:show, :download, :thumbnail]
27
  before_action :file_readable, :read_authorize, :only => [:show, :download, :thumbnail, :preview_media]
28 28
  before_action :update_authorize, :only => :update
29 29
  before_action :delete_authorize, :only => :destroy
30 30
  before_action :authorize_global, :only => :upload
......
103 103
    end
104 104
  end
105 105

  
106
  def preview_media
107
    if (media = @attachment.preview_media(params[:path]))
108
      if stale?(etag: media, template: false)
109
        send_file(media,
110
          :type => Redmine::MimeType.of(media).presence || 'application/octet-stream',
111
          :filename => "preview_#{@attachment.id}_#{File.basename(media)}",
112
          :disposition => 'inline')
113
      end
114
    else
115
      head :not_found
116
    end
117
  end
118

  
106 119
  def upload
107 120
    # Make sure that API users get used to set this content type
108 121
    # as it won't trigger Rails' automatic parsing of the request body for parameters
app/models/attachment.rb
299 299
  end
300 300

  
301 301
  def self.clear_markdownized_previews
302
    Dir.glob(File.join(markdownized_previews_storage_path, "*.md")).each do |file|
303
      File.delete file
304
    end
302
    FileUtils.rm_rf(Dir.glob(File.join(markdownized_previews_storage_path, '*')))
305 303
  end
306 304

  
307 305
  def is_text?
......
357 355
    return nil unless markdownized_previewable?
358 356

  
359 357
    target = markdownized_preview_cache_path
360
    if Redmine::Markdownizer.convert(diskfile, target)
358
    if Redmine::Markdownizer.convert(diskfile, target, id)
361 359
      File.read(target, :mode => "rb")
362 360
    end
363 361
  rescue => e
......
370 368
    nil
371 369
  end
372 370

  
371
  def preview_media(path)
372
    base_path = File.expand_path(markdownized_preview_directory)
373
    file_path = File.expand_path(path, base_path)
374

  
375
    return nil unless file_path.start_with?(base_path + File::SEPARATOR)
376
    return nil unless File.file?(file_path)
377
    return nil unless Redmine::MimeType.is_type?('image', file_path)
378

  
379
    file_path
380
  end
381

  
382
  def markdownized_preview_directory
383
    File.join(self.class.markdownized_previews_storage_path, id.to_s)
384
  end
385

  
373 386
  def markdownized_preview_cache_path
374
    File.join(self.class.markdownized_previews_storage_path, "#{digest}_#{filesize}.md")
387
    File.join(markdownized_preview_directory, "preview.md")
375 388
  end
376 389

  
377 390
  def previewable?
......
605 618
    Dir[thumbnail_path("*")].each do |thumb|
606 619
      File.delete(thumb)
607 620
    end
608
    FileUtils.rm_f(markdownized_preview_cache_path)
621
    FileUtils.rm_rf(markdownized_preview_directory)
609 622
  end
610 623

  
611 624
  def thumbnail_path(size)
config/routes.rb
332 332
  get 'attachments/download/:id/:filename', :to => 'attachments#download', :id => /\d+/, :filename => /.*/, :as => 'download_named_attachment', :format => 'html'
333 333
  get 'attachments/download/:id', :to => 'attachments#download', :id => /\d+/
334 334
  get 'attachments/thumbnail/:id(/:size)', :to => 'attachments#thumbnail', :id => /\d+/, :size => /\d+/, :as => 'thumbnail'
335
  get 'attachments/markdownized_previews/:id/*path', :to => 'attachments#preview_media', :id => /\d+/, :format => false
335 336
  resources :attachments, :only => [:show, :update, :destroy]
336 337

  
337 338
  # register plugin object types with ObjectTypeConstraint.register_object_type(PluginModel.name.underscore.pluralize')
lib/redmine/markdownizer.rb
35 35
      markdownizable_extensions.include?(File.extname(filename.to_s).downcase)
36 36
    end
37 37

  
38
    def self.convert(source, target)
38
    def self.convert(source, target, attachment_id)
39 39
      return nil unless available?
40 40
      return target if File.exist?(target)
41 41

  
......
45 45
      end
46 46

  
47 47
      directory = File.dirname(target)
48
      basedir = File.dirname(Attachment.markdownized_previews_storage_path)
48 49
      FileUtils.mkdir_p(directory)
49
      args = [COMMAND, source, "-t", "gfm"]
50
      args = [COMMAND, source, "-t", "gfm", "--extract-media=markdownized_previews/#{attachment_id}/"]
50 51
      pid = nil
51 52
      output = Tempfile.new('markdownized-preview')
52 53

  
53 54
      begin
54 55
        Timeout.timeout(PREVIEW_GENERATION_TIMEOUT) do
55
          pid = Process.spawn(*args, out: output.path)
56
          pid = Process.spawn(*args, chdir: basedir, out: output.path)
56 57
          _, status = Process.wait2(pid)
57 58
          unless status.success?
58 59
            logger.error("Markdownized preview generation failed (#{status.exitstatus}):\nCommand: #{args.shelljoin}")
test/functional/attachments_controller_test.rb
314 314
    assert_equal 'text/html', @response.media_type
315 315
    assert_select 'div.filecontent.wiki', :text => /Redmine is a flexible project management web application/
316 316
    assert_select '.nodata', :count => 0
317
    assert_select 'div.filecontent.wiki img', 1
318
    img = css_select('div.filecontent.wiki img').first
319
    assert_match(%r{markdownized_previews/#{a.id}/}, img['src'])
320
    path = img['src'][%r{markdownized_previews/#{a.id}/(.+)$}, 1]
321

  
322
    get(:preview_media, :params => {:id => a.id, :path => path})
323

  
324
    assert_response :success
325
    assert_equal 'image/png', response.media_type
317 326
  end
318 327

  
319 328
  def test_show_libreoffice_writer
......
336 345
    assert_equal 'text/html', @response.media_type
337 346
    assert_select 'div.filecontent.wiki', :text => /Redmine is a flexible project management web application/
338 347
    assert_select '.nodata', :count => 0
348
    assert_select 'div.filecontent.wiki img', 1
349
    img = css_select('div.filecontent.wiki img').first
350
    assert_match(%r{markdownized_previews/#{a.id}/}, img['src'])
351
    path = img['src'][%r{markdownized_previews/#{a.id}/(.+)$}, 1]
352

  
353
    get(:preview_media, :params => {:id => a.id, :path => path})
354

  
355
    assert_response :success
356
    assert_equal 'image/png', response.media_type
357
  end
358

  
359
  def test_preview_media_from_private_issue_without_permission
360
    attachment = Attachment.find(15)
361
    FileUtils.mkdir_p(File.join(attachment.markdownized_preview_directory, 'Pictures'))
362
    File.binwrite(File.join(attachment.markdownized_preview_directory, 'Pictures', 'image.png'), 'PNG')
363

  
364
    get(:preview_media, :params => {:id => attachment.id, :path => 'Pictures/image.png'})
365

  
366
    assert_response :unauthorized
367
  end
368

  
369
  def test_preview_media_from_private_issue_with_permission
370
    @request.session[:user_id] = 2
371

  
372
    attachment = Attachment.find(15)
373
    FileUtils.mkdir_p(File.join(attachment.markdownized_preview_directory, 'Pictures'))
374
    File.binwrite(File.join(attachment.markdownized_preview_directory, 'Pictures', 'image.png'), 'PNG')
375

  
376
    get(:preview_media, :params => {:id => attachment.id, :path => 'Pictures/image.png'})
377

  
378
    assert_response :success
379
    assert_equal 'image/png', response.media_type
380
  end
381

  
382
  def test_preview_media_should_be_denied_without_permission
383
    @request.session[:user_id] = 3
384

  
385
    attachment = Attachment.find(15)
386
    FileUtils.mkdir_p(File.join(attachment.markdownized_preview_directory, 'Pictures'))
387
    File.binwrite(File.join(attachment.markdownized_preview_directory, 'Pictures', 'image.png'), 'PNG')
388

  
389
    get(:preview_media, :params => {:id => attachment.id, :path => 'Pictures/image.png'})
390

  
391
    assert_response :forbidden
392
  end
393

  
394
  def test_preview_media_should_return_404_for_missing_file
395
    @request.session[:user_id] = 2
396

  
397
    get(:preview_media, :params => {:id => 16, :path => 'media/missing.png'})
398

  
399
    assert_response :not_found
400
  end
401

  
402
  def test_preview_media_should_return_404_for_media_of_another_attachment
403
    @request.session[:user_id] = 2
404
    # The directory of attachment 160 starts with the directory of attachment 16
405
    media = File.join(Attachment.markdownized_previews_storage_path, '160', 'media', 'image1.png')
406
    FileUtils.mkdir_p(File.dirname(media))
407
    File.binwrite(media, 'image')
408

  
409
    get(:preview_media, :params => {:id => 16, :path => '../160/media/image1.png'})
410

  
411
    assert_response :not_found
412
  ensure
413
    FileUtils.rm_rf(File.join(Attachment.markdownized_previews_storage_path, '160'))
414
  end
415

  
416
  def test_preview_media_should_return_404_for_non_image_file
417
    @request.session[:user_id] = 2
418
    attachment = Attachment.find(16)
419
    FileUtils.mkdir_p(attachment.markdownized_preview_directory)
420
    File.write(File.join(attachment.markdownized_preview_directory, 'preview.md'), 'preview')
421

  
422
    get(:preview_media, :params => {:id => attachment.id, :path => 'preview.md'})
423

  
424
    assert_response :not_found
425
  ensure
426
    FileUtils.rm_rf(attachment.markdownized_preview_directory)
427
  end
428

  
429
  def test_preview_media_should_return_404_for_invalid_path
430
    @request.session[:user_id] = 2
431

  
432
    get(:preview_media, :params => {:id => 16, :path => '../secret.txt'})
433

  
434
    assert_response :not_found
339 435
  end
340 436

  
341 437
  def test_show_rtf
test/integration/routing/attachments_test.rb
31 31
    should_route 'GET /attachments/thumbnail/1' => 'attachments#thumbnail', :id => '1'
32 32
    should_route 'GET /attachments/thumbnail/1/200' => 'attachments#thumbnail', :id => '1', :size => '200'
33 33

  
34
    should_route 'GET /attachments/markdownized_previews/1/pictures/image.png' => 'attachments#preview_media', :id => '1', :path => 'pictures/image.png'
35

  
34 36
    should_route 'DELETE /attachments/1' => 'attachments#destroy', :id => '1'
35 37

  
36 38
    should_route 'GET /attachments/issues/1/edit' => 'attachments#edit_all', :object_type => 'issues', :object_id => '1'
test/unit/attachment_test.rb
675 675
  end
676 676

  
677 677
  def test_delete_from_disk_should_delete_markdownized_preview_cache
678
    skip unless Redmine::Markdownizer.available?
679

  
678 680
    attachment = Attachment.create!(
679 681
      :container => Issue.find(1),
680 682
      :file => uploaded_test_file(
......
683 685
      ),
684 686
      :author => User.find(1)
685 687
    )
686
    preview = attachment.markdownized_preview_cache_path
687
    FileUtils.mkdir_p(File.dirname(preview))
688
    File.write(preview, "preview")
689
    assert File.exist?(preview)
688
    preview_dir = attachment.markdownized_preview_directory
689
    attachment.markdownized_preview_content
690
    assert Dir.exist?(preview_dir)
690 691

  
691 692
    attachment.send(:delete_from_disk!)
692
    assert_not File.exist?(preview)
693
    assert_not Dir.exist?(preview_dir)
693 694
  end
694 695

  
695 696
  if convert_installed?
(6-6/8)