Fix `NoMethodError` in `Request#wrap_ipv6` when `x-forwarded-host` is empty by oieioi · Pull Request #2270 · rack/rack

oieioi · 2024-12-26T09:43:18Z

Currently, sending a request with an empty x-forwarded-host causes a NoMethodError due to nil access in wrap_ipv6. Since forwarded_authority is the only method that could pass nil to wrap_ipv6, I added a nil check in forwarded_authority to prevent this issue.

For example, when making a request to a Rack server using JavaScript, an error occurred on the server.

// client.js
let headers = new Headers
headers.set('x-forwarded-host', '')
let res = await fetch('http://rack-server.test', { method: 'GET', headers: headers})

# server.rb
# ...
logger.info("host_with_port: #{request.host_with_port}")
# ...

#<NoMethodError: undefined method `start_with?' for nil:NilClass
if !host.start_with?('[') && host.count(':') > 1
        ^^^^^^^^^^^^>

dentarg · 2024-12-26T15:23:49Z

Is the problem really in Rack? Can you provide the full backtrace?

I tried to reproduce it but couldn't:

$ echo 'require "rack";app { |env| req = ::Rack::Request.new(env); pp req.host_with_port; [200, {}, ["OK"]] }' | puma --config /dev/stdin --log-requests --port 8888
Puma starting in single mode...
* Puma version: 6.5.0 ("Sky's Version")
* Ruby version: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [arm64-darwin23]
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 63959
* Listening on http://0.0.0.0:8888
Use Ctrl-C to stop
"localhost:8888"
127.0.0.1 - - [26/Dec/2024:16:22:15 +0100] "GET / HTTP/1.1" 200 - 0.0225
"\"\""
127.0.0.1 - - [26/Dec/2024:16:22:33 +0100] "GET / HTTP/1.1" 200 - 0.0004

$ curl -H 'x-forwarded-host: ' localhost:8888
OK

$ curl -H 'x-forwarded-host: ""' localhost:8888
OK

oieioi · 2024-12-26T22:13:46Z

@dentarg
curl automatically removes empty headers, so you need to use an HTTP client like Node.js's fetch or Ruby's net/http library, which do not remove empty headers, to make the request.

server

# puma-config.rb
require 'rack'

app { |env|
  req = ::Rack::Request.new(env);
  pp req.env['HTTP_X_FORWARDED_HOST']
  pp req.env['HTTP_X_FORWARDED_HOST'].class
  begin
    pp req.host_with_port
    [200, {}, ["OK"]]
  rescue => e
    pp e
    [500, {}, ['ERROR']]
  end
}

$ puma --config puma-config.rb --log-requests --port 8888
Puma starting in single mode...
* Puma version: 6.5.0 ("Sky's Version")
* Ruby version: ruby 3.2.3 (2024-01-18 revision 52bb2ac0a6) [x86_64-linux]
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 18853
* Listening on http://0.0.0.0:8888
Use Ctrl-C to stop
""
String
#<NoMethodError: undefined method `start_with?' for nil:NilClass>
127.0.0.1 - - [27/Dec/2024:07:02:47 +0900] "GET / HTTP/1.1" 500 - 0.0452
nil
NilClass
"localhost:8888"
127.0.0.1 - - [27/Dec/2024:07:04:12 +0900] "GET / HTTP/1.1" 200 - 0.0007

request

$ ruby -r 'net/http' -e "pp Net::HTTP.get(URI.parse('http://localhost:8888'), { 'x-forwarded-host': '' })"
"ERROR"

$ curl -H 'x-forwarded-host: ' localhost:8888
OK

oieioi · 2024-12-26T22:32:33Z

Here is an example of outputting e.backtrace by modifying the code shown above.

# puma-config.rb
require 'rack'

app { |env|
  req = ::Rack::Request.new(env);
  pp req.env['HTTP_X_FORWARDED_HOST']
  pp req.env['HTTP_X_FORWARDED_HOST'].class
  begin
    pp req.host_with_port
    [200, {}, ["OK"]]
  rescue => e
    # pp e
    pp e.backtrace
    [500, {}, ['ERROR']]
  end
}

$ puma --config puma-config.rb --log-requests --port 8888
Puma starting in single mode...
* Puma version: 6.5.0 ("Sky's Version")
* Ruby version: ruby 3.2.3 (2024-01-18 revision 52bb2ac0a6) [x86_64-linux]
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 22231
* Listening on http://0.0.0.0:8888
Use Ctrl-C to stop
""
String
["/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/rack-3.1.8/lib/rack/request.rb:637:in `wrap_ipv6'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/rack-3.1.8/lib/rack/request.rb:402:in `block in forwarded_authority'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/rack-3.1.8/lib/rack/request.rb:394:in `each'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/rack-3.1.8/lib/rack/request.rb:394:in `forwarded_authority'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/rack-3.1.8/lib/rack/request.rb:267:in `authority'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/rack-3.1.8/lib/rack/request.rb:322:in `host_with_port'",
 "puma-config.rb:8:in `block in _load_from'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/puma-6.5.0/lib/puma/commonlogger.rb:47:in `call'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/puma-6.5.0/lib/puma/configuration.rb:279:in `call'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/puma-6.5.0/lib/puma/request.rb:99:in `block in handle_request'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/puma-6.5.0/lib/puma/thread_pool.rb:389:in `with_force_shutdown'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/puma-6.5.0/lib/puma/request.rb:98:in `handle_request'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/puma-6.5.0/lib/puma/server.rb:468:in `process_client'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/puma-6.5.0/lib/puma/server.rb:249:in `block in run'",
 "/home/oieioi/.anyenv/envs/rbenv/versions/3.2.3/lib/ruby/gems/3.2.0/gems/puma-6.5.0/lib/puma/thread_pool.rb:166:in `block in spawn_thread'"]
