Merge pull request #24982 from tomkadwill/improve_clarity_of_raw_host_with_port

Improve documentation and tests for raw_host_with_port and host_with_…

2 files changed, 30 insertions, 3 deletions

diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index 37f41ae988..b7a6aeee7d 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -217,7 +217,7 @@ module ActionDispatch
         @protocol ||= ssl? ? 'https://' : 'http://'
       end
 
-      # Returns the \host for this request, such as "example.com".
+      # Returns the \host and port for this request, such as "example.com:8080".
       #
       #   class Request < Rack::Request
       #     include ActionDispatch::Http::URL
@@ -226,6 +226,9 @@ module ActionDispatch
       #   req = Request.new 'HTTP_HOST' => 'example.com'
       #   req.raw_host_with_port # => "example.com"
       #
+      #   req = Request.new 'HTTP_HOST' => 'example.com:80'
+      #   req.raw_host_with_port # => "example.com:80"
+      #
       #   req = Request.new 'HTTP_HOST' => 'example.com:8080'
       #   req.raw_host_with_port # => "example.com:8080"
       def raw_host_with_port
@@ -236,7 +239,7 @@ module ActionDispatch
         end
       end
 
-      # Returns the host for this request, such as example.com.
+      # Returns the host for this request, such as "example.com".
       #
       #   class Request < Rack::Request
       #     include ActionDispatch::Http::URL
@@ -249,12 +252,16 @@ module ActionDispatch
       end
 
       # Returns a \host:\port string for this request, such as "example.com" or
-      # "example.com:8080".
+      # "example.com:8080". Port is only included if it is not a default port
+      # (80 or 443)
       #
       #   class Request < Rack::Request
       #     include ActionDispatch::Http::URL
       #   end
       #
+      #   req = Request.new 'HTTP_HOST' => 'example.com'
+      #   req.host_with_port # => "example.com"
+      #
       #   req = Request.new 'HTTP_HOST' => 'example.com:80'
       #   req.host_with_port # => "example.com"
       #
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index a4cb8ce449..781f852633 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -417,6 +417,11 @@ class RequestPath < BaseRequestTest
 end
 
 class RequestHost < BaseRequestTest
+  test "host without specifying port" do
+    request = stub_request 'HTTP_HOST' => 'rubyonrails.org'
+    assert_equal "rubyonrails.org", request.host_with_port
+  end
+
   test "host with default port" do
     request = stub_request 'HTTP_HOST' => 'rubyonrails.org:80'
     assert_equal "rubyonrails.org", request.host_with_port
@@ -427,6 +432,21 @@ class RequestHost < BaseRequestTest
     assert_equal "rubyonrails.org:81", request.host_with_port
   end
 
+  test "raw without specifying port" do
+    request = stub_request 'HTTP_HOST' => 'rubyonrails.org'
+    assert_equal "rubyonrails.org", request.raw_host_with_port
+  end
+
+  test "raw host with default port" do
+    request = stub_request 'HTTP_HOST' => 'rubyonrails.org:80'
+    assert_equal "rubyonrails.org:80", request.raw_host_with_port
+  end
+
+  test "raw host with non default port" do
+    request = stub_request 'HTTP_HOST' => 'rubyonrails.org:81'
+    assert_equal "rubyonrails.org:81", request.raw_host_with_port
+  end
+
   test "proxy request" do
     request = stub_request 'HTTP_HOST' => 'glu.ttono.us:80'
     assert_equal "glu.ttono.us", request.host_with_port