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>
This commit is contained in:
Martin Brennan 2020-09-30 15:20:00 +10:00 committed by GitHub
parent a0bbc346cb
commit 39b2fb8649
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 18 additions and 0 deletions

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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