From 39b2fb8649bb7c1956bc915d824878ad133d648f Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Wed, 30 Sep 2020 15:20:00 +1000
Subject: [PATCH] FIX: Invalid URLs could raise exceptions when calling
UrlHelper.rails_route_from_url (#10782)
Upload.secure_media_url? raised an exceptions when the URL was invalid,
which was a issue in some situations where secure media URLs must be
removed.
For example, sending digests used PrettyText.strip_secure_media,
which used Upload.secure_media_url? to replace secure media with
placeholders. If the URL was invalid, then an exception would be raised
and left unhandled.
Now instead in UrlHelper.rails_route_from_url we return nil if there is something wrong with the URL.
Co-authored-by: Bianca Nenciu <nenciu.bianca@gmail.com>
---
app/models/upload.rb | 1 +
lib/url_helper.rb | 2 ++
spec/components/url_helper_spec.rb | 10 ++++++++++
spec/models/upload_spec.rb | 5 +++++
4 files changed, 18 insertions(+)
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 1656fdb5ec9..36fc6da0a27 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -167,6 +167,7 @@ class Upload < ActiveRecord::Base
# 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
route = UrlHelper.rails_route_from_url(url)
+ return false if route.blank?
route[:action] == "show_secure" && route[:controller] == "uploads" && FileHelper.is_supported_media?(url)
rescue ActionController::RoutingError
false
diff --git a/lib/url_helper.rb b/lib/url_helper.rb
index 1915b78951d..7968c9ab39a 100644
--- a/lib/url_helper.rb
+++ b/lib/url_helper.rb
@@ -70,6 +70,8 @@ class UrlHelper
def self.rails_route_from_url(url)
path = URI.parse(encode(url)).path
Rails.application.routes.recognize_path(path)
+ rescue Addressable::URI::InvalidURIError
+ nil
end
def self.s3_presigned_url?(url)
diff --git a/spec/components/url_helper_spec.rb b/spec/components/url_helper_spec.rb
index 25c5dbdc99f..d56efc2b15b 100644
--- a/spec/components/url_helper_spec.rb
+++ b/spec/components/url_helper_spec.rb
@@ -196,4 +196,14 @@ describe UrlHelper do
end
end
+ describe "rails_route_from_url" do
+ it "returns a rails route from the path" do
+ expect(described_class.rails_route_from_url("/u")).to eq({ controller: "users", action: "index" })
+ end
+
+ it "does not raise for invalid URLs" do
+ url = "http://URL:%20https://google.com"
+ expect(described_class.rails_route_from_url(url)).to eq(nil)
+ end
+ end
end
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index 10ddea17389..d33ae4c5b1e 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -493,5 +493,10 @@ describe Upload do
url = "/uploads/default/test_0/original/1X/e1864389d8252958586c76d747b069e9f68827e3.png"
expect(Upload.secure_media_url?(url)).to eq(false)
end
+
+ it "does not raise for invalid URLs" do
+ url = "http://URL:%20https://google.com"
+ expect(Upload.secure_media_url?(url)).to eq(false)
+ end
end
end