From cc685b416aab9cff4b8d80b6dbdaf55cecca3c83 Mon Sep 17 00:00:00 2001 From: Francisco Mejia Date: Thu, 27 Feb 2025 17:34:22 -0500 Subject: [PATCH 1/2] Stop setting SERVER_PORT env key if nil --- lib/protocol/rack/adapter/generic.rb | 3 +++ test/protocol/rack/adapter/generic.rb | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/protocol/rack/adapter/generic.rb b/lib/protocol/rack/adapter/generic.rb index 075e8b1..c073d1c 100644 --- a/lib/protocol/rack/adapter/generic.rb +++ b/lib/protocol/rack/adapter/generic.rb @@ -106,6 +106,9 @@ def unwrap_request(request, env) if peer = request.peer env[CGI::REMOTE_ADDR] = peer.ip_address end + + # SERVER_PORT should not be set if it is nil + env.delete(CGI::SERVER_PORT) if env[CGI::SERVER_PORT].nil? end def make_environment(request) diff --git a/test/protocol/rack/adapter/generic.rb b/test/protocol/rack/adapter/generic.rb index f6f83bb..125780f 100644 --- a/test/protocol/rack/adapter/generic.rb +++ b/test/protocol/rack/adapter/generic.rb @@ -35,6 +35,18 @@ expect(env).to have_keys("CONTENT_TYPE" => be == "text/plain") end end + + with "app server without SERVER_PORT" do + let(:request) do + Protocol::HTTP::Request.new("https", "example.com", "GET", "/", "http/1.1", Protocol::HTTP::Headers[{"accept" => "text/html"}], nil) + end + + it "does not include SERVER_PORT in the Rack environment" do + env = adapter.make_environment(request) + + expect(env).not.to have_keys(Protocol::Rack::CGI::SERVER_PORT) + end + end with "a app that returns nil" do include DisableConsoleContext From ffa9d3ffefb8d27f322019d2a696aba8aaae3465 Mon Sep 17 00:00:00 2001 From: Francisco Mejia Date: Fri, 28 Feb 2025 14:01:40 -0500 Subject: [PATCH 2/2] Optimize implementation --- lib/protocol/rack/adapter/generic.rb | 3 --- lib/protocol/rack/adapter/rack2.rb | 6 +++++- lib/protocol/rack/adapter/rack3.rb | 6 +++++- lib/protocol/rack/adapter/rack31.rb | 6 +++++- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/protocol/rack/adapter/generic.rb b/lib/protocol/rack/adapter/generic.rb index c073d1c..075e8b1 100644 --- a/lib/protocol/rack/adapter/generic.rb +++ b/lib/protocol/rack/adapter/generic.rb @@ -106,9 +106,6 @@ def unwrap_request(request, env) if peer = request.peer env[CGI::REMOTE_ADDR] = peer.ip_address end - - # SERVER_PORT should not be set if it is nil - env.delete(CGI::SERVER_PORT) if env[CGI::SERVER_PORT].nil? end def make_environment(request) diff --git a/lib/protocol/rack/adapter/rack2.rb b/lib/protocol/rack/adapter/rack2.rb index ca98c63..01eee07 100644 --- a/lib/protocol/rack/adapter/rack2.rb +++ b/lib/protocol/rack/adapter/rack2.rb @@ -59,8 +59,12 @@ def make_environment(request) # I'm not sure what sane defaults should be here: CGI::SERVER_NAME => server_name, - CGI::SERVER_PORT => server_port, } + + # SERVER_PORT is optional but must not be set if it is not present. + if server_port + env[CGI::SERVER_PORT] = server_port + end self.unwrap_request(request, env) diff --git a/lib/protocol/rack/adapter/rack3.rb b/lib/protocol/rack/adapter/rack3.rb index eb00e5f..b43febb 100644 --- a/lib/protocol/rack/adapter/rack3.rb +++ b/lib/protocol/rack/adapter/rack3.rb @@ -55,8 +55,12 @@ def make_environment(request) # I'm not sure what sane defaults should be here: CGI::SERVER_NAME => server_name, - CGI::SERVER_PORT => server_port, } + + # SERVER_PORT is optional but must not be set if it is not present. + if server_port + env[CGI::SERVER_PORT] = server_port + end self.unwrap_request(request, env) diff --git a/lib/protocol/rack/adapter/rack31.rb b/lib/protocol/rack/adapter/rack31.rb index 0e1aae5..b2cf73e 100644 --- a/lib/protocol/rack/adapter/rack31.rb +++ b/lib/protocol/rack/adapter/rack31.rb @@ -46,8 +46,12 @@ def make_environment(request) # I'm not sure what sane defaults should be here: CGI::SERVER_NAME => server_name, - CGI::SERVER_PORT => server_port, } + + # SERVER_PORT is optional but must not be set if it is not present. + if server_port + env[CGI::SERVER_PORT] = server_port + end if body = request.body if body.empty?