From c24c0c883496f581d9092bbe7f7431129eeb7190 Mon Sep 17 00:00:00 2001
From: schneems <richard.schneeman+foo@gmail.com>
Date: Fri, 4 Jan 2019 13:15:31 -0600
Subject: [PATCH] Rack handler should use provided default host

This issue is somewhat tricky. When Rails is booted via `rails server` there are two types of configuration options passed, ones specified directly by a user like `rails s -p 3001` will always "win".

For any other config that is not explicitly passed in, puma will consider it a "default". For example when you run `rails s` (without -p) then the default port will be 3000.

There is one other way to configure puma though, and that is via a config file:

```
# config/puma.rb
port 3002
```

This is the order of precedence for configuration

1) Anything the user explicitly passes to `rails s`
2) Config specified in `config/puma.rb` file
3) Default values passed in via `rails s`
4) Defaults values stored in puma

This fallback mechanism works well except in the case of calling `port` in a `config/puma.rb` file. To understand look at the [old method definition](https://github.com/puma/puma/blob/2668597ec1dd9546d83db9f2ec5ad092add483e6/lib/puma/dsl.rb#L140-L145):

```
def port(port, host=nil)
  host ||= Configuration::DefaultTCPHost
  bind "tcp://#{host}:#{port}"
end
```

When the `port` method gets called, even if the user did not specify a `host` the `Configuration::DefaultTCPHost` will be used, which is a problem for local development because it defaults to `0.0.0.0`. [SO about 0.0.0.0 versus localhost](https://stackoverflow.com/questions/20778771/what-is-the-difference-between-0-0-0-0-127-0-0-1-and-localhost).

In this case, while a user did directly specify a port, they did not specify a host, so you would expect the `rails s` defaults passed in to take affect.

To make Puma respect that the host coming from `rails s` has more precedence than it's own default host, we must introduce the ability to set and retrieve a default_host value.

This is then used in the rack handler so when `rails s` passes in `:Host => "localhost"` then it is used instead of reverting to `0.0.0.0`.

The issue with #1699 is the test was wrong, it would have failed if a config file was present with a `port` invocation.
---
 lib/puma/dsl.rb           | 10 +++++++++-
 lib/rack/handler/puma.rb  |  3 +++
 test/test_rack_handler.rb | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/lib/puma/dsl.rb b/lib/puma/dsl.rb
index b5bebf43ce..a342200edf 100644
--- a/lib/puma/dsl.rb
+++ b/lib/puma/dsl.rb
@@ -57,6 +57,14 @@ def _offer_plugins
       @plugins.clear
     end
 
+    def set_default_host(host)
+      @options[:default_host] = host
+    end
+
+    def default_host
+      @options[:default_host] || Configuration::DefaultTCPHost
+    end
+
     def inject(&blk)
       instance_eval(&blk)
     end
@@ -140,7 +148,7 @@ def clear_binds!
     # Define the TCP port to bind to. Use +bind+ for more advanced options.
     #
     def port(port, host=nil)
-      host ||= Configuration::DefaultTCPHost
+      host ||= default_host
       bind "tcp://#{host}:#{port}"
     end
 
diff --git a/lib/rack/handler/puma.rb b/lib/rack/handler/puma.rb
index 3ff189584b..4607b7c103 100644
--- a/lib/rack/handler/puma.rb
+++ b/lib/rack/handler/puma.rb
@@ -49,6 +49,9 @@ def self.config(app, options = {})
             self.set_host_port_to_config(host, port, user_config)
           end
 
+          if default_options[:Host]
+            file_config.set_default_host(default_options[:Host])
+          end
           self.set_host_port_to_config(default_options[:Host], default_options[:Port], default_config)
 
           user_config.app app
diff --git a/test/test_rack_handler.rb b/test/test_rack_handler.rb
index 5007e20e59..3f3a6987c9 100644
--- a/test/test_rack_handler.rb
+++ b/test/test_rack_handler.rb
@@ -123,6 +123,44 @@ def test_config_file_wins_over_port
       end
     end
   end
+
+  def test_default_host_when_using_config_file
+    user_port = 5001
+    file_port = 6001
+
+    Dir.mktmpdir do |d|
+      Dir.chdir(d) do
+        FileUtils.mkdir("config")
+        File.open("config/puma.rb", "w") { |f| f << "port #{file_port}" }
+
+        @options[:Host] = "localhost"
+        @options[:Port] = user_port
+        conf = Rack::Handler::Puma.config(->{}, @options)
+        conf.load
+
+        assert_equal ["tcp://localhost:#{file_port}"], conf.options[:binds]
+      end
+    end
+  end
+
+  def test_default_host_when_using_config_file_with_explicit_host
+    user_port = 5001
+    file_port = 6001
+
+    Dir.mktmpdir do |d|
+      Dir.chdir(d) do
+        FileUtils.mkdir("config")
+        File.open("config/puma.rb", "w") { |f| f << "port #{file_port}, '1.2.3.4'" }
+
+        @options[:Host] = "localhost"
+        @options[:Port] = user_port
+        conf = Rack::Handler::Puma.config(->{}, @options)
+        conf.load
+
+        assert_equal ["tcp://1.2.3.4:#{file_port}"], conf.options[:binds]
+      end
+    end
+  end
 end
 
 class TestUserSuppliedOptionsIsNotPresent < Minitest::Test