IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2026-09-05 04:40:41 -05:00
Code Issues Packages Projects Releases Wiki Activity

DEV: extract hotlinked media handling so non-post targets can reuse it (#42415)

Browse Source 
Hotlinked media downloading lived inside `Jobs::PullHotlinkedImages` and
`CookedProcessorMixin` reached for `@post.post_hotlinked_media`
directly, so no other target could reuse either.

This change extracts the download handling into
`HotlinkedMediaDownloader` and the shared primitives into
`HotlinkedMedia`, and has the mixin read its records from a
`hotlinked_media_map` hook, leaving post behavior unchanged.
This commit is contained in:
Renato Atilio
2026-08-13 15:09:51 -03:00
committed by GitHub
parent 28b3afbe4f
commit f3962e6cc5
5 changed files with 206 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/jobs/regular/pull_hotlinked_images.rb
+37 -109
View File
@@ -28,47 +28,41 @@ module Jobs
changed_hotlink_records = false
extract_images_from(post.cooked).each do |node|
download_src =
original_src = node["src"] || node[PrettyText::BLOCKED_HOTLINKED_SRC_ATTR] || node["href"]
download_src = replace_encoded_src(download_src)
download_src =
"#{SiteSetting.force_https ? "https" : "http"}:#{original_src}" if original_src.start_with?(
"//",
)
normalized_src = normalize_src(download_src)
HotlinkedMedia
.extract_candidates(post.cooked)
.each do |node|
download_src = HotlinkedMedia.download_src_for(node)
normalized_src = normalize_src(download_src)
next if !should_download_image?(download_src, post)
next if !should_download_image?(download_src, post)
hotlink_record = hotlinked_map[normalized_src]
hotlink_record = hotlinked_map[normalized_src]
if hotlink_record.nil?
hotlinked_map[normalized_src] = hotlink_record =
PostHotlinkedMedia.new(post: post, url: normalized_src)
begin
hotlink_record.upload = attempt_download(download_src, post.user_id)
hotlink_record.status = :downloaded
rescue ImageTooLargeError
hotlink_record.status = :too_large
rescue ImageBrokenError
hotlink_record.status = :download_failed
rescue UploadCreateError
hotlink_record.status = :upload_create_failed
if hotlink_record.nil?
hotlinked_map[normalized_src] = hotlink_record =
PostHotlinkedMedia.new(post: post, url: normalized_src)
status, upload =
HotlinkedMedia.download(
download_src,
post.user_id,
tmp_file_name: "discourse-hotlinked",
)
hotlink_record.upload = upload
hotlink_record.status = status
end
end
if hotlink_record.changed?
changed_hotlink_records = true
hotlink_record.save!
if hotlink_record.changed?
changed_hotlink_records = true
hotlink_record.save!
end
rescue => e
raise e if Rails.env.test?
log(
:error,
"Failed to pull hotlinked image (#{download_src}) post: #{@post_id}\n" + e.message +
"\n" + e.backtrace.join("\n"),
)
end
rescue => e
raise e if Rails.env.test?
log(
:error,
"Failed to pull hotlinked image (#{download_src}) post: #{@post_id}\n" + e.message +
"\n" + e.backtrace.join("\n"),
)
end
if changed_hotlink_records
post.trigger_post_process(
@@ -84,81 +78,19 @@ module Jobs
end
end
def download(src)
downloaded = nil
begin
retries ||= 3
if SiteSetting.verbose_upload_logging
Rails.logger.warn("Verbose Upload Logging: Downloading hotlinked image from #{src}")
end
downloaded =
FileHelper.download(
src,
max_file_size: SiteSetting.max_image_size_kb.kilobytes,
retain_on_max_file_size_exceeded: true,
tmp_file_name: "discourse-hotlinked",
follow_redirect: true,
read_timeout: 15,
)
rescue => e
if SiteSetting.verbose_upload_logging
Rails.logger.warn("Verbose Upload Logging: Error '#{e.message}' while downloading #{src}")
end
if (retries -= 1) > 0 && !Rails.env.test?
sleep 1
retry
end
end
downloaded
end
class ImageTooLargeError < StandardError
end
class ImageBrokenError < StandardError
end
class UploadCreateError < StandardError
end
# Error classes live on HotlinkedMediaDownloader now; these aliases keep the
# `rescue ImageTooLargeError` call sites in the
# PullUserProfileHotlinkedImages subclass working unchanged.
ImageTooLargeError = HotlinkedMediaDownloader::ImageTooLargeError
ImageBrokenError = HotlinkedMediaDownloader::ImageBrokenError
UploadCreateError = HotlinkedMediaDownloader::UploadCreateError
def attempt_download(src, user_id)
# secure-uploads endpoint prevents anonymous downloads, so we
# need the presigned S3 URL here
if Upload.secure_uploads_url?(src)
src = Upload.signed_url_from_secure_uploads_url(src, include_content_disposition: false)
end
hotlinked = download(src)
raise ImageBrokenError if !hotlinked
if File.size(hotlinked.path) > SiteSetting.max_image_size_kb.kilobytes
raise ImageTooLargeError
end
filename = File.basename(URI.parse(src).path)
filename << File.extname(hotlinked.path) unless filename["."]
upload = UploadCreator.new(hotlinked, filename, origin: src).create_for(user_id)
if upload.persisted?
upload
else
log(
:info,
"Failed to persist downloaded hotlinked image for post: #{@post_id}: #{src} - #{upload.errors.full_messages.join("\n")}",
)
raise UploadCreateError
end
HotlinkedMediaDownloader.download(src, user_id, tmp_file_name: "discourse-hotlinked")
end
def extract_images_from(html)
doc = Nokogiri::HTML5.fragment(html)
doc.css("img[src], [#{PrettyText::BLOCKED_HOTLINKED_SRC_ATTR}], a.lightbox[href]") -
doc.css("img.avatar") - doc.css(".lightbox img[src]")
HotlinkedMedia.extract_candidates(html)
end
def should_download_image?(src, post = nil)
@@ -228,10 +160,6 @@ module Jobs
guardian.can_see_post?(access_control_post)
end
def replace_encoded_src(src)
PostHotlinkedMedia.normalize_src(src, reset_scheme: false)
end
def normalize_src(src)
PostHotlinkedMedia.normalize_src(src)
end
lib/cooked_processor_mixin.rb
+9 -3
View File
@@ -489,10 +489,9 @@ module CookedProcessorMixin
def process_hotlinked_image(img)
onebox = img.ancestors(".onebox, .onebox-body").first
# Skip hotlinked media processing if @post is not available (e.g., for chat messages)
return true if @post.nil?
@hotlinked_map ||= hotlinked_media_map
return true if @hotlinked_map.nil?
@hotlinked_map ||= @post.post_hotlinked_media.preload(:upload).index_by(&:url)
normalized_src =
PostHotlinkedMedia.normalize_src(img["src"] || img[PrettyText::BLOCKED_HOTLINKED_SRC_ATTR])
info = @hotlinked_map[normalized_src]
@@ -524,6 +523,13 @@ module CookedProcessorMixin
still_an_image
end
# Tracked hotlinked media for the target being processed, keyed by normalized
# url. Targets that don't track any (or aren't processing a post) return nil to
# skip localization entirely.
def hotlinked_media_map
@post&.post_hotlinked_media&.preload(:upload)&.index_by(&:url)
end
def optimize_image!(img, upload, cropped: false)
w, h = img["width"].to_i, img["height"].to_i
onebox = img.ancestors(".onebox, .onebox-body").first
lib/hotlinked_media.rb
+42
View File
@@ -0,0 +1,42 @@
# frozen_string_literal: true
# Primitives shared by the post- and chat-side hotlinked media jobs: finding
# candidate media in cooked HTML, and downloading it into an Upload with the
# status that gets recorded against the target.
module HotlinkedMedia
# Nodes in +html+ whose media may be hotlinked. Avatars and lightbox thumbnails
# are skipped: the former are never hotlinked, the latter duplicate the link
# that wraps them.
def self.extract_candidates(html)
doc = html.is_a?(Nokogiri::XML::Node) ? html : Nokogiri::HTML5.fragment(html)
doc.css("img[src], [#{PrettyText::BLOCKED_HOTLINKED_SRC_ATTR}], a.lightbox[href]") -
doc.css("img.avatar") - doc.css(".lightbox img[src]")
end
# The src to download for +node+: normalized without dropping the scheme, then
# given one if it was protocol-relative.
def self.download_src_for(node)
original_src = node["src"] || node[PrettyText::BLOCKED_HOTLINKED_SRC_ATTR] || node["href"]
return original_src if original_src.blank?
src = PostHotlinkedMedia.normalize_src(original_src, reset_scheme: false)
src =
"#{SiteSetting.force_https ? "https" : "http"}:#{original_src}" if original_src.start_with?(
"//",
)
src
end
# Downloads +src+ for +user_id+, returning [status, upload]. The status is the
# one to record against the target; upload is nil unless it is :downloaded.
def self.download(src, user_id, tmp_file_name:)
[:downloaded, HotlinkedMediaDownloader.download(src, user_id, tmp_file_name:)]
rescue HotlinkedMediaDownloader::ImageTooLargeError
[:too_large, nil]
rescue HotlinkedMediaDownloader::ImageBrokenError
[:download_failed, nil]
rescue HotlinkedMediaDownloader::UploadCreateError
[:upload_create_failed, nil]
end
end
lib/hotlinked_media_downloader.rb
+83
View File
@@ -0,0 +1,83 @@
# frozen_string_literal: true
# Shared logic for downloading a hotlinked (external) image and turning it into
# a local Upload. Used by the post-side Jobs::PullHotlinkedImages and the
# chat-side Jobs::Chat::PullHotlinkedImages so the download/retry/secure-upload
# handling lives in one place.
class HotlinkedMediaDownloader
class ImageTooLargeError < StandardError
end
class ImageBrokenError < StandardError
end
class UploadCreateError < StandardError
end
# Downloads +src+ and creates an Upload owned by +user_id+.
# Returns the persisted Upload or raises one of the typed errors above.
def self.download(src, user_id, tmp_file_name:)
new(tmp_file_name).download(src, user_id)
end
def initialize(tmp_file_name)
@tmp_file_name = tmp_file_name
end
def download(src, user_id)
# secure-uploads endpoint prevents anonymous downloads, so we
# need the presigned S3 URL here
if Upload.secure_uploads_url?(src)
src = Upload.signed_url_from_secure_uploads_url(src, include_content_disposition: false)
end
file = download_file(src)
raise ImageBrokenError if !file
raise ImageTooLargeError if File.size(file.path) > SiteSetting.max_image_size_kb.kilobytes
filename = File.basename(URI.parse(src).path)
filename << File.extname(file.path) if !filename["."]
upload = UploadCreator.new(file, filename, origin: src).create_for(user_id)
if !upload.persisted?
Rails.logger.info(
"#{RailsMultisite::ConnectionManagement.current_db}: Failed to persist downloaded hotlinked image: #{src} - #{upload.errors.full_messages.join("\n")}",
)
raise UploadCreateError
end
upload
end
private
def download_file(src)
downloaded = nil
retries = 3
begin
if SiteSetting.verbose_upload_logging
Rails.logger.warn("Verbose Upload Logging: Downloading hotlinked image from #{src}")
end
downloaded =
FileHelper.download(
src,
max_file_size: SiteSetting.max_image_size_kb.kilobytes,
retain_on_max_file_size_exceeded: true,
tmp_file_name: @tmp_file_name,
follow_redirect: true,
read_timeout: 15,
)
rescue StandardError => e
if SiteSetting.verbose_upload_logging
Rails.logger.warn("Verbose Upload Logging: Error '#{e.message}' while downloading #{src}")
end
if (retries -= 1) > 0 && !Rails.env.test?
sleep 1
retry
end
end
downloaded
end
end
spec/lib/hotlinked_media_downloader_spec.rb
+35
View File
@@ -0,0 +1,35 @@
# frozen_string_literal: true
RSpec.describe HotlinkedMediaDownloader do
fab!(:user)
let(:image_url) { "http://example.com/image.png" }
let(:png) { Base64.decode64("R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7") }
before do
SiteSetting.max_image_size_kb = 2
stub_request(:get, image_url).to_return(body: png, headers: { "Content-Type" => "image/png" })
end
it "downloads and creates an upload" do
stub_image_size
upload = described_class.download(image_url, user.id, tmp_file_name: "test-hotlinked")
expect(upload).to be_persisted
expect(upload.user_id).to eq(user.id)
end
it "raises ImageBrokenError when the download fails" do
stub_request(:get, image_url).to_return(status: 404)
expect {
described_class.download(image_url, user.id, tmp_file_name: "test-hotlinked")
}.to raise_error(described_class::ImageBrokenError)
end
it "raises ImageTooLargeError when the file exceeds the limit" do
huge = "a" * (SiteSetting.max_image_size_kb * 1024 * 2)
stub_request(:get, image_url).to_return(body: huge, headers: { "Content-Type" => "image/png" })
expect {
described_class.download(image_url, user.id, tmp_file_name: "test-hotlinked")
}.to raise_error(described_class::ImageTooLargeError)
end
end