Fix redirect to http://_/ #8
Conversation
|
Can one of the admins verify this patch? |
|
I've sent patch to Jiří Konečný @jkonecny |
| @@ -6,6 +6,6 @@ server { | |||
| {%- if site.redirect is defined %} | |||
| rewrite ^(.*) {{ site.redirect.get('protocol', 'http') }}://{{ site.redirect.get('host', '$server_name') }}{{ site.redirect.get('path', '$1') }} permanent; | |||
| {%- else %} | |||
| rewrite ^(.*) https://$server_name$1 permanent; | |||
| rewrite ^(.*) https://$http_host$1 permanent; | |||
There was a problem hiding this comment.
$http_host can be undefined (HTTP/1.0) or defined with port, for example - the redirect from http://<...>:8080 to https://<...>:8080 would not work.
The underscore is a catch all $server_name, maybe it should be avoided or we could use $host variable to at least somewhat mitigate the issue.
There was a problem hiding this comment.
Yep, $host is better, but not $servername.
If site is empty, or redirect is not specified, it should use $http_host
|
I'd handle this situation only in case the site.host is set '_' in all other cases it behaves as expected. |
|
@cznewt ok. BTW, do you have any tests? |
|
@cznewt What about a situation when the $server_name is not set at all or "", e.g. ip based virtual hosts? I think $host is a good solution and it has fallback to server_name incorporated. The only worry I have in this case is the HTTP/1.0 + default virtual host without server_name defined, but that should be very uncommon. |
|
$host will fallback to $server_name in case of HTTP/1.0, but this scenario is VERY uncommon for horizon. |
If site is empty, or redirect is not specified, it should use $http_host