Re: X-Accel-Redirect и uri escape

[Maxim Dounin]

Hello!

On Mon, Nov 25, 2013 at 07:38:08PM +0300, Rommer wrote:

> Hello,
> 
> Насколько я вижу, все предыдущие попытки ни к чему конструктивному
> так и не привели.
> 
> Поэтому предлагаю новую опцию safe_redirect on/off.
> Если установлена в "on" в http, server или location, откуда
> идет proxy_pass, путь в X-Accel-Redirect воспринимает
> как escaped uri. По дефолту стоит в off и поведение
> парсера не меняет.

Я не возражаю против того, чтобы поведение парсера таки поменять 
без всяких опций (более того, я против того, чтобы вводить 
подобные опции - проще один раз сделать правильно).

Проблема в том, что никто так и не сделал приличный патч, 
консистентно меняющий поведение парсера.

[...]

> diff -Nru a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c	2013-11-19 15:25:25.000000000 +0400
> +++ b/src/http/ngx_http_core_module.c	2013-11-25 18:49:09.253001176 +0400
> @@ -746,6 +746,13 @@
>        offsetof(ngx_http_core_loc_conf_t, resolver_timeout),
>        NULL },
>  
> +    { ngx_string("safe_redirect"),
> +      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
> +      ngx_conf_set_flag_slot,
> +      NGX_HTTP_LOC_CONF_OFFSET,
> +      offsetof(ngx_http_core_loc_conf_t, safe_redirect),
> +      NULL },

Совершенно непонятно, зачем вы решили сделать опцию в 
ngx_http_core_module...

[...]

> --- a/src/http/ngx_http_upstream.c	2013-11-19 15:25:25.000000000 +0400
> +++ b/src/http/ngx_http_upstream.c	2013-11-25 20:19:44.019000094 +0400
> @@ -1893,14 +1893,18 @@
>  static ngx_int_t
>  ngx_http_upstream_process_headers(ngx_http_request_t *r, ngx_http_upstream_t *u)
>  {
> +    u_char                         *dst, *src;
> +    size_t                          len;
>      ngx_str_t                      *uri, args;
>      ngx_uint_t                      i, flags;
>      ngx_list_part_t                *part;
>      ngx_table_elt_t                *h;
>      ngx_http_upstream_header_t     *hh;
>      ngx_http_upstream_main_conf_t  *umcf;
> +    ngx_http_core_loc_conf_t       *clcf;
>  
>      umcf = ngx_http_get_module_main_conf(r, ngx_http_upstream_module);
> +    clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
>  
>      if (u->headers_in.x_accel_redirect
>          && !(u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_XA_REDIRECT))
> @@ -1938,9 +1942,27 @@
>          ngx_str_null(&args);
>          flags = NGX_HTTP_LOG_UNSAFE;
>  
> -        if (ngx_http_parse_unsafe_uri(r, uri, &args, &flags) != NGX_OK) {
> -            ngx_http_finalize_request(r, NGX_HTTP_NOT_FOUND);
> -            return NGX_DONE;
> +        if (clcf->safe_redirect) {

...и при этом используете её только в upstream'е.

> +
> +            dst = uri->data;
> +            src = uri->data;
> +
> +            ngx_unescape_uri(&dst, &src, uri->len, NGX_UNESCAPE_URI);
> +
> +            len = (uri->data + uri->len) - src;
> +            if (len) {
> +                dst = ngx_movemem(dst, src, len);
> +            }
> +
> +            uri->len = dst - uri->data;
> +
> +        } else {
> +
> +            if (ngx_http_parse_unsafe_uri(r, uri, &args, &flags) != NGX_OK) {
> +                ngx_http_finalize_request(r, NGX_HTTP_NOT_FOUND);
> +                return NGX_DONE;
> +            }
> +

Так совсем неправильно: аргументы в "X-Accel-Redirect" 
обрабатываются только в том случае, если флаг clcf->safe_redirect 
не установлен.  Не говоря уже про unsafe-проверки, которые не 
делаются.

И даже если сделать как в SSI, то проблема "?" в пути не решается.

-- 
Maxim Dounin
http://nginx.org/en/donation.html