From fe7a7ecf6cc9a8e048c8250859be179502041feb Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 10 Aug 2020 17:59:29 +0100 Subject: [PATCH] FIX: Include secure media URLs when linking post uploads (#10404) Normally, secure media urls are linked like `/secure-media-uploads/...`. In this case, uploads were already being linked correctly. But sometimes (e.g. when pulling hotlinked onebox images) secure media is referenced with a full domain name (`//example.com/secure-media-uploads`). This commit ensures that those uploads are also linked correctly. --- app/models/post.rb | 2 +- spec/models/post_spec.rb | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/app/models/post.rb b/app/models/post.rb index 8bfe3cb8f13..e07a78f8410 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -988,7 +988,7 @@ class Post < ActiveRecord::Base next if Rails.configuration.multisite && src.exclude?(current_db) src = "#{SiteSetting.force_https ? "https" : "http"}:#{src}" if src.start_with?("//") - next unless Discourse.store.has_been_uploaded?(src) || (include_local_upload && src =~ /\A\/[^\/]/i) + next unless Discourse.store.has_been_uploaded?(src) || Upload.secure_media_url?(src) || (include_local_upload && src =~ /\A\/[^\/]/i) path = begin URI(UrlHelper.unencode(GlobalSetting.cdn_url ? src.sub(GlobalSetting.cdn_url, "") : src))&.path diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 2e3f44d183e..5acc196a7ff 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -1642,6 +1642,30 @@ describe Post do ) end + it "correctly identifies secure uploads" do + enable_secure_media_and_s3 + upload1 = Fabricate(:upload_s3, secure: true) + upload2 = Fabricate(:upload_s3, secure: true) + + # Test including domain: + upload1_url = UrlHelper.cook_url(upload1.url, secure: true) + # Test without domain: + upload2_path = URI.parse(UrlHelper.cook_url(upload2.url, secure: true)).path + + post = Fabricate(:post, raw: <<~RAW) + + + RAW + + sha1s = [] + + post.each_upload_url do |src, path, sha| + sha1s << sha + end + + expect(sha1s).to contain_exactly(upload1.sha1, upload2.sha1) + end + it "correctly identifies missing uploads with short url" do upload = Fabricate(:upload) url = upload.short_url