Respect X-Forward-* headers and generate correct URLs in url_for (#1465)

* handle forwarded address in request

* added test cases

* Fix lint errors

* Fix uncovered code branch

* Update docstrings

* Update documents

* fix import order

docs/sanic/request_data.md

@@ -145,13 +145,17 @@ The following variables are accessible as properties on `Request` objects:
 
   ```
 - `url`: The full URL of the request, ie: `http://localhost:8000/posts/1/?foo=bar`
-- `scheme`: The URL scheme associated with the request: `http` or `https`
+- `scheme`: The URL scheme associated with the request: 'http|https|ws|wss' or arbitrary value given by the headers.
-- `host`: The host associated with the request: `localhost:8080`
+- `host`: The host associated with the request(which in the `Host` header): `localhost:8080`
+- `server_name`: The hostname of the server, without port number. the value is seeked in this order: `config.SERVER_NAME`, `x-forwarded-host` header, :func:`Request.host`
+- `server_port`: Like `server_name`. Seeked in this order: `x-forwarded-port` header, :func:`Request.host`, actual port used by the transport layer socket.
 - `path`: The path of the request: `/posts/1/`
 - `query_string`: The query string of the request: `foo=bar` or a blank string `''`
 - `uri_template`: Template for matching route handler: `/posts/<id>/`
 - `token`: The value of Authorization header: `Basic YWRtaW46YWRtaW4=`
 
+- `url_for`: Just like `sanic.Sanic.url_for`, but automatically determine `scheme` and `netloc` base on the request. Since this method is aiming to generate correct schema & netloc, `_external` is implied.
+
 
 ## Changing the default parsing rules of the queryset
 

sanic/request.py

@@ -27,7 +27,6 @@ except ImportError:
     else:
         json_loads = json.loads
 
-
 DEFAULT_HTTP_CONTENT_TYPE = "application/octet-stream"
 EXPECT_HEADER = "EXPECT"
 
@@ -329,12 +328,18 @@ class Request(dict):
 
     @property
     def ip(self):
+        """
+        :return: peer ip of the socket
+        """
         if not hasattr(self, "_socket"):
             self._get_address()
         return self._ip
 
     @property
     def port(self):
+        """
+        :return: peer port of the socket
+        """
         if not hasattr(self, "_socket"):
             self._get_address()
         return self._port
@@ -353,6 +358,39 @@ class Request(dict):
         self._ip = self._socket[0]
         self._port = self._socket[1]
 
+    @property
+    def server_name(self):
+        """
+        Attempt to get the server's hostname in this order:
+        `config.SERVER_NAME`, `x-forwarded-host` header, :func:`Request.host`
+
+        :return: the server name without port number
+        :rtype: str
+        """
+        return (
+            self.app.config.get("SERVER_NAME")
+            or self.headers.get("x-forwarded-host")
+            or self.host.split(":")[0]
+        )
+
+    @property
+    def server_port(self):
+        """
+        Attempt to get the server's port in this order:
+        `x-forwarded-port` header, :func:`Request.host`, actual port used by
+        the transport layer socket.
+        :return: server port
+        :rtype: int
+        """
+        forwarded_port = self.headers.get("x-forwarded-port") or (
+            self.host.split(":")[1] if ":" in self.host else None
+        )
+        if forwarded_port:
+            return int(forwarded_port)
+        else:
+            _, port = self.transport.get_extra_info("sockname")
+            return port
+
     @property
     def remote_addr(self):
         """Attempt to return the original client ip based on X-Forwarded-For
@@ -393,6 +431,20 @@ class Request(dict):
 
     @property
     def scheme(self):
+        """
+        Attempt to get the request scheme.
+        Seeking the value in this order:
+        `x-forwarded-proto` header, `x-scheme` header, the sanic app itself.
+
+        :return: http|https|ws|wss or arbitrary value given by the headers.
+        :rtype: str
+        """
+        forwarded_proto = self.headers.get(
+            "x-forwarded-proto"
+        ) or self.headers.get("x-scheme")
+        if forwarded_proto:
+            return forwarded_proto
+
         if (
             self.app.websocket_enabled
             and self.headers.get("upgrade") == "websocket"
@@ -408,8 +460,12 @@ class Request(dict):
 
     @property
     def host(self):
+        """
+        :return: the Host specified in the header, may contains port number.
+        """
         # it appears that httptools doesn't return the host
         # so pull it from the headers
+
         return self.headers.get("Host", "")
 
     @property
@@ -438,6 +494,31 @@ class Request(dict):
             (self.scheme, self.host, self.path, None, self.query_string, None)
         )
 
+    def url_for(self, view_name, **kwargs):
+        """
+        Same as :func:`sanic.Sanic.url_for`, but automatically determine
+        `scheme` and `netloc` base on the request. Since this method is aiming
+        to generate correct schema & netloc, `_external` is implied.
+
+        :param kwargs: takes same parameters as in :func:`sanic.Sanic.url_for`
+        :return: an absolute url to the given view
+        :rtype: str
+        """
+        scheme = self.scheme
+        host = self.server_name
+        port = self.server_port
+
+        if (scheme.lower() in ("http", "ws") and port == 80) or (
+            scheme.lower() in ("https", "wss") and port == 443
+        ):
+            netloc = host
+        else:
+            netloc = "{}:{}".format(host, port)
+
+        return self.app.url_for(
+            view_name, _external=True, _scheme=scheme, _server=netloc, **kwargs
+        )
+
 
 File = namedtuple("File", ["type", "body", "name"])
 

tests/test_requests.py

@@ -629,6 +629,21 @@ async def test_remote_addr_custom_headers_asgi(app):
     assert response.text == "127.0.0.2"
 
 
+def test_forwarded_scheme(app):
+    @app.route("/")
+    async def handler(request):
+        return text(request.remote_addr)
+
+    request, response = app.test_client.get("/")
+    assert request.scheme == 'http'
+
+    request, response = app.test_client.get("/", headers={'X-Forwarded-Proto': 'https'})
+    assert request.scheme == 'https'
+
+    request, response = app.test_client.get("/", headers={'X-Scheme': 'https'})
+    assert request.scheme == 'https'
+
+
 def test_match_info(app):
     @app.route("/api/v1/user/<user_id>/")
     async def handler(request, user_id):
@@ -1656,6 +1671,70 @@ def test_request_socket(app):
     assert hasattr(request, "_socket")
 
 
+def test_request_server_name(app):
+    @app.get("/")
+    def handler(request):
+        return text("OK")
+
+    request, response = app.test_client.get("/")
+    assert request.server_name == '127.0.0.1'
+
+
+def test_request_server_name_in_host_header(app):
+    @app.get("/")
+    def handler(request):
+        return text("OK")
+
+    request, response = app.test_client.get("/", headers={'Host': 'my_server:5555'})
+    assert request.server_name == 'my_server'
+
+
+def test_request_server_name_forwarded(app):
+    @app.get("/")
+    def handler(request):
+        return text("OK")
+
+    request, response = app.test_client.get("/", headers={
+        'Host': 'my_server:5555',
+        'X-Forwarded-Host': 'your_server'
+    })
+    assert request.server_name == 'your_server'
+
+
+def test_request_server_port(app):
+    @app.get("/")
+    def handler(request):
+        return text("OK")
+
+    request, response = app.test_client.get("/", headers={
+        'Host': 'my_server'
+    })
+    assert request.server_port == app.test_client.port
+
+
+def test_request_server_port_in_host_header(app):
+    @app.get("/")
+    def handler(request):
+        return text("OK")
+
+    request, response = app.test_client.get("/", headers={
+        'Host': 'my_server:5555'
+    })
+    assert request.server_port == 5555
+
+
+def test_request_server_port_forwarded(app):
+    @app.get("/")
+    def handler(request):
+        return text("OK")
+
+    request, response = app.test_client.get("/", headers={
+        'Host': 'my_server:5555',
+        'X-Forwarded-Port': '4444'
+    })
+    assert request.server_port == 4444
+
+
 def test_request_form_invalid_content_type(app):
     @app.route("/", methods=["POST"])
     async def post(request):
@@ -1666,6 +1745,38 @@ def test_request_form_invalid_content_type(app):
     assert request.form == {}
 
 
+def test_url_for_with_forwarded_request(app):
+    @app.get("/")
+    def handler(request):
+        return text("OK")
+
+    @app.get("/another_view/")
+    def view_name(request):
+        return text("OK")
+
+    request, response = app.test_client.get("/", headers={
+        'X-Forwarded-Proto': 'https',
+    })
+    assert app.url_for('view_name') == '/another_view'
+    assert app.url_for('view_name', _external=True) == 'http:///another_view'
+    assert request.url_for('view_name') == 'https://127.0.0.1:{}/another_view'.format(app.test_client.port)
+
+    app.config.SERVER_NAME = "my_server"
+    request, response = app.test_client.get("/", headers={
+        'X-Forwarded-Proto': 'https',
+        'X-Forwarded-Port': '6789',
+    })
+    assert app.url_for('view_name') == '/another_view'
+    assert app.url_for('view_name', _external=True) == 'http://my_server/another_view'
+    assert request.url_for('view_name') == 'https://my_server:6789/another_view'
+
+    request, response = app.test_client.get("/", headers={
+        'X-Forwarded-Proto': 'https',
+        'X-Forwarded-Port': '443',
+    })
+    assert request.url_for('view_name') == 'https://my_server/another_view'
+
+    
 @pytest.mark.asyncio
 async def test_request_form_invalid_content_type_asgi(app):
     @app.route("/", methods=["POST"])
@@ -1676,7 +1787,7 @@ async def test_request_form_invalid_content_type_asgi(app):
 
     assert request.form == {}
 
-
+    
 def test_endpoint_basic():
     app = Sanic()