Ensure Gravatar host looks like an actual hostfix-gravator-default-url

3 files changed, 59 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index a15bbfbc49e..e3a1bc5d9f9 100644
--- a/CHANGELOG
+++ b/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
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index d625a909bf1..04a7c16ebde 100644
--- a/config/initializers/1_settings.rb
+++ b/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
diff --git a/spec/initializers/settings_spec.rb b/spec/initializers/settings_spec.rb
new file mode 100644
index 00000000000..e58f2c80e95
--- /dev/null
+++ b/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