DEV: Replace SECURE_MEDIA_ROUTE const with other methods (#10545)

This is so if the route changes this const won't be around to bite us, use the Rails route methods instead.

app/models/post.rb

@@ -304,7 +304,11 @@ class Post < ActiveRecord::Base
       each_upload_url do |url|
         uri = URI.parse(url)
         if FileHelper.is_supported_media?(File.basename(uri.path))
-          raw = raw.sub(Discourse.store.s3_upload_host, "#{Discourse.base_url}/#{Upload::SECURE_MEDIA_ROUTE}")
+          raw = raw.sub(
+            url, Rails.application.routes.url_for(
+              controller: "uploads", action: "show_secure", path: uri.path[1..-1], host: Discourse.current_hostname
+            )
+          )
         end
       end
     end

app/models/upload.rb

@@ -9,7 +9,6 @@ class Upload < ActiveRecord::Base
   SHA1_LENGTH = 40
   SEEDED_ID_THRESHOLD = 0
   URL_REGEX ||= /(\/original\/\dX[\/\.\w]*\/(\h+)[\.\w]*)/
-  SECURE_MEDIA_ROUTE = "secure-media-uploads"
 
   belongs_to :user
   belongs_to :access_control_post, class_name: 'Post'
@@ -155,16 +154,28 @@ class Upload < ActiveRecord::Base
   def self.secure_media_url?(url)
     # we do not want to exclude topic links that for whatever reason
     # have secure-media-uploads in the URL e.g. /t/secure-media-uploads-are-cool/223452
-    url.include?(SECURE_MEDIA_ROUTE) && !url.include?("/t/") && FileHelper.is_supported_media?(url)
+    route = UrlHelper.rails_route_from_url(url)
+    route[:action] == "show_secure" && route[:controller] == "uploads" && FileHelper.is_supported_media?(url)
+  rescue ActionController::RoutingError
+    false
   end
 
   def self.signed_url_from_secure_media_url(url)
-    secure_upload_s3_path = url.sub(Discourse.base_url, "").sub("/#{SECURE_MEDIA_ROUTE}/", "")
+    route = UrlHelper.rails_route_from_url(url)
+    url = Rails.application.routes.url_for(route.merge(only_path: true))
+    secure_upload_s3_path = url[url.index(route[:path])..-1]
     Discourse.store.signed_url_for_path(secure_upload_s3_path)
   end
 
   def self.secure_media_url_from_upload_url(url)
-    url.sub(SiteSetting.Upload.absolute_base_url, "/#{SECURE_MEDIA_ROUTE}")
+    return url if !url.include?(SiteSetting.Upload.absolute_base_url)
+    uri = URI.parse(url)
+    Rails.application.routes.url_for(
+      controller: "uploads",
+      action: "show_secure",
+      path: uri.path[1..-1],
+      only_path: true
+    )
   end
 
   def self.short_path(sha1:, extension:)

lib/url_helper.rb

@@ -67,6 +67,11 @@ class UrlHelper
     UrlHelper.encode_component(CGI.unescapeHTML(UrlHelper.unencode(uri)))
   end
 
+  def self.rails_route_from_url(url)
+    path = URI.parse(url).path
+    Rails.application.routes.recognize_path(path)
+  end
+
   def self.s3_presigned_url?(url)
     url[/x-amz-(algorithm|credential)/i].present?
   end

spec/models/upload_spec.rb

@@ -432,6 +432,72 @@ describe Upload do
       expect(user.user_profile.card_background_upload_id).to eq(nil)
       expect(user.user_profile.profile_background_upload_id).to eq(nil)
     end
+  end
 
+  context ".signed_url_from_secure_media_url" do
+    before do
+      # must be done so signed_url_for_path exists
+      enable_secure_media
+    end
+
+    it "correctly gives back a signed url from a path only" do
+      secure_url = "/secure-media-uploads/original/1X/c5a2c4ba0fa390f5aac5c2c1a12416791ebdd9e9.png"
+      signed_url = Upload.signed_url_from_secure_media_url(secure_url)
+      expect(signed_url).not_to include("secure-media-uploads")
+      expect(UrlHelper.s3_presigned_url?(signed_url)).to eq(true)
+    end
+
+    it "correctly gives back a signed url from a full url" do
+      secure_url = "http://localhost:3000/secure-media-uploads/original/1X/c5a2c4ba0fa390f5aac5c2c1a12416791ebdd9e9.png"
+      signed_url = Upload.signed_url_from_secure_media_url(secure_url)
+      expect(signed_url).not_to include(Discourse.base_url)
+      expect(UrlHelper.s3_presigned_url?(signed_url)).to eq(true)
+    end
+  end
+
+  context ".secure_media_url_from_upload_url" do
+    before do
+      # must be done so signed_url_for_path exists
+      enable_secure_media
+    end
+
+    it "gets the secure media url from an S3 upload url" do
+      upload = Fabricate(:upload_s3, secure: true)
+      url = upload.url
+      secure_url = Upload.secure_media_url_from_upload_url(url)
+      expect(secure_url).not_to include(SiteSetting.Upload.absolute_base_url)
+    end
+  end
+
+  context ".secure_media_url?" do
+    it "works for a secure media url with or without schema + host" do
+      url = "//localhost:3000/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.png"
+      expect(Upload.secure_media_url?(url)).to eq(true)
+      url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.png"
+      expect(Upload.secure_media_url?(url)).to eq(true)
+      url = "http://localhost:3000/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.png"
+      expect(Upload.secure_media_url?(url)).to eq(true)
+    end
+
+    it "does not get false positives on a topic url" do
+      url = "/t/secure-media-uploads-are-cool/42839"
+      expect(Upload.secure_media_url?(url)).to eq(false)
+    end
+
+    it "returns true only for secure media URL for actual media (images/video/audio)" do
+      url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.mp4"
+      expect(Upload.secure_media_url?(url)).to eq(true)
+      url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.png"
+      expect(Upload.secure_media_url?(url)).to eq(true)
+      url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.mp3"
+      expect(Upload.secure_media_url?(url)).to eq(true)
+      url = "/secure-media-uploads/original/2X/f/f62055931bb702c7fd8f552fb901f977e0289a18.pdf"
+      expect(Upload.secure_media_url?(url)).to eq(false)
+    end
+
+    it "does not work for regular upload urls" do
+      url = "/uploads/default/test_0/original/1X/e1864389d8252958586c76d747b069e9f68827e3.png"
+      expect(Upload.secure_media_url?(url)).to eq(false)
+    end
   end
 end