From 51df6d458dace7bea68e4a7dfe63ed732ec6779a Mon Sep 17 00:00:00 2001
From: Marcel Herrguth <mh@zammad.com>
Date: Tue, 8 Dec 2020 15:12:36 +0100
Subject: [PATCH] Fixes #2651 - "Getting Started" setup wizard removes custom
 port from FQDN.

---
 app/controllers/getting_started_controller.rb | 37 +++++++++------
 .../getting_started_controller_spec.rb        | 45 +++++++++++++++++++
 spec/system/setup/system_spec.rb              |  2 +-
 test/browser/aaa_getting_started_test.rb      |  2 +-
 4 files changed, 71 insertions(+), 15 deletions(-)
 create mode 100644 spec/controllers/getting_started_controller_spec.rb

diff --git a/app/controllers/getting_started_controller.rb b/app/controllers/getting_started_controller.rb
index 3b59518e8..4671203a6 100644
--- a/app/controllers/getting_started_controller.rb
+++ b/app/controllers/getting_started_controller.rb
@@ -108,18 +108,11 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password}
     messages = {}
     settings = {}
     if !Setting.get('system_online_service')
-      if !params[:url] || params[:url] !~ %r{^(http|https)://.+?$}
-        messages[:url] = 'A URL looks like http://zammad.example.com'
-      end
-
-      # split url in http_type and fqdn
-      if params[:url]
-        if params[:url] =~ %r{^(http|https)://(.+?)(:.+?|/.+?|)$}
-          settings[:http_type] = $1
-          settings[:fqdn] = $2
-        else
-          messages[:url] = 'A URL looks like http://zammad.example.com'
-        end
+      if (result = self.class.validate_uri(params[:url]))
+        settings[:http_type] = result[:scheme]
+        settings[:fqdn]      = result[:fqdn]
+      else
+        messages[:url] = 'An URL looks like this: http://zammad.example.com'
       end
     end
 
@@ -186,6 +179,25 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password}
     }
   end
 
+  def self.validate_uri(string)
+    uri = URI(string)
+
+    return false if %w[http https].exclude?(uri.scheme) || uri.host.blank?
+
+    defaults = [['http', 80], ['https', 443]]
+    actual   = [uri.scheme, uri.port]
+
+    fqdn = if defaults.include? actual
+             uri.host
+           else
+             "#{uri.host}:#{uri.port}"
+           end
+
+    { scheme: uri.scheme, fqdn: fqdn }
+  rescue
+    false
+  end
+
   private
 
   def auto_wizard_enabled_response
@@ -233,5 +245,4 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password}
       product_logo: Setting.get('product_logo')
     }
   end
-
 end
diff --git a/spec/controllers/getting_started_controller_spec.rb b/spec/controllers/getting_started_controller_spec.rb
new file mode 100644
index 000000000..ccc46e6b2
--- /dev/null
+++ b/spec/controllers/getting_started_controller_spec.rb
@@ -0,0 +1,45 @@
+require 'rails_helper'
+
+RSpec.describe GettingStartedController do
+  describe '.validate_uri' do
+    it 'false for nil' do
+      expect(described_class.validate_uri(nil)).to be_falsey
+    end
+
+    it 'false for empty' do
+      expect(described_class.validate_uri('')).to be_falsey
+    end
+
+    it 'false for non-http(s)' do
+      expect(described_class.validate_uri('a://example.org')).to be_falsey
+    end
+
+    it 'false for gibberish uri' do
+      expect(described_class.validate_uri('http:///a')).to be_falsey
+    end
+
+    it 'http and fqdn for http' do
+      expect(described_class.validate_uri('http://example.org')).to eq({ scheme: 'http', fqdn: 'example.org' })
+    end
+
+    it 'https and fqdn for https' do
+      expect(described_class.validate_uri('https://example.org')).to eq({ scheme: 'https', fqdn: 'example.org' })
+    end
+
+    it 'http and fqdn for http on default port' do
+      expect(described_class.validate_uri('http://example.org:80')).to eq({ scheme: 'http', fqdn: 'example.org' })
+    end
+
+    it 'https and fqdn for https on default port' do
+      expect(described_class.validate_uri('https://example.org:443')).to eq({ scheme: 'https', fqdn: 'example.org' })
+    end
+
+    it 'http and fqdn with port for http on custom port' do
+      expect(described_class.validate_uri('http://example.org:443')).to eq({ scheme: 'http', fqdn: 'example.org:443' })
+    end
+
+    it 'https and fqdn with port for https on custom port' do
+      expect(described_class.validate_uri('https://example.org:80')).to eq({ scheme: 'https', fqdn: 'example.org:80' })
+    end
+  end
+end
diff --git a/spec/system/setup/system_spec.rb b/spec/system/setup/system_spec.rb
index b0568219f..58efc554e 100644
--- a/spec/system/setup/system_spec.rb
+++ b/spec/system/setup/system_spec.rb
@@ -45,7 +45,7 @@ RSpec.describe 'System setup process', type: :system, set_up: false, authenticat
       # fill in wrong URL
       fill_in 'url', with: 'some host'
       click_on('Next')
-      expect(page).to have_css('.alert', text: 'A URL looks like')
+      expect(page).to have_css('.alert', text: 'An URL looks like')
 
       # fill in valild/current URL
       fill_in 'url', with: app_host
diff --git a/test/browser/aaa_getting_started_test.rb b/test/browser/aaa_getting_started_test.rb
index 4dfff0cd9..72cf601ba 100644
--- a/test/browser/aaa_getting_started_test.rb
+++ b/test/browser/aaa_getting_started_test.rb
@@ -65,7 +65,7 @@ class AaaGettingStartedTest < TestCase
     )
     watch_for(
       css:   'body',
-      value: 'A URL looks like',
+      value: 'An URL looks like this',
     )
     set(
       css:   '.js-base input[name="url"]',