diff --git a/app/models/application.rb b/app/models/application.rb index e0fe4ab..84c44ed 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -278,18 +278,52 @@ class Application < ApplicationRecord end def sanitize_svg_icon - return unless icon.content_type == "image/svg+xml" + # Runs in before_validation. The blob has NOT yet been uploaded to disk at + # this point (Active Storage uploads in before_save), so we cannot call + # icon.download — we must read from the pending attachable. + # + # icon.attach below re-sets attachment_changes and would re-fire this + # callback; we skip if the pending attachable is the cleaned hash we just + # installed (tracked by object identity). + change = attachment_changes["icon"] + return unless change + attachable = change.attachable + return if attachable.equal?(@svg_sanitized_attachable) + + raw_svg, filename, content_type = read_pending_icon(attachable) + return unless raw_svg + return unless content_type == "image/svg+xml" || filename.to_s.downcase.end_with?(".svg") - raw_svg = icon.download doc = Loofah.xml_document(raw_svg) doc.scrub!(SvgScrubber.new) clean_svg = doc.to_xml - icon.attach( + sanitized = { io: StringIO.new(clean_svg), - filename: icon.filename.to_s, + filename: filename, content_type: "image/svg+xml" - ) + } + @svg_sanitized_attachable = sanitized + icon.attach(sanitized) + end + + def read_pending_icon(attachable) + case attachable + when ActionDispatch::Http::UploadedFile, Rack::Test::UploadedFile + content = attachable.read + attachable.rewind + [content, attachable.original_filename, attachable.content_type] + when Hash + io = attachable[:io] || attachable["io"] + return [nil, nil, nil] unless io + content = io.read + io.rewind if io.respond_to?(:rewind) + [content, + attachable[:filename] || attachable["filename"], + attachable[:content_type] || attachable["content_type"]] + else + [nil, nil, nil] + end end def icon_validation diff --git a/config/initializers/version.rb b/config/initializers/version.rb index a5c2565..da9ee54 100644 --- a/config/initializers/version.rb +++ b/config/initializers/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Clinch - VERSION = "0.13.0" + VERSION = "0.13.1" end diff --git a/test/models/application_test.rb b/test/models/application_test.rb index 4103045..063b623 100644 --- a/test/models/application_test.rb +++ b/test/models/application_test.rb @@ -1,7 +1,32 @@ require "test_helper" class ApplicationTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end + test "sanitizes an SVG icon uploaded via UploadedFile (regression for FileNotFoundError)" do + app = applications(:kavita_app) + + svg = %() + tempfile = Tempfile.new(["icon", ".svg"]).tap do |t| + t.write(svg) + t.rewind + end + uploaded = ActionDispatch::Http::UploadedFile.new( + tempfile: tempfile, + filename: "icon.svg", + type: "image/svg+xml" + ) + + # Previously raised ActiveStorage::FileNotFoundError because the + # before_validation callback called icon.download before the blob was + # uploaded to disk. + assert_nothing_raised do + app.update!(icon: uploaded) + end + + cleaned = app.icon.download + refute_match(/