Repository: falconry/falcon
Pull request #1680 (2020-02-20 12:29:40)

Request: use netloc in forwarded_host (#1678)
# Summary of Changes

Use netloc instead of host in forwarded_host, to correctly reconstruct the request URI when the proxy headers are not set.

# Related Issues

#1678

# Pull Request Checklist

This is just a reminder about the most common mistakes.  Please make sure that you tick all *appropriate* boxes.  But please read our [contribution guide](https://github.com/falconry/falcon/blob/master/CONTRIBUTING.md) at least once; it will save you a few review cycles!

If an item doesn't apply to your pull request, **check it anyway** to make it apparent that there's nothing to do.

- [x] Added **tests** for changed code.
- [x] Prefixed code comments with GitHub nick and an appropriate prefix.
- [x] Coding style is consistent with the rest of the framework.
- [ ] Updated **documentation** for changed code.
    - [ ] Added docstrings for any new classes, functions, or modules.
    - [ ] Updated docstrings for any modifications to existing code.
    - [ ] Added references to new classes, functions, or modules to the relevant RST file under `docs/`.
    - [ ] Updated all relevant supporting documentation files under `docs/`.
    - [ ] A copyright notice is included at the top of any new modules (using your own name or the name of your organization).
    - [ ] Changed/added classes/methods/functions have appropriate `versionadded`, `versionchanged`, or `deprecated` [directives](http://www.sphinx-doc.org/en/stable/usage/restructuredtext/directives.html?highlight=versionadded#directive-versionadded).
- [x] Changes (and possible deprecations) have [towncrier](https://pypi.org/project/towncrier/) news fragments under `docs/_newsfragments/`. (Run `towncrier --draft` to ensure it renders correctly.)

If you have *any* questions to *any* of the points above, just **submit and ask**! This checklist is here to *help* you, not to deter you from contributing!

*PR template inspired by the attrs project.*


codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1680?src=pr&el=h1) Report
> Merging [#1680](https://codecov.io/gh/falconry/falcon/pull/1680?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/c3e91875f1030c68d17efa27cf8df3d18f337761?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1680/graphs/tree.svg?width=650&token=zOtkfNQ3h6&height=150&src=pr)](https://codecov.io/gh/falconry/falcon/pull/1680?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master   #1680   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          47      47           
  Lines        3568    3562    -6     
  Branches      545     545           
======================================
- Hits         3568    3562    -6
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1680?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/asgi/request.py](https://codecov.io/gh/falconry/falcon/pull/1680/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVxdWVzdC5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/util/reader.py](https://codecov.io/gh/falconry/falcon/pull/1680/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvcmVhZGVyLnB5) | `100% <0%> (ø)` | :arrow_up: |
| [falcon/media/multipart.py](https://codecov.io/gh/falconry/falcon/pull/1680/diff?src=pr&el=tree#diff-ZmFsY29uL21lZGlhL211bHRpcGFydC5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [falcon/request\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1680/diff?src=pr&el=tree#diff-ZmFsY29uL3JlcXVlc3RfaGVscGVycy5weQ==) | `100% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1680?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1680?src=pr&el=footer). Last update [c3e9187...b86c749](https://codecov.io/gh/falconry/falcon/pull/1680?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

kgriffs: Thanks for working on this! Could you add a towncrier news fragment? You can just name it based on the issue (i.e., `1678.bugfix.rst`).

Patch:
diff --git a/docs/_newsfragments/1678.bugfix.rst b/docs/_newsfragments/1678.bugfix.rst
new file mode 100644
index 0000000..1435ee4
--- /dev/null
+++ b/docs/_newsfragments/1678.bugfix.rst
@@ -0,0 +1,3 @@
+:attr:`falcon.Request.forwarded_host` now contains the port when proxy headers
+are not set, to make it possible to correctly reconstruct the URL when the
+application is not behind a proxy.
diff --git a/falcon/asgi/request.py b/falcon/asgi/request.py
index 4c09325..ddf7787 100644
--- a/falcon/asgi/request.py
+++ b/falcon/asgi/request.py
@@ -599,7 +599,7 @@ class Request(falcon.request.Request):
         # raised exception.
         if 'forwarded' in self._asgi_headers:
             first_hop = self.forwarded[0]
-            host = first_hop.host or self.host
+            host = first_hop.host or self.netloc
         else:
             # PERF(kgriffs): This call should normally succeed, assuming
             # that the caller is expecting a forwarded header, so
@@ -608,7 +608,7 @@ class Request(falcon.request.Request):
             try:
                 host = self._asgi_headers['x-forwarded-host']
             except KeyError:
-                host = self.host
+                host = self.netloc
 
         return host
 
diff --git a/falcon/request.py b/falcon/request.py
index 37a2a5a..827aaaa 100644
--- a/falcon/request.py
+++ b/falcon/request.py
@@ -831,7 +831,7 @@ class Request:
         # raised exception.
         if 'HTTP_FORWARDED' in self.env:
             first_hop = self.forwarded[0]
-            host = first_hop.host or self.host
+            host = first_hop.host or self.netloc
         else:
             # PERF(kgriffs): This call should normally succeed, assuming
             # that the caller is expecting a forwarded header, so
@@ -840,7 +840,7 @@ class Request:
             try:
                 host = self.env['HTTP_X_FORWARDED_HOST']
             except KeyError:
-                host = self.host
+                host = self.netloc
 
         return host
 


Test patch:
diff --git a/tests/test_request_forwarded.py b/tests/test_request_forwarded.py
index 49e82b7..8caeae1 100644
--- a/tests/test_request_forwarded.py
+++ b/tests/test_request_forwarded.py
@@ -17,6 +17,21 @@ def test_no_forwarded_headers(asgi):
     assert req.forwarded_prefix == 'http://example.com/backoffice'
 
 
+def test_no_forwarded_headers_with_port(asgi):
+    req = create_req(
+        asgi,
+        host='example.com',
+        port=8000,
+        path='/languages',
+        root_path='backoffice'
+    )
+
+    assert req.forwarded is None
+    assert req.forwarded_uri == req.uri
+    assert req.forwarded_uri == 'http://example.com:8000/backoffice/languages'
+    assert req.forwarded_prefix == 'http://example.com:8000/backoffice'
+
+
 def test_x_forwarded_host(asgi):
     req = create_req(
         asgi,
@@ -33,6 +48,22 @@ def test_x_forwarded_host(asgi):
     assert req.forwarded_prefix == 'http://something.org'  # Check cached value
 
 
+def test_x_forwarded_host_with_port(asgi):
+    req = create_req(
+        asgi,
+        host='suchproxy.suchtesting.com',
+        path='/languages',
+        headers={'X-Forwarded-Host': 'something.org:8000'}
+    )
+
+    assert req.forwarded is None
+    assert req.forwarded_host == 'something.org:8000'
+    assert req.forwarded_uri != req.uri
+    assert req.forwarded_uri == 'http://something.org:8000/languages'
+    assert req.forwarded_prefix == 'http://something.org:8000'
+    assert req.forwarded_prefix == 'http://something.org:8000'  # Check cached value
+
+
 def test_x_forwarded_proto(asgi):
     req = create_req(
         asgi,