FIX: skip external urls which has upload url in query string.

FIX: skip external urls which has upload url in query string.

Add spec tests for post.each_upload_url method. e8fafbc123170dd1f7d2a8adea4e7810585d3e76

diff --git a/app/models/post.rb b/app/models/post.rb
index 09691d1..fc2aaf3 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -897,7 +897,7 @@ class Post < ActiveRecord::Base
     links = fragments.css("a/@href", "img/@src").map { |media| media.value }.uniq
 
     links.each do |src|
-      next if src.blank? || upload_patterns.none? { |pattern| src =~ pattern }
+      next if src.blank? || upload_patterns.none? { |pattern| src.split("?")[0] =~ pattern }
 
       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)
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index 07cfc80..223400f 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -1340,4 +1340,31 @@ describe Post do
     end
   end
 
+  describe '#each_upload_url' do
+    let(:upload) { Fabricate(:upload_s3) }
+
+    it "correctly identifies all upload urls" do
+      urls = []
+      upload1 = Fabricate(:upload)
+      upload2 = Fabricate(:upload)
+      post = Fabricate(:post, raw: "A post with image and link upload.\n\n![](#{upload1.short_url})\n\n<a href='#{upload2.url}'>Link to upload</a>")
+      post.each_upload_url { |src, _, _| urls << src }
+      expect(urls).to eq([upload1.url, upload2.url])
+    end
+
+    it "should skip external urls with upload url in query string" do
+      SiteSetting.enable_s3_uploads = true
+      SiteSetting.s3_upload_bucket = "s3-upload-bucket"
+      SiteSetting.s3_access_key_id = "some key"
+      SiteSetting.s3_secret_access_key = "some secret key"
+      SiteSetting.s3_cdn_url = "https://cdn.s3.amazonaws.com"
+
+      urls = []
+      upload = Fabricate(:upload_s3)
+      post = Fabricate(:post, raw: "<a href='https://link.example.com/redirect?url=#{Discourse.store.cdn_url(upload.url)}'>Link to upload</a>")
+      post.each_upload_url { |src, _, _| urls << src }
+      expect(urls).to be_empty
+    end
+  end
+
 end

GitHub sha: 788f995f

1 Like

SPEC: correctly skips invalid upload urls