Fix FileNotFoundError when uploading an SVG icon
Some checks failed
Some checks failed
The sanitize_svg_icon before_validation callback called icon.download, but Active Storage uploads pending blobs in before_save — so at before_validation time the file only existed in the request tempfile, not at the configured storage path. Read from the pending attachable (UploadedFile / IO hash) instead. Guards against the recursive callback that icon.attach would otherwise trigger by tracking the cleaned attachable by object identity. Bumps to 0.13.1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -278,18 +278,52 @@ class Application < ApplicationRecord
|
|||||||
end
|
end
|
||||||
|
|
||||||
def sanitize_svg_icon
|
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 = Loofah.xml_document(raw_svg)
|
||||||
doc.scrub!(SvgScrubber.new)
|
doc.scrub!(SvgScrubber.new)
|
||||||
clean_svg = doc.to_xml
|
clean_svg = doc.to_xml
|
||||||
|
|
||||||
icon.attach(
|
sanitized = {
|
||||||
io: StringIO.new(clean_svg),
|
io: StringIO.new(clean_svg),
|
||||||
filename: icon.filename.to_s,
|
filename: filename,
|
||||||
content_type: "image/svg+xml"
|
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
|
end
|
||||||
|
|
||||||
def icon_validation
|
def icon_validation
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
module Clinch
|
module Clinch
|
||||||
VERSION = "0.13.0"
|
VERSION = "0.13.1"
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -1,7 +1,32 @@
|
|||||||
require "test_helper"
|
require "test_helper"
|
||||||
|
|
||||||
class ApplicationTest < ActiveSupport::TestCase
|
class ApplicationTest < ActiveSupport::TestCase
|
||||||
# test "the truth" do
|
test "sanitizes an SVG icon uploaded via UploadedFile (regression for FileNotFoundError)" do
|
||||||
# assert true
|
app = applications(:kavita_app)
|
||||||
# end
|
|
||||||
|
svg = %(<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script><path d="M0 0"/></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(/<script/i, cleaned)
|
||||||
|
assert_match(/<path/, cleaned)
|
||||||
|
ensure
|
||||||
|
tempfile&.close
|
||||||
|
tempfile&.unlink
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user