Re: X-Accel-Redirect и uri escape

[Роман Шишнев]

Hello,

Тогда всё проще и вот так.

P.S. зачем flags в ngx_http_parse_unsafe_uri()
передается по ссылке?

On 11/25/2013 08:21 PM, Maxim Dounin wrote:
> 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, то проблема "?" в пути не решается.
>

-- 
С уважением,
Роман Шишнёв,
CTO | ActiveCloud | http://www.active.by
Т +375 17 2 911 511 доб. 308 | rommer at active.by
Облачные решения | Серверы и инфраструктура | IaaS | SaaS | Хостинг
-------------- next part --------------
diff -Nru a/src/http/ngx_http_parse.c c/src/http/ngx_http_parse.c
--- a/src/http/ngx_http_parse.c	2013-11-19 15:25:25.000000000 +0400
+++ c/src/http/ngx_http_parse.c	2013-11-26 02:24:12.897000136 +0400
@@ -1799,7 +1799,7 @@
             continue;
         }
 
-        if (ch == '?') {
+        if (ch == '?' && args != NULL) {
             args->len = len - 1;
             args->data = p;
             uri->len -= len;
diff -Nru a/src/http/ngx_http_upstream.c c/src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c	2013-11-19 15:25:25.000000000 +0400
+++ c/src/http/ngx_http_upstream.c	2013-11-26 02:30:36.354000390 +0400
@@ -1893,6 +1893,8 @@
 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;
@@ -1943,6 +1945,23 @@
             return NGX_DONE;
         }
 
+        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;
+
+        if (ngx_http_parse_unsafe_uri(r, uri, NULL, &flags) != NGX_OK) {
+            ngx_http_finalize_request(r, NGX_HTTP_NOT_FOUND);
+            return NGX_DONE;
+        }
+
         if (r->method != NGX_HTTP_HEAD) {
             r->method = NGX_HTTP_GET;
         }