Merge pull request #329 from Pylons/cleanup/server-name

Cleanup: Server Name deduction logic

5 files changed, 41 insertions, 83 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 8dd82b5..6e28bdd 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,13 +1,22 @@
 2.0.0 (unreleased)
 ------------------
+
+- Waitress no longer attempts to guess at what the ``server_name`` should be for
+  a listen socket, instead it always use a new adjustment/argument named
+  ``server_name``.
+
+  Please see the documentation for ``server_name`` in
+  https://docs.pylonsproject.org/projects/waitress/en/latest/arguments.html and
+  see https://github.com/Pylons/waitress/pull/329
+
 - Allow tasks to notice if the client disconnected.
 
-  This inserts a callable `waitress.client_disconnected` into the environment
+  This inserts a callable ``waitress.client_disconnected`` into the environment
   that allows the task to check if the client disconnected while waiting for
   the response at strategic points in the execution and to cancel the
   operation.
 
-  It requires setting the new adjustment `channel_request_lookahead` to a value
+  It requires setting the new adjustment ``channel_request_lookahead`` to a value
   larger than 0, which continues to read requests from a channel even if a
   request is already being processed on that channel, up to the given count,
   since a client disconnect is detected by reading from a readable socket and
@@ -15,7 +24,7 @@
 
   See https://github.com/Pylons/waitress/pull/310
 
-- Drop Python 2.7 support
+- Drop Python 2.7, 3.5 support
 
 - The server now issues warning output when it there are enough open
   connections (controlled by "connection_limit"), that it is no longer
diff --git a/docs/arguments.rst b/docs/arguments.rst
index 3822f2c..f9b9310 100644
--- a/docs/arguments.rst
+++ b/docs/arguments.rst
@@ -35,6 +35,23 @@ listen
 
     .. versionadded:: 1.0
 
+server_name
+    This is the value that will be placed in the WSGI environment as
+    ``SERVER_NAME``, the only time that this value is used in the WSGI
+    environment for a request is if the client sent a HTTP/1.0 request without
+    a ``Host`` header set, and no other proxy headers.
+
+    The default is value is ``waitress.invalid``, if your WSGI application is
+    creating URL's that include this as the hostname and you are using a
+    reverse proxy setup, you may want to validate that your reverse proxy is
+    sending the appropriate headers.
+
+    In most situations you will not need to set this value.
+
+    Default: ``waitress.invalid``
+
+    .. versionadded:: 2.0
+
 ipv4
     Enable or disable IPv4 (boolean)
 
diff --git a/src/waitress/adjustments.py b/src/waitress/adjustments.py
index 42d2bc0..466b5c4 100644
--- a/src/waitress/adjustments.py
+++ b/src/waitress/adjustments.py
@@ -136,6 +136,7 @@ class Adjustments:
         ("unix_socket_perms", asoctal),
         ("sockets", as_socket_list),
         ("channel_request_lookahead", int),
+        ("server_name", str),
     )
 
     _param_map = dict(_params)
@@ -288,6 +289,11 @@ class Adjustments:
     # is being processed.
     channel_request_lookahead = 0
 
+    # This setting controls the SERVER_NAME of the WSGI environment, this is
+    # only ever used if the remote client sent a request without a Host header
+    # (or when using the Proxy settings, without forwarding a Host header)
+    server_name = "waitress.invalid"
+
     def __init__(self, **kw):
 
         if "listen" in kw and ("host" in kw or "port" in kw):
diff --git a/src/waitress/server.py b/src/waitress/server.py
index b37980e..fe7b6da 100644
--- a/src/waitress/server.py
+++ b/src/waitress/server.py
@@ -241,7 +241,7 @@ class BaseWSGIServer(wasyncore.dispatcher):
             self.bind_server_socket()
 
         self.effective_host, self.effective_port = self.getsockname()
-        self.server_name = self.get_server_name(self.effective_host)
+        self.server_name = adj.server_name
         self.active_channels = {}
         if _start:
             self.accept_connections()
@@ -249,39 +249,6 @@ class BaseWSGIServer(wasyncore.dispatcher):
     def bind_server_socket(self):
         raise NotImplementedError  # pragma: no cover
 
