Merge pull request #42 from Pylons/feature.x_wsgi_url_scheme-header

Override 'wsgi.url_scheme' via a request header, 'X_WSGI_URL_SCHEME'.

8 files changed, 166 insertions, 15 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index b8611ca..7fa486b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,3 +1,11 @@
+Unreleased
+----------
+
+- Allow trusted proxies to override ``wsgi.url_scheme`` via a request header,
+  ``X_FORWARDED_PROTO``.  Allows proxies which serve mixed HTTP / HTTPS
+  requests to control signal which are served as HTTPS.  See
+  https://github.com/Pylons/waitress/pull/42.
+
 0.8.8 (2013-11-30)
 ------------------
 
diff --git a/docs/arguments.rst b/docs/arguments.rst
index 5e75db3..f96a3ff 100644
--- a/docs/arguments.rst
+++ b/docs/arguments.rst
@@ -28,8 +28,14 @@ threads
     number of threads used to process application logic (integer), default
     ``4``
 
+trusted_proxy
+    IP addreess of a client allowed to override ``url_scheme`` via the
+    ``X_FORWARDED_PROTO`` header.
+
 url_scheme
-    default ``wsgi.url_scheme`` value (string), default ``http``
+    default ``wsgi.url_scheme`` value (string), default ``http``;  can be
+    overridden per-request by the value of the ``X_FORWARDED_PROTO`` header,
+    but only if the client address matches ``trusted_proxy``.
 
 ident
     server identity (string) used in "Server:" header in responses, default
diff --git a/docs/differences.rst b/docs/differences.rst
index 07783fa..f4af227 100644
--- a/docs/differences.rst
+++ b/docs/differences.rst
@@ -13,6 +13,9 @@ Differences from ``zope.server``
 
 - Calls "close()" on the app_iter object returned by the WSGI application.
 
+- Allows trusted proxies to override ``wsgi.url_scheme`` for particular
+  requests by supplying the ``X_FORWARDED_PROTO`` header.
+
 - Supports an explicit ``wsgi.url_scheme`` parameter for ease of deployment
   behind SSL proxies.
 
diff --git a/docs/index.rst b/docs/index.rst
index c09566f..cf0eef8 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -119,12 +119,18 @@ which start with ``https://``), the URLs generated by your application when
 used behind a reverse proxy served by Waitress might inappropriately be
 ``http://foo`` rather than ``https://foo``.  To fix this, you'll want to
 change the ``wsgi.url_scheme`` in the WSGI environment before it reaches your
-application.  You can do this in one of two ways:
+application.  You can do this in one of three ways:
 
 1.  You can pass a ``url_scheme`` configuration variable to the
     ``waitress.serve`` function.
 
-2.  You can use Paste's ``PrefixMiddleware`` in conjunction with
+2.  You can configure the proxy reverse server to pass a header,
+    ``X_FORWARDED_PROTO``, whose value will be set for that request as
+    the ``wsgi.url_scheme`` environment value.  Note that you must also
+    conigure ``waitress.serve`` by passing the IP address of that proxy
+    as its ``trusted_proxy``.
+
+3.  You can use Paste's ``PrefixMiddleware`` in conjunction with
     configuration settings on the reverse proxy server.
 
 Using ``url_scheme`` to set ``wsgi.url_scheme``
@@ -138,6 +144,26 @@ You can have the Waitress server use the ``https`` url scheme by default.::
 This works if all URLs generated by your application should use the ``https``
 scheme.
 
+Passing the ``X_FORWARDED_PROTO`` header to set ``wsgi.url_scheme``
+-------------------------------------------------------------------
+
+If your proxy accepts both HTTP and HTTPS URLs, and you want your application
+to generate the appropriate url based on the incoming scheme, also set up
+your proxy to send a ``X-Forwarded-Proto`` with the original URL scheme along
+with each proxied request.  For example, when using Nginx::
+
+    proxy_set_header        X-Forwarded-Proto $scheme;
+
+or via Apache::
+
+   RequestHeader set X-Forwarded-Proto https
+
+.. note::
+
+   You must also configure the Waitress server's ``trusted_proxy`` to
+   contain the IP address of the proxy in order for this header to override
+   the default URL scheme.
+
 Using ``url_prefix`` to influence ``SCRIPT_NAME`` and ``PATH_INFO``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/waitress/adjustments.py b/waitress/adjustments.py
index 2835e97..d5b237b 100644
--- a/waitress/adjustments.py
+++ b/waitress/adjustments.py
@@ -52,6 +52,7 @@ class Adjustments(object):
         ('host', str),
         ('port', int),
         ('threads', int),
