summary refs log tree commit
diff options
context:
space:
mode:
authorJeremy Evans <code@jeremyevans.net>2022-05-06 15:55:44 -0700
committerJeremy Evans <code@jeremyevans.net>2022-05-07 10:27:29 -0700
commit1d71f8741bb8343711924505a2bbc82159667691 (patch)
tree4bcba6a635ed6b55ae2a2d37662ac8f7addfd7af
parentdb94cebf864e9e082c6ffba96a5315f29cfccd61 (diff)
downloadrack-1d71f8741bb8343711924505a2bbc82159667691.tar.gz
Add 100% line/branch coverage to rack/request.rb
Simplify #server_port now that it is required to be an integer.

Simplify #port.

Remove nested conditional in #ip.

Remove private #extract_proto_header, unused since
b87d1828bd90b24eb0fa4a99abf580d9ddde4a0e (added in
6f349e1d2d1f528c486417d3421609be6e033e31, so only available since
2.1).

Coverage after this commit:

3286 relevant lines, 3282 lines covered and 4 lines missed. ( 99.88% )
1114 total branches, 1067 branches covered and 47 branches missed. ( 95.78% )
-rw-r--r--lib/rack/request.rb46
-rw-r--r--test/spec_request.rb38
2 files changed, 45 insertions, 39 deletions
diff --git a/lib/rack/request.rb b/lib/rack/request.rb
index 99d9674b..45214014 100644
--- a/lib/rack/request.rb
+++ b/lib/rack/request.rb
@@ -287,9 +287,7 @@ module Rack
       end
 
       def server_port
-        if port = get_header(SERVER_PORT)
-          Integer(port)
-        end
+        get_header(SERVER_PORT)
       end
 
       def cookies
@@ -347,23 +345,9 @@ module Rack
       def port
         if authority = self.authority
           _, _, port = split_authority(authority)
-
-          if port
-            return port
-          end
         end
 
-        if forwarded_port = self.forwarded_port
-          return forwarded_port.last
-        end
-
-        if scheme = self.scheme
-          if port = DEFAULT_PORTS[scheme]
-            return port
-          end
-        end
-
-        self.server_port
+        port || forwarded_port&.last || DEFAULT_PORTS[scheme] || server_port
       end
 
       def forwarded_for
@@ -435,14 +419,12 @@ module Rack
           return external_addresses.last
         end
 
-        if forwarded_for = self.forwarded_for
-          unless forwarded_for.empty?
-            # The forwarded for addresses are ordered: client, proxy1, proxy2.
-            # So we reject all the trusted addresses (proxy*) and return the
-            # last client. Or if we trust everyone, we just return the first
-            # address.
-            return reject_trusted_ip_addresses(forwarded_for).last || forwarded_for.first
-          end
+        if (forwarded_for = self.forwarded_for) && !forwarded_for.empty?
+          # The forwarded for addresses are ordered: client, proxy1, proxy2.
+          # So we reject all the trusted addresses (proxy*) and return the
+          # last client. Or if we trust everyone, we just return the first
+          # address.
+          return reject_trusted_ip_addresses(forwarded_for).last || forwarded_for.first
         end
 
         # If all the addresses are trusted, and we aren't forwarded, just return
@@ -756,16 +738,6 @@ module Rack
         header if ALLOWED_SCHEMES.include?(header)
       end
 
-      def extract_proto_header(header)
-        if header
-          if (comma_index = header.index(','))
-            header[0, comma_index]
-          else
-            header
-          end
-        end
-      end
-
       def forwarded_priority
         Request.forwarded_priority
       end
@@ -780,4 +752,6 @@ module Rack
   end
 end
 
+# :nocov:
 require_relative 'multipart' unless defined?(Rack::Multipart)
+# :nocov:
diff --git a/test/spec_request.rb b/test/spec_request.rb
index 58e42372..58797462 100644
--- a/test/spec_request.rb
+++ b/test/spec_request.rb
@@ -47,6 +47,20 @@ class RackRequestTest < Minitest::Spec
     assert_equal "example.com:443", req.authority
   end
 
+  it 'can calculate the server authority' do
+    req = make_request('SERVER_NAME' => 'example.com')
+    assert_equal "example.com", req.server_authority
+    req = make_request('SERVER_NAME' => 'example.com', 'SERVER_PORT' => 8080)
+    assert_equal "example.com:8080", req.server_authority
+  end
+
+  it 'can calculate the port without an authority' do
+    req = make_request('SERVER_PORT' => 8080)
+    assert_equal 8080, req.port
+    req = make_request('HTTPS' => 'on')
+    assert_equal 443, req.port
+  end
+
   it 'yields to the block if no value has been set' do
     req = make_request(Rack::MockRequest.env_for("http://example.com:8080/"))
     yielded = false
@@ -376,7 +390,7 @@ class RackRequestTest < Minitest::Spec
       req("HTTP_X_FORWARDED_SCHEME" => "http").
         forwarded_scheme.must_equal "http"
 
-      Rack::Request.forwarded_priority = [:x_forwarded, :forwarded]
+      Rack::Request.forwarded_priority = [nil, :x_forwarded, :forwarded]
 
       req("HTTP_FORWARDED"=>"for=1.2.3.4",
         "HTTP_X_FORWARDED_FOR" => "2.3.4.5").
@@ -402,7 +416,7 @@ class RackRequestTest < Minitest::Spec
       req("HTTP_FORWARDED"=>"proto=https").
         forwarded_scheme.must_equal "https"
 
-      Rack::Request.x_forwarded_proto_priority = [:scheme, :proto]
+      Rack::Request.x_forwarded_proto_priority = [nil, :scheme, :proto]
 
       req("HTTP_FORWARDED"=>"proto=https",
         "HTTP_X_FORWARDED_PROTO" => "ws",
@@ -1781,7 +1795,17 @@ EOF
 
   it "sets the default session to an empty hash" do
     req = make_request(Rack::MockRequest.env_for("http://example.com:8080/"))
-    assert_equal Hash.new, req.session
+    session = req.session
+    assert_equal Hash.new, session
+    req.env['rack.session'].must_be_same_as session
+  end
+
+  it "sets the default session options to an empty hash" do
+    req = make_request(Rack::MockRequest.env_for("http://example.com:8080/"))
+    session_options = req.session_options
+    assert_equal Hash.new, session_options
+    req.env['rack.session.options'].must_be_same_as session_options
+    assert_equal Hash.new, req.session_options
   end
 
   class MyRequest < Rack::Request
@@ -1849,6 +1873,14 @@ EOF
     end
   end
 
+  it "Env sets @env on initialization" do
+    c = Class.new do
+      include Rack::Request::Env
+    end
+    h = {}
+    c.new(h).env.must_be_same_as h
+  end
+
   class NonDelegate < Rack::Request
     def delegate?; false; end
   end