GeoNode / geonode-docker

Django base images for GeoNode
Other
9 stars 33 forks source link

set nginx host with custom port #32

Open nicokant opened 7 months ago

nicokant commented 7 months ago

Nginx configuration uses $host variable that doesn't provide custom ports, causing errors with request.build_absolute_url. This PR aims to fix providing also non-standard port specified as HTTP_PORT/HTTPS_PORT.

Related issues:

t-book commented 3 months ago

@giohappy good from my side.

ridoo commented 3 months ago

This issue seems to be wide spread and there are different solutions:

Both would work, but I tend to pick the $http_host solution as it does not introduce yet another URL variable in GeoNode settings. @ricardogsilva created a PR to close #11734 linked by @nicokant . I tested successfully to use $http_host already as I had to use a non-default HTTPS port in one of those blueprint setups.

ricardogsilva commented 3 months ago

I'd also prefer the changes proposed in GeoNode/geonode#11736, (regardless of being the author of that PR 😜) as they seem adequate for fixing the issue and are just a small modification of a single line in a config file.

WRT this PR, I would rather prefer the number of configuration variables for GeoNode to decrease, so adding yet another one is not the way, in my opinion.

giohappy commented 3 months ago

I haven't the opportunity to test the $http_host approach now, but I also prefee it over additional variables.

If a custom host is needed it generally means that we are forwarding from anothe gateway / reverse proxy. In that case I guess the expected result is to have the oroginal http header forwarded automatically as is, without having to configure it.

The only concern is that $http_host assumes different values depending on whether the host header is set or not., In that case it uses the host in the request line, or the server_name as a fallback. Maybe these are the cases where forcing its configuration might be required?

ridoo commented 3 months ago

If a custom host is needed it generally means that we are forwarding from anothe gateway / reverse proxy. In that case I guess the expected result is to have the oroginal http header forwarded automatically as is, without having to configure it.

@giohappy maybe this could be done by just setting proxy_set_header Host $http_host. Did not test it, though.

The only concern is that $http_host assumes different values depending on whether the host header is set or not., In that case it uses the host in the request line, or the server_name as a fallback. Maybe these are the cases where forcing its configuration might be required?

MDN documentation on Host request header says this:

A Host header field must be sent in all HTTP/1.1 request messages. A 400 (Bad Request) status code may be sent to any HTTP/1.1 request message that lacks or contains more than one Host header field.

Do you know cases other than HTTP 1.0 where Host header can be absent or empty? Do we want to support HTTP 1.0 requests (in cases where non-default HTTPS port is configured)? Then we would have to introduce another variable. IMO, however, that is be a very special case, for which I still would vote for $http_host.

giohappy commented 3 months ago

@giohappy maybe this could be done by just setting proxy_set_header Host $http_host. Did not test it, though.

My comment was to agree with the proposal from @ricardogsilva, but it needs testing.

Ok for the required host header. Nginx is designed to cover the edge cases, but I think we can assume the more usual case.