Merge branch 'fix-gravator-default-url' into 'master'

Ensure Gravatar host looks like an actual host

Solves #10243.

I've chosen to simplify the method that extracts the host: since we
only need the host, let's get rid of the path and thus get rid of the
escaping problems!

Unit tests should ensure that most of the cases are covered.

See merge request !2482

CHANGELOG

@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.4.0 (unreleased)
+  - Ensure Gravatar host looks like an actual host
   - Add pagination headers to already paginated API resources
   - Properly generate diff of orphan commits, like the first commit in a repository
   - Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages

config/initializers/1_settings.rb

@@ -9,13 +9,8 @@ class Settings < Settingslogic
       gitlab.port.to_i == (gitlab.https ? 443 : 80)
     end
 
-    # get host without www, thanks to http://stackoverflow.com/a/6674363/1233435
-    def get_host_without_www(url)
-      url = CGI.escape(url)
-      uri = URI.parse(url)
-      uri = URI.parse("http://#{url}") if uri.scheme.nil?
-      host = uri.host.downcase
-      host.start_with?('www.') ? host[4..-1] : host
+    def host_without_www(url)
+      host(url).sub('www.', '')
     end
 
     def build_gitlab_ci_url
@@ -87,6 +82,17 @@ class Settings < Settingslogic
         custom_port
       ]
     end
+
+    # Extract the host part of the given +url+.
+    def host(url)
+      url = url.downcase
+      url = "http://#{url}" unless url.start_with?('http')
+
+      # Get rid of the path so that we don't even have to encode it
+      url_without_path = url.sub(%r{(https?://[^\/]+)/?.*}, '\1')
+
+      URI.parse(url_without_path).host
+    end
   end
 end
 
@@ -228,7 +234,7 @@ Settings['gravatar'] ||= Settingslogic.new({})
 Settings.gravatar['enabled']      = true if Settings.gravatar['enabled'].nil?
 Settings.gravatar['plain_url']  ||= 'http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon'
 Settings.gravatar['ssl_url']    ||= 'https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon'
-Settings.gravatar['host']         = Settings.get_host_without_www(Settings.gravatar['plain_url'])
+Settings.gravatar['host']         = Settings.host_without_www(Settings.gravatar['plain_url'])
 
 #
 # Cron Jobs

spec/initializers/settings_spec.rb

@@ -0,0 +1,44 @@
+require_relative '../../config/initializers/1_settings'
+
+describe Settings, lib: true do
+
+  describe '#host_without_www' do
+    context 'URL with protocol' do
+      it 'returns the host' do
+        expect(Settings.host_without_www('http://foo.com')).to eq 'foo.com'
+        expect(Settings.host_without_www('http://www.foo.com')).to eq 'foo.com'
+        expect(Settings.host_without_www('http://secure.foo.com')).to eq 'secure.foo.com'
+        expect(Settings.host_without_www('http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com'
+
+        expect(Settings.host_without_www('https://foo.com')).to eq 'foo.com'
+        expect(Settings.host_without_www('https://www.foo.com')).to eq 'foo.com'
+        expect(Settings.host_without_www('https://secure.foo.com')).to eq 'secure.foo.com'
+        expect(Settings.host_without_www('https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'secure.gravatar.com'
+      end
+    end
+
+    context 'URL without protocol' do
+      it 'returns the host' do
+        expect(Settings.host_without_www('foo.com')).to eq 'foo.com'
+        expect(Settings.host_without_www('www.foo.com')).to eq 'foo.com'
+        expect(Settings.host_without_www('secure.foo.com')).to eq 'secure.foo.com'
+        expect(Settings.host_without_www('www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com'
+      end
+
+      context 'URL with user/port' do
+        it 'returns the host' do
+          expect(Settings.host_without_www('bob:pass@foo.com:8080')).to eq 'foo.com'
+          expect(Settings.host_without_www('bob:pass@www.foo.com:8080')).to eq 'foo.com'
+          expect(Settings.host_without_www('bob:pass@secure.foo.com:8080')).to eq 'secure.foo.com'
+          expect(Settings.host_without_www('bob:pass@www.gravatar.com:8080/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com'
+
+          expect(Settings.host_without_www('http://bob:pass@foo.com:8080')).to eq 'foo.com'
+          expect(Settings.host_without_www('http://bob:pass@www.foo.com:8080')).to eq 'foo.com'
+          expect(Settings.host_without_www('http://bob:pass@secure.foo.com:8080')).to eq 'secure.foo.com'
+          expect(Settings.host_without_www('http://bob:pass@www.gravatar.com:8080/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com'
+        end
+      end
+    end
+  end
+
+end