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:2 by dgcgh, 5 years ago

Owner: changed from nobody to dgcgh
Status: newassigned

comment:3 by dgcgh, 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:4 by Florian Apolloner, 5 years ago

LGTM, ses my comment on the PR for a small change.

comment:5 by Mariusz Felisiak, 5 years ago

Component: UncategorizedHTTP handling
Summary: When I am using a reverse proxy on a non-standard port request.get_host() does not include the portHttpRequest.get_host() doesn't include the port from META['HTTP_X_FORWARDED_PORT'].
Triage Stage: UnreviewedAccepted
Version: 3.0master

comment:6 by Mariusz Felisiak, 5 years ago

Patch needs improvement: set

comment:7 by Mariusz Felisiak, 5 years ago

Needs documentation: set

comment:8 by Mariusz Felisiak, 5 years ago

comment:9 by David Smith, 3 years ago

Owner: dgcgh removed
Status: assignednew

comment:10 by LightChain09, 3 years ago

Owner: set to LightChain09
Status: newassigned

comment:11 by LightChain09, 3 years ago

Needs documentation: unset
Owner: LightChain09 removed
Status: assignednew

comment:12 by LightChain09, 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 SREEHARI K.V, 3 years ago

Owner: set to SREEHARI K.V
Status: newassigned

comment:14 by Mariusz Felisiak, 3 years ago

Easy pickings: unset

comment:15 by SREEHARI K.V, 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 Jacob Walls, 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 Mariusz Felisiak, 2 years ago

Owner: SREEHARI K.V removed
Status: assignednew

comment:18 by Mateusz Kurowski, 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 Ülgen Sarıkavak, 9 months ago

Cc: Ülgen Sarıkavak added

comment:20 by Calvin Vu, 3 weeks ago

Cc: Calvin Vu added

comment:21 by Calvin Vu, 3 weeks ago

Owner: set to Calvin Vu
Status: newassigned
Note: See TracTickets for help on using tickets.
Back to Top