+        ('trusted_proxy', str),
         ('url_scheme', str),
         ('url_prefix', slash_fixed_str),
         ('backlog', int),
@@ -84,6 +85,9 @@ class Adjustments(object):
     # mumber of threads available for tasks
     threads = 4
 
+    # Host allowed to overrid ``wsgi.url_scheme`` via header
+    trusted_proxy = None
+
     # default ``wsgi.url_scheme`` value
     url_scheme = 'http'
 
diff --git a/waitress/task.py b/waitress/task.py
index a4c8f2e..e28df4c 100644
--- a/waitress/task.py
+++ b/waitress/task.py
@@ -483,9 +483,17 @@ class WSGITask(Task):
         environ['SCRIPT_NAME'] = url_prefix
         environ['PATH_INFO'] = path
         environ['QUERY_STRING'] = request.query
-        environ['REMOTE_ADDR'] = channel.addr[0]
+        host = environ['REMOTE_ADDR'] = channel.addr[0]
 
-        for key, value in request.headers.items():
+        headers = dict(request.headers)
+        if host == server.adj.trusted_proxy:
+            wsgi_url_scheme = headers.pop('X_FORWARDED_PROTO',
+                                          request.url_scheme)
+        else:
+            wsgi_url_scheme = request.url_scheme
+        if wsgi_url_scheme not in ('http', 'https'):
+            raise ValueError('Invalid X_FORWARDED_PROTO value')
+        for key, value in headers.items():
             value = value.strip()
             mykey = rename_headers.get(key, None)
             if mykey is None:
@@ -495,7 +503,7 @@ class WSGITask(Task):
 
         # the following environment variables are required by the WSGI spec
         environ['wsgi.version'] = (1, 0)
-        environ['wsgi.url_scheme'] = request.url_scheme
+        environ['wsgi.url_scheme'] = wsgi_url_scheme
         environ['wsgi.errors'] = sys.stderr # apps should use the logging module
         environ['wsgi.multithread'] = True
         environ['wsgi.multiprocess'] = False
diff --git a/waitress/tests/test_adjustments.py b/waitress/tests/test_adjustments.py
index fe390dc..f2b28c2 100644
--- a/waitress/tests/test_adjustments.py
+++ b/waitress/tests/test_adjustments.py
@@ -51,18 +51,34 @@ class TestAdjustments(unittest.TestCase):
 
     def test_goodvars(self):
         inst = self._makeOne(
-            host='host', port='8080', threads='5',
-            url_scheme='https', backlog='20', recv_bytes='200',
-            send_bytes='300', outbuf_overflow='400', inbuf_overflow='500',
-            connection_limit='1000', cleanup_interval='1100',
-            channel_timeout='1200', log_socket_errors='true',
-            max_request_header_size='1300', max_request_body_size='1400',
-            expose_tracebacks='true', ident='abc', asyncore_loop_timeout='5',
-            asyncore_use_poll=True, unix_socket='/tmp/waitress.sock',
-            unix_socket_perms='777', url_prefix='///foo/')
+            host='host',
+            port='8080',
+            threads='5',
+            trusted_proxy='192.168.1.1',
+            url_scheme='https',
+            backlog='20',
+            recv_bytes='200',
+            send_bytes='300',
+            outbuf_overflow='400',
+            inbuf_overflow='500',
+            connection_limit='1000',
+            cleanup_interval='1100',
+            channel_timeout='1200',
+            log_socket_errors='true',
+            max_request_header_size='1300',
+            max_request_body_size='1400',
+            expose_tracebacks='true',
+            ident='abc',
+            asyncore_loop_timeout='5',
+            asyncore_use_poll=True,
+            unix_socket='/tmp/waitress.sock',
+            unix_socket_perms='777',
+            url_prefix='///foo/',
+        )
         self.assertEqual(inst.host, 'host')
         self.assertEqual(inst.port, 8080)
         self.assertEqual(inst.threads, 5)
+        self.assertEqual(inst.trusted_proxy, '192.168.1.1')
         self.assertEqual(inst.url_scheme, 'https')
         self.assertEqual(inst.backlog, 20)
         self.assertEqual(inst.recv_bytes, 200)
diff --git a/waitress/tests/test_task.py b/waitress/tests/test_task.py
index 8cae8dc..14a84f3 100644
--- a/waitress/tests/test_task.py
+++ b/waitress/tests/test_task.py
@@ -650,6 +650,85 @@ class TestWSGITask(unittest.TestCase):
         self.assertEqual(environ['wsgi.input'], 'stream')
         self.assertEqual(inst.environ, environ)
 
