Merge pull request #228 from Pylons/proxy-updates

fix a few proxy issues after testing in production

4 files changed, 36 insertions, 20 deletions

diff --git a/docs/reverse-proxy.rst b/docs/reverse-proxy.rst
index 7081fda..6490e3d 100644
--- a/docs/reverse-proxy.rst
+++ b/docs/reverse-proxy.rst
@@ -9,13 +9,13 @@ Using Behind a Reverse Proxy
 Often people will set up "pure Python" web servers behind reverse proxies,
 especially if they need TLS support (Waitress does not natively support TLS).
 Even if you don't need TLS support, it's not uncommon to see Waitress and
-other pure-Python web servers set up to "live" behind a reverse proxy; these
-proxies often have lots of useful deployment knobs.
+other pure-Python web servers set up to only handle requests behind a reverse proxy;
+these proxies often have lots of useful deployment knobs.
 
 If you're using Waitress behind a reverse proxy, you'll almost always want
 your reverse proxy to pass along the ``Host`` header sent by the client to
 Waitress, in either case, as it will be used by most applications to generate
-correct URLs. You may also use the proxy headers if passing the Host directly
+correct URLs. You may also use the proxy headers if passing ``Host`` directly
 is not possible, or there are multiple proxies involved.
 
 For example, when using nginx as a reverse proxy, you might add the following
@@ -83,7 +83,7 @@ You will also want to add to Apache::
 
 Configure waitress's ``trusted_proxy_headers`` as appropriate::
 
-    trusted_proxy_headers = "x-forwarded-for, x-forwarded-host, x-forwarded-proto, x-forwarded-port"
+    trusted_proxy_headers = "x-forwarded-for x-forwarded-host x-forwarded-proto x-forwarded-port"
 
 At this point waitress will set up the WSGI environment using the information
 specified in the trusted proxy headers. This will setup the following
diff --git a/waitress/adjustments.py b/waitress/adjustments.py
index c1aa66d..aa55c73 100644
--- a/waitress/adjustments.py
+++ b/waitress/adjustments.py
@@ -156,7 +156,7 @@ class Adjustments(object):
     # only 1 valid proxy, then that initial IP address "192.0.2.1" is not
     # trusted and we completely ignore it. If there are two trusted proxies in
     # the path, this value should be set to a higher number.
-    trusted_proxy_count = 1
+    trusted_proxy_count = None
 
     # Which of the proxy headers should we trust, this is a set where you
     # either specify forwarded or one or more of forwarded-host, forwarded-for,
@@ -327,7 +327,8 @@ class Adjustments(object):
                 try:
                     # Try turning the port into an integer
                     port = int(port)
-                except:
+
+                except Exception:
                     raise ValueError(
                         'Windows does not support service names instead of port numbers'
                     )
@@ -364,24 +365,29 @@ class Adjustments(object):
                     ):
                         wanted_sockets.append((family, socktype, proto, sockaddr))
                         hp_pairs.append((sockaddr[0].split('%', 1)[0], sockaddr[1]))
-            except:
+
+            except Exception:
                 raise ValueError('Invalid host/port specified.')
 
-        if (
-            self.trusted_proxy is None and
-            (
-                self.trusted_proxy_headers or
-                (self.clear_untrusted_proxy_headers is not _bool_marker)
+        if self.trusted_proxy_count is not None and self.trusted_proxy is None:
+            raise ValueError(
+                "trusted_proxy_count has no meaning without setting "
+                "trusted_proxy"
             )
-        ):
+
+        elif self.trusted_proxy_count is None:
+            self.trusted_proxy_count = 1
+
+        if self.trusted_proxy_headers and self.trusted_proxy is None:
             raise ValueError(
-                "The values trusted_proxy_headers and clear_untrusted_proxy_headers "
-                "have no meaning without setting trusted_proxy. Cowardly refusing to "
-                "continue."
+                "trusted_proxy_headers has no meaning without setting "
+                "trusted_proxy"
             )
 
         if self.trusted_proxy_headers:
-            self.trusted_proxy_headers = {header.lower() for header in self.trusted_proxy_headers}
+            self.trusted_proxy_headers = {
+                header.lower() for header in self.trusted_proxy_headers
+            }
 
             unknown_values = self.trusted_proxy_headers - KNOWN_PROXY_HEADERS
             if unknown_values:
@@ -399,6 +405,7 @@ class Adjustments(object):
                     "X-Forwarded-{By,Host,Proto,Port,For} headers are mutually "
                     "exclusive. Can't trust both!"
                 )
+
         elif self.trusted_proxy is not None:
             warnings.warn(
                 'No proxy headers were marked as trusted, but trusted_proxy was set. '
@@ -408,7 +415,7 @@ class Adjustments(object):
             )
             self.trusted_proxy_headers = {'x-forwarded-proto'}
 
-        if self.trusted_proxy and self.clear_untrusted_proxy_headers is _bool_marker:
+        if self.clear_untrusted_proxy_headers is _bool_marker:
             warnings.warn(
                 'In future versions of Waitress clear_untrusted_proxy_headers will be '
                 'set to True by default. You may opt-out by setting this value to '
diff --git a/waitress/tests/test_adjustments.py b/waitress/tests/test_adjustments.py
index f841efc..e35fdaf 100644
--- a/waitress/tests/test_adjustments.py
+++ b/waitress/tests/test_adjustments.py
@@ -315,12 +315,21 @@ class TestAdjustments(unittest.TestCase):
             str(cm.exception)
         )
 
+    def test_trusted_proxy_count_no_trusted_proxy(self):
+        with self.assertRaises(ValueError) as cm:
+            self._makeOne(trusted_proxy_count=1)
+
+        self.assertIn(
+            'trusted_proxy_count has no meaning',
+            str(cm.exception)
+        )
+
     def test_trusted_proxy_headers_no_trusted_proxy(self):
         with self.assertRaises(ValueError) as cm:
             self._makeOne(trusted_proxy_headers={'forwarded'})
 
         self.assertIn(
-            'Cowardly refusing to continue.',
+            'trusted_proxy_headers has no meaning',
             str(cm.exception)
         )
 
diff --git a/waitress/utilities.py b/waitress/utilities.py
index 2d56e35..a21ca4e 100644
--- a/waitress/utilities.py
+++ b/waitress/utilities.py
@@ -271,7 +271,7 @@ def clear_untrusted_headers(
         if headers.pop(header, False) is not False
     ]
 
-    if any(untrusted_headers_removed) and log_warning:
+    if log_warning and untrusted_headers_removed:
         untrusted_headers_removed = [
             "-".join([x.capitalize() for x in header.split("_")])
             for header in untrusted_headers_removed