-    def get_server_name(self, ip):
-        """Given an IP or hostname, try to determine the server name."""
-
-        if not ip:
-            raise ValueError("Requires an IP to get the server name")
-
-        server_name = str(ip)
-
-        # If we are bound to all IP's, just return the current hostname, only
-        # fall-back to "localhost" if we fail to get the hostname
-        if server_name == "0.0.0.0" or server_name == "::":
-            try:
-                return str(self.socketmod.gethostname())
-            except (OSError, UnicodeDecodeError):  # pragma: no cover
-                # We also deal with UnicodeDecodeError in case of Windows with
-                # non-ascii hostname
-                return "localhost"
-
-        # Now let's try and convert the IP address to a proper hostname
-        try:
-            server_name = self.socketmod.gethostbyaddr(server_name)[0]
-        except (OSError, UnicodeDecodeError):  # pragma: no cover
-            # We also deal with UnicodeDecodeError in case of Windows with
-            # non-ascii hostname
-            pass
-
-        # If it contains an IPv6 literal, make sure to surround it with
-        # brackets
-        if ":" in server_name and "[" not in server_name:
-            server_name = "[{}]".format(server_name)
-
-        return server_name
-
     def getsockname(self):
         raise NotImplementedError  # pragma: no cover
 
@@ -391,20 +358,11 @@ class TcpWSGIServer(BaseWSGIServer):
         self.bind(sockaddr)
 
     def getsockname(self):
-        try:
-            return self.socketmod.getnameinfo(
-                self.socket.getsockname(), self.socketmod.NI_NUMERICSERV
-            )
-        except:  # pragma: no cover
-            # This only happens on Linux because a DNS issue is considered a
-            # temporary failure that will raise (even when NI_NAMEREQD is not
-            # set). Instead we try again, but this time we just ask for the
-            # numerichost and the numericserv (port) and return those. It is
-            # better than nothing.
-            return self.socketmod.getnameinfo(
-                self.socket.getsockname(),
-                self.socketmod.NI_NUMERICHOST | self.socketmod.NI_NUMERICSERV,
-            )
+        # Return the IP address, port as numeric
+        return self.socketmod.getnameinfo(
+            self.socket.getsockname(),
+            self.socketmod.NI_NUMERICHOST | self.socketmod.NI_NUMERICSERV,
+        )
 
     def set_socket_options(self, conn):
         for (level, optname, value) in self.adj.socket_options:
@@ -451,9 +409,6 @@ if hasattr(socket, "AF_UNIX"):
         def fix_addr(self, addr):
             return ("localhost", None)
 
-        def get_server_name(self, ip):
-            return "localhost"
-
 
 # Compatibility alias.
 WSGIServer = TcpWSGIServer
diff --git a/tests/test_server.py b/tests/test_server.py
index 508b382..6edc3b2 100644
--- a/tests/test_server.py
+++ b/tests/test_server.py
@@ -113,35 +113,6 @@ class TestWSGIServer(unittest.TestCase):
         inst = self._makeOneWithMap(_start=False)
         self.assertEqual(inst.accepting, False)
 
-    def test_get_server_name_empty(self):
-        inst = self._makeOneWithMap(_start=False)
-        self.assertRaises(ValueError, inst.get_server_name, "")
-
-    def test_get_server_name_with_ip(self):
-        inst = self._makeOneWithMap(_start=False)
-        result = inst.get_server_name("127.0.0.1")
-        self.assertTrue(result)
-
-    def test_get_server_name_with_hostname(self):
-        inst = self._makeOneWithMap(_start=False)
-        result = inst.get_server_name("fred.flintstone.com")
-        self.assertEqual(result, "fred.flintstone.com")
-
-    def test_get_server_name_0000(self):
-        inst = self._makeOneWithMap(_start=False)
-        result = inst.get_server_name("0.0.0.0")
-        self.assertTrue(len(result) != 0)
-
-    def test_get_server_name_double_colon(self):
-        inst = self._makeOneWithMap(_start=False)
-        result = inst.get_server_name("::")
-        self.assertTrue(len(result) != 0)
-
-    def test_get_server_name_ipv6(self):
-        inst = self._makeOneWithMap(_start=False)
-        result = inst.get_server_name("2001:DB8::ffff")
-        self.assertEqual("[2001:DB8::ffff]", result)
-
     def test_get_server_multi(self):
         inst = self._makeOneWithMulti()
         self.assertEqual(inst.__class__.__name__, "MultiSocketServer")