+    def test_get_environment_values_w_scheme_override_untrusted(self):
+        inst = self._makeOne()
+        request = DummyParser()
+        request.headers = {
+            'CONTENT_TYPE': 'abc',
+            'CONTENT_LENGTH': '10',
+            'X_FOO': 'BAR',
+            'X_FORWARDED_PROTO': 'https',
+            'CONNECTION': 'close',
+        }
+        request.query = 'abc'
+        inst.request = request
+        environ = inst.get_environment()
+        self.assertEqual(environ['wsgi.url_scheme'], 'http')
+
+    def test_get_environment_values_w_scheme_override_trusted(self):
+        import sys
+        inst = self._makeOne()
+        inst.channel.addr = ['192.168.1.1']
+        inst.channel.server.adj.trusted_proxy = '192.168.1.1'
+        request = DummyParser()
+        request.headers = {
+            'CONTENT_TYPE': 'abc',
+            'CONTENT_LENGTH': '10',
+            'X_FOO': 'BAR',
+            'X_FORWARDED_PROTO': 'https',
+            'CONNECTION': 'close',
+        }
+        request.query = 'abc'
+        inst.request = request
+        environ = inst.get_environment()
+
+        # nail the keys of environ
+        self.assertEqual(sorted(environ.keys()), [
+            'CONTENT_LENGTH', 'CONTENT_TYPE', 'HTTP_CONNECTION', 'HTTP_X_FOO',
+            'PATH_INFO', 'QUERY_STRING', 'REMOTE_ADDR', 'REQUEST_METHOD',
+            'SCRIPT_NAME', 'SERVER_NAME', 'SERVER_PORT', 'SERVER_PROTOCOL',
+            'SERVER_SOFTWARE', 'wsgi.errors', 'wsgi.file_wrapper', 'wsgi.input',
+            'wsgi.multiprocess', 'wsgi.multithread', 'wsgi.run_once',
+            'wsgi.url_scheme', 'wsgi.version'])
+
+        self.assertEqual(environ['REQUEST_METHOD'], 'GET')
+        self.assertEqual(environ['SERVER_PORT'], '80')
+        self.assertEqual(environ['SERVER_NAME'], 'localhost')
+        self.assertEqual(environ['SERVER_SOFTWARE'], 'waitress')
+        self.assertEqual(environ['SERVER_PROTOCOL'], 'HTTP/1.0')
+        self.assertEqual(environ['SCRIPT_NAME'], '')
+        self.assertEqual(environ['HTTP_CONNECTION'], 'close')
+        self.assertEqual(environ['PATH_INFO'], '/')
+        self.assertEqual(environ['QUERY_STRING'], 'abc')
+        self.assertEqual(environ['REMOTE_ADDR'], '192.168.1.1')
+        self.assertEqual(environ['CONTENT_TYPE'], 'abc')
+        self.assertEqual(environ['CONTENT_LENGTH'], '10')
+        self.assertEqual(environ['HTTP_X_FOO'], 'BAR')
+        self.assertEqual(environ['wsgi.version'], (1, 0))
+        self.assertEqual(environ['wsgi.url_scheme'], 'https')
+        self.assertEqual(environ['wsgi.errors'], sys.stderr)
+        self.assertEqual(environ['wsgi.multithread'], True)
+        self.assertEqual(environ['wsgi.multiprocess'], False)
+        self.assertEqual(environ['wsgi.run_once'], False)
+        self.assertEqual(environ['wsgi.input'], 'stream')
+        self.assertEqual(inst.environ, environ)
+
+    def test_get_environment_values_w_bogus_scheme_override(self):
+        inst = self._makeOne()
+        inst.channel.addr = ['192.168.1.1']
+        inst.channel.server.adj.trusted_proxy = '192.168.1.1'
+        request = DummyParser()
+        request.headers = {
+            'CONTENT_TYPE': 'abc',
+            'CONTENT_LENGTH': '10',
+            'X_FOO': 'BAR',
+            'X_FORWARDED_PROTO': 'http://p02n3e.com?url=http',
+            'CONNECTION': 'close',
+        }
+        request.query = 'abc'
+        inst.request = request
+        self.assertRaises(ValueError, inst.get_environment)
+
 class TestErrorTask(unittest.TestCase):
 
     def _makeOne(self, channel=None, request=None):
@@ -757,6 +836,7 @@ class DummyAdj(object):
     host = '127.0.0.1'
     port = 80
     url_prefix = ''
+    trusted_proxy = None
 
 class DummyServer(object):
     server_name = 'localhost'