Update logic to allow encoded multi-line URLs from HTTP(s)

Previously we explicitly considered the Git scheme, but this allowed FTP
and other protocols to have multi-line URLs. We now allowed encoded
newlines in HTTP(s) calls.

lib/gitlab/url_blocker.rb

@@ -165,14 +165,20 @@ module Gitlab
 
       def parse_url(url)
         Addressable::URI.parse(url).tap do |parsed_url|
-          raise Addressable::URI::InvalidURIError if git_multiline?(parsed_url)
+          raise Addressable::URI::InvalidURIError if multiline_blocked?(parsed_url)
         end
       rescue Addressable::URI::InvalidURIError, URI::InvalidURIError
         raise BlockedUrlError, 'URI is invalid'
       end
 
-      def git_multiline?(parsed_url)
-        parsed_url.scheme == 'git' && CGI.unescape(parsed_url.to_s) =~ /\n|\r/
+      def multiline_blocked?(parsed_url)
+        url = parsed_url.to_s
+
+        return true if url =~ /\n|\r/
+        # Google Cloud Storage uses a multi-line, encoded Signature query string
+        return false if %w(http https).include?(parsed_url.scheme&.downcase)
+
+        CGI.unescape(url) =~ /\n|\r/
       end
 
       def validate_port(port, ports)

spec/models/project_spec.rb

@@ -426,7 +426,7 @@ RSpec.describe Project, factory_default: :keep do
       include_context 'invalid urls'
       include_context 'valid urls with CRLF'
 
-      it 'does not allow URLs with CR or LF characters with git:// scheme' do
+      it 'does not allow URLs with unencoded CR or LF characters' do
         project = build(:project)
 
         aggregate_failures do

spec/support/shared_contexts/url_shared_context.rb

@@ -3,12 +3,6 @@
 RSpec.shared_context 'valid urls with CRLF' do
   let(:valid_urls_with_CRLF) do
     [
-      "http://example.com/pa\rth",
-      "http://example.com/pa\nth",
-      "http://example.com/pa\r\nth",
-      "http://example.com/path?param=foo\r\nbar",
-      "http://example.com/path?param=foo\rbar",
-      "http://example.com/path?param=foo\nbar",
       "http://example.com/pa%0dth",
       "http://example.com/pa%0ath",
       "http://example.com/pa%0d%0th",
@@ -16,7 +10,7 @@ RSpec.shared_context 'valid urls with CRLF' do
       "http://gitlab.com/path?param=foo%0Abar",
       "https://gitlab.com/path?param=foo%0Dbar",
       "http://example.org:1024/path?param=foo%0D%0Abar",
-      "https://storage.googleapis.com/bucket/import_export_upload/import_file/57265/express.tar.gz?GoogleAccessId=hello@example.org&Signature=VBJkDX2MV4gi5wcARkvEmSVrNxRgbumVYcH6xw615IbIyT9kDw/+NXmHEqg7\n1mSTRypOjr2IkqaCgHJeyIF4mdOsII/XdgYomdV6zRSqrVmAD0BXg6jfCCCk&Expires=1634663304"
+      "https://storage.googleapis.com/bucket/import_export_upload/import_file/57265/express.tar.gz?GoogleAccessId=hello@example.org&Signature=ABCD%0AEFGHik&Expires=1634663304"
     ]
   end
 end
@@ -24,18 +18,15 @@ end
 RSpec.shared_context 'invalid urls' do
   let(:urls_with_CRLF) do
     [
-      "git://example.com/pa\rth",
-      "git://example.com/pa\nth",
-      "git://example.com/pa\r\nth",
-      "git://example.com/path?param=foo\r\nbar",
-      "git://example.com/path?param=foo\rbar",
-      "git://example.com/path?param=foo\nbar",
       "git://example.com/pa%0dth",
       "git://example.com/pa%0ath",
-      "git://127.0a.0.1/pa%0d%0th",
-      "git://example.com/pa%0D%0Ath",
-      "git://gitlab.com/project%0Dpath",
-      "git://gitlab.com/path?param=foo%0Abar"
+      "git://example.com/pa%0d%0th",
+      "http://example.com/pa\rth",
+      "http://example.com/pa\nth",
+      "http://example.com/pa\r\nth",
+      "http://example.com/path?param=foo\r\nbar",
+      "http://example.com/path?param=foo\rbar",
+      "http://example.com/path?param=foo\nbar"
     ]
   end
 end

spec/validators/addressable_url_validator_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe AddressableUrlValidator do
       expect(badge.errors.added?(:link_url, validator.options.fetch(:message))).to be true
     end
 
-    it 'allows urls with CR or LF characters in query strings' do
+    it 'allows urls with encoded CR or LF characters' do
       aggregate_failures do
         valid_urls_with_CRLF.each do |url|
           validator.validate_each(badge, :link_url, url)
@@ -41,6 +41,7 @@ RSpec.describe AddressableUrlValidator do
     it 'does not allow urls with CR or LF characters' do
       aggregate_failures do
         urls_with_CRLF.each do |url|
+          badge = build(:badge, link_url: 'http://www.example.com')
           validator.validate_each(badge, :link_url, url)
 
           expect(badge.errors.added?(:link_url, 'is blocked: URI is invalid')).to be true