From 788f995f30d09d6d99de6f213120ee7957248dd5 Mon Sep 17 00:00:00 2001
From: Vinoth Kannan <vinothkannan@vinkas.com>
Date: Tue, 11 Jun 2019 19:55:02 +0530
Subject: [PATCH] FIX: skip external urls which has upload url in query string.

Add spec tests for post.each_upload_url method. e8fafbc123170dd1f7d2a8adea4e7810585d3e76
---
 app/models/post.rb       |  2 +-
 spec/models/post_spec.rb | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/app/models/post.rb b/app/models/post.rb
index 09691d10b86..fc2aaf35991 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 07cfc8027c6..223400f41d5 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