Refactor ActionDispatch::Http::URL.build_host_url

Add support for extracting the port from the :host option and for
removing the subdomain by using nil, false or ''.

1 files changed, 49 insertions, 20 deletions

diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index 5caff769bb..6f5a52c568 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -4,7 +4,9 @@ require 'active_support/core_ext/hash/slice'
 module ActionDispatch
   module Http
     module URL
-      IP_HOST_REGEXP = /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/
+      IP_HOST_REGEXP  = /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/
+      HOST_REGEXP     = /(^.*:\/\/)?([^:]+)(?::(\d+$))?/
+      PROTOCOL_REGEXP = /^([^:]+)(:)?(\/\/)?$/
 
       mattr_accessor :tld_length
       self.tld_length = 1
@@ -60,15 +62,20 @@ module ActionDispatch
           result = ""
 
           unless options[:only_path]
-            protocol = extract_protocol(options)
-            unless options[:protocol] == false
-              result << protocol
-              result << ":" unless result.match(%r{:|//})
+            if match = options[:host].match(HOST_REGEXP)
+              options[:protocol] ||= match[1] unless options[:protocol] == false
+              options[:host]       = match[2]
+              options[:port]       = match[3] unless options.key?(:port)
             end
-            result << "//" unless result.match("//")
+
+            options[:protocol] = normalize_protocol(options)
+            options[:host]     = normalize_host(options)
+            options[:port]     = normalize_port(options)
+
+            result << options[:protocol]
             result << rewrite_authentication(options)
-            result << host_or_subdomain_and_domain(options)
-            result << ":#{options.delete(:port)}" if options[:port]
+            result << options[:host]
+            result << ":#{options[:port]}" if options[:port]
           end
           result
         end
@@ -77,6 +84,10 @@ module ActionDispatch
           host && IP_HOST_REGEXP !~ host
         end
 
+        def same_host?(options)
+          (options[:subdomain] == true || !options.key?(:subdomain)) && options[:domain].nil?
+        end
+
         def rewrite_authentication(options)
           if options[:user] && options[:password]
             "#{Rack::Utils.escape(options[:user])}:#{Rack::Utils.escape(options[:password])}@"
@@ -85,29 +96,47 @@ module ActionDispatch
           end
         end
 
-        # Extracts protocol http:// or https:// from options[:host]
-        # needs to be called whether the :protocol is being used or not
-        def extract_protocol(options)
-          if options[:host] && match = options[:host].match(/(^.*:\/\/)(.*)/)
-            options[:protocol] ||= match[1]
-            options[:host]     =   match[2]
+        def normalize_protocol(options)
+          case options[:protocol]
+          when nil
+            "http://"
+          when false, "//"
+            "//"
+          when PROTOCOL_REGEXP
+            "#{$1}://"
+          else
+            raise ArgumentError, "Invalid :protocol option: #{options[:protocol].inspect}"
           end
-          options[:protocol] || "http"
         end
 
-        def host_or_subdomain_and_domain(options)
-          return options[:host] if !named_host?(options[:host]) || (options[:subdomain].nil? && options[:domain].nil?)
+        def normalize_host(options)
+          return options[:host] if !named_host?(options[:host]) || same_host?(options)
 
           tld_length = options[:tld_length] || @@tld_length
 
           host = ""
-          unless options[:subdomain] == false || options[:subdomain] == ''
-            host << (options[:subdomain] || extract_subdomain(options[:host], tld_length)).to_param
-            host << "."
+          if options[:subdomain] == true || !options.key?(:subdomain)
+            host << extract_subdomain(options[:host], tld_length).to_param
+          elsif options[:subdomain].present?
+            host << options[:subdomain].to_param
           end
+          host << "." unless host.empty?
           host << (options[:domain] || extract_domain(options[:host], tld_length))
           host
         end
+
+        def normalize_port(options)
+          return nil if options[:port].nil? || options[:port] == false
+
+          case options[:protocol]
+          when "//"
+            nil
+          when "https://"
+            options[:port].to_i == 443 ? nil : options[:port]
+          else
+            options[:port].to_i == 80 ? nil : options[:port]
+          end
+        end
       end
 
       def initialize(env)