127.0.0.1 - - [27/Dec/2024:07:27:18 +0900] "GET / HTTP/1.1" 500 - 0.0261

$ ruby -r 'net/http' -e "pp Net::HTTP.get(URI.parse('http://localhost:8888'), { 'x-forwarded-host': '' })"
"ERROR"

jeremyevans

Thanks for the patch! I agree this is a bug that we should fix.

dentarg · 2024-12-27T09:06:59Z

curl automatically removes empty headers

TIL, thanks!

dentarg · 2024-12-27T10:37:00Z

You can trigger the bug with curl like this: curl -s -v -H 'x-forwarded-host;' localhost:8888

oieioi · 2024-12-27T11:02:18Z

You can trigger the bug with curl like this: curl -s -v -H 'x-forwarded-host;' localhost:8888

Thank you! I was able to reproduce it with that command.

mikegee

Seems legit to me too. Are there any concerns preventing merge?

oieioi · 2025-02-03T08:45:10Z

@jeremyevans @dentarg
I've fixed your feedbacks. Please review again.
a67a50c
50126fc

What can I do for this PR?

…d-host` is empty Currently, sending a request with an empty `x-forwarded-host` causes nil access in `wrap_ipv6`, resulting in a `NoMethodError`. Added a nil check in `forwarded_authority` to prevent this issue.

Co-authored-by: Jeremy Evans <code@jeremyevans.net>

ioquatix · 2025-02-21T22:25:23Z

Thanks for your contribution.

oieioi changed the title ~~Fix NoMethodError in Request#forwarded_authority when x-forwarded-host is empty~~ Fix NoMethodError in Request#wrap_ipv6 when x-forwarded-host is empty Dec 26, 2024

jeremyevans approved these changes Dec 26, 2024

View reviewed changes

Comment thread lib/rack/request.rb Outdated

dentarg reviewed Dec 27, 2024

View reviewed changes

Comment thread test/spec_request.rb Outdated

mikegee approved these changes Jan 23, 2025

View reviewed changes

oieioi and others added 4 commits February 22, 2025 11:16

Fix NoMethodError in Request#forwarded_authority when `x-forwarde…

d88d6aa

…d-host` is empty Currently, sending a request with an empty `x-forwarded-host` causes nil access in `wrap_ipv6`, resulting in a `NoMethodError`. Added a nil check in `forwarded_authority` to prevent this issue.

Update lib/rack/request.rb

37d3f90

Co-authored-by: Jeremy Evans <code@jeremyevans.net>

Fix wrong test data

f738e0b

Add a line to CHANGELOG

02ece97

ioquatix force-pushed the fix-x-forwarded-host-nil-exception branch from 3a75f53 to 02ece97 Compare February 21, 2025 22:16

ioquatix merged commit 3e382ab into rack:main Feb 21, 2025

oieioi deleted the fix-x-forwarded-host-nil-exception branch February 25, 2025 04:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `NoMethodError` in `Request#wrap_ipv6` when `x-forwarded-host` is empty#2270

Fix `NoMethodError` in `Request#wrap_ipv6` when `x-forwarded-host` is empty#2270
ioquatix merged 4 commits into
rack:mainfrom
oieioi:fix-x-forwarded-host-nil-exception

oieioi commented Dec 26, 2024 •

edited

Loading

Uh oh!

dentarg commented Dec 26, 2024

Uh oh!

oieioi commented Dec 26, 2024

Uh oh!

oieioi commented Dec 26, 2024

Uh oh!

jeremyevans left a comment

Uh oh!

Uh oh!

Uh oh!

dentarg commented Dec 27, 2024

Uh oh!

dentarg commented Dec 27, 2024

Uh oh!

oieioi commented Dec 27, 2024

Uh oh!

mikegee left a comment

Uh oh!

oieioi commented Feb 3, 2025

Uh oh!

ioquatix commented Feb 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

oieioi commented Dec 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dentarg commented Dec 26, 2024

Uh oh!

oieioi commented Dec 26, 2024

Uh oh!

oieioi commented Dec 26, 2024

Uh oh!

jeremyevans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dentarg commented Dec 27, 2024

Uh oh!

dentarg commented Dec 27, 2024

Uh oh!

oieioi commented Dec 27, 2024

Uh oh!

mikegee left a comment

Choose a reason for hiding this comment

Uh oh!

oieioi commented Feb 3, 2025

Uh oh!

ioquatix commented Feb 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

oieioi commented Dec 26, 2024 •

edited

Loading