Opened 5 years ago
Last modified 3 weeks ago
#31354 assigned Bug
HttpRequest.get_host() doesn't include the port from META['HTTP_X_FORWARDED_PORT'].
Reported by: | dgcgh | Owned by: | Calvin Vu |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | request get_host build_absolute_uri |
Cc: | Ülgen Sarıkavak, Calvin Vu | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
request.get_port() should include the port of the authority if it is not 80 for http or 443 for https. Currently this does not happen when HTTP_X_FORWARDED_HOST is present in request.META.
This causes issues for e.g. request.build_absolute_uri() if the django app is running behind a reverse proxy on a non-standard port.
Example would be: nginx is listening on port 8443 and forwarding to a django server listening on port 8001:
server { listen 8443 ssl; server_name localhost.example.com location / { proxy_pass http://localhost:8001; # include /etc/nginx/uwsgi_params; proxy_cache_bypass $http_upgrade; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection "upgrade"; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header X-Forwarded-Host $host; proxy_set_header X-Forwarded-Port $server_port; } }
In this case URIs built with request.build_absolute_uri() in a django app will exclude the port, i.e.:
request.build_django_uri("/") == "https://localhost.example.com/"
where it should return "https://localhost.example.com:8443/"
Change History (21)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 5 years ago
I think the problem here might be that django assumes X-Forwarded-Host will include the port number, as opposed to putting the port number in X-Forwarded-Port.
I can get the correct behavior by setting the nginx header as:
proxy_set_header X-Forwarded-Host $host:$server_port;
and disabling USE_X_FORWARDED_PORT
.
I take it that X-Forwarded-Port
is not popular, so maybe this bug is not worth fixing, but I will update my PR with a more explicit solution.
comment:5 by , 5 years ago
Component: | Uncategorized → HTTP handling |
---|---|
Summary: | When I am using a reverse proxy on a non-standard port request.get_host() does not include the port → HttpRequest.get_host() doesn't include the port from META['HTTP_X_FORWARDED_PORT']. |
Triage Stage: | Unreviewed → Accepted |
Version: | 3.0 → master |
comment:6 by , 5 years ago
Patch needs improvement: | set |
---|
comment:7 by , 5 years ago
Needs documentation: | set |
---|
comment:9 by , 3 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:10 by , 3 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:11 by , 3 years ago
Needs documentation: | unset |
---|---|
Owner: | removed |
Status: | assigned → new |
comment:12 by , 3 years ago
As far as I can see, there already is documentation about the patch, so I changed the "Needs documentation" tag.
comment:13 by , 3 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:14 by , 3 years ago
Easy pickings: | unset |
---|
comment:15 by , 3 years ago
I'd like to work on this bug.
I notice that https://github.com/django/django/pull/12844 has been closed due to inactivity.
Can I pick up where Apollo13 left off or start from scratch?
comment:16 by , 3 years ago
Ordinarily yes, you can start from a previous attempt and credit the prior author as a co-author in the commit message.
comment:17 by , 2 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:18 by , 21 months ago
Hello, i seen multiple pull requests that have their origin in this issue - when it comes to host and port with proxy protocol - does it need some big rewrite in the django source?
comment:19 by , 9 months ago
Cc: | added |
---|
comment:20 by , 3 weeks ago
Cc: | added |
---|
comment:21 by , 3 weeks ago
Owner: | set to |
---|---|
Status: | new → assigned |
PR: https://github.com/django/django/pull/12550