[PATCH 05 of 20] Combining unknown headers during variables lookup (ticket #1316)

Sergey Kandaurov pluknet at nginx.com
Wed May 11 16:09:10 UTC 2022


On Thu, Apr 21, 2022 at 01:18:45AM +0300, Maxim Dounin wrote:
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1650492321 -10800
> #      Thu Apr 21 01:05:21 2022 +0300
> # Node ID 2ea48b5e4643a818cd81179f040f0b36be9050d6
> # Parent  3618f35ac3c8833cfd4a4c80295ab94eb924a9a7
> Combining unknown headers during variables lookup (ticket #1316).
> 
> Previously, $http_*, $sent_http_*, $upstream_http_*, and $upstream_trailer_*
> variables returned only the first header (with a few specially handled
> exceptions: $http_cookie, $http_x_forwarded_for, $sent_http_cache_control,
> $sent_http_link).

forgot to mention "sent_trailer_"

jftr, this is an API change, used at least in mod_zip for for "upstream_http_"

> 
> With this change, all headers are returned, combined together.  For
> example, $http_foo variable will be "a, b" if there are "Foo: a" and
> "Foo: b" headers in the request.
> 
> Note that $upstream_http_set_cookie will also return all "Set-Cookie"
> headers (ticket #1843), though this might not be what one want, since
> the "Set-Cookie" header does not follow the list syntax (see RFC 7230,
> section 3.2.2).
> 
> diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c
> +++ b/src/http/ngx_http_upstream.c
> @@ -5703,7 +5703,7 @@ ngx_http_upstream_header_variable(ngx_ht
>          return NGX_OK;
>      }
>  
> -    return ngx_http_variable_unknown_header(v, (ngx_str_t *) data,
> +    return ngx_http_variable_unknown_header(r, v, (ngx_str_t *) data,
>                                           &r->upstream->headers_in.headers.part,
>                                           sizeof("upstream_http_") - 1);
>  }
> @@ -5718,7 +5718,7 @@ ngx_http_upstream_trailer_variable(ngx_h
>          return NGX_OK;
>      }
>  
> -    return ngx_http_variable_unknown_header(v, (ngx_str_t *) data,
> +    return ngx_http_variable_unknown_header(r, v, (ngx_str_t *) data,
>                                          &r->upstream->headers_in.trailers.part,
>                                          sizeof("upstream_trailer_") - 1);
>  }
> diff --git a/src/http/ngx_http_variables.c b/src/http/ngx_http_variables.c
> --- a/src/http/ngx_http_variables.c
> +++ b/src/http/ngx_http_variables.c
> @@ -919,7 +919,7 @@ static ngx_int_t
>  ngx_http_variable_unknown_header_in(ngx_http_request_t *r,
>      ngx_http_variable_value_t *v, uintptr_t data)
>  {
> -    return ngx_http_variable_unknown_header(v, (ngx_str_t *) data,
> +    return ngx_http_variable_unknown_header(r, v, (ngx_str_t *) data,
>                                              &r->headers_in.headers.part,
>                                              sizeof("http_") - 1);
>  }
> @@ -929,7 +929,7 @@ static ngx_int_t
>  ngx_http_variable_unknown_header_out(ngx_http_request_t *r,
>      ngx_http_variable_value_t *v, uintptr_t data)
>  {
> -    return ngx_http_variable_unknown_header(v, (ngx_str_t *) data,
> +    return ngx_http_variable_unknown_header(r, v, (ngx_str_t *) data,
>                                              &r->headers_out.headers.part,
>                                              sizeof("sent_http_") - 1);
>  }
> @@ -939,19 +939,26 @@ static ngx_int_t
>  ngx_http_variable_unknown_trailer_out(ngx_http_request_t *r,
>      ngx_http_variable_value_t *v, uintptr_t data)
>  {
> -    return ngx_http_variable_unknown_header(v, (ngx_str_t *) data,
> +    return ngx_http_variable_unknown_header(r, v, (ngx_str_t *) data,
>                                              &r->headers_out.trailers.part,
>                                              sizeof("sent_trailer_") - 1);
>  }
>  
>  
>  ngx_int_t
> -ngx_http_variable_unknown_header(ngx_http_variable_value_t *v, ngx_str_t *var,
> +ngx_http_variable_unknown_header(ngx_http_request_t *r,
> +    ngx_http_variable_value_t *v, ngx_str_t *var,
>      ngx_list_part_t *part, size_t prefix)
>  {
> -    u_char            ch;
> +    u_char           *p, ch;
> +    size_t            len;
>      ngx_uint_t        i, n;
> -    ngx_table_elt_t  *header;
> +    ngx_table_elt_t  *header, *h, **ph;
> +
> +    ph = &h;
> +#if (NGX_SUPPRESS_WARN)
> +    len = 0;
> +#endif
>  
>      header = part->elts;
>  
> @@ -971,7 +978,11 @@ ngx_http_variable_unknown_header(ngx_htt
>              continue;
>          }
>  
> -        for (n = 0; n + prefix < var->len && n < header[i].key.len; n++) {
> +        if (header[i].key.len != var->len - prefix) {
> +            continue;
> +        }
> +
> +        for (n = 0; n < var->len - prefix; n++) {
>              ch = header[i].key.data[n];
>  
>              if (ch >= 'A' && ch <= 'Z') {
> @@ -986,18 +997,59 @@ ngx_http_variable_unknown_header(ngx_htt
>              }
>          }
>  
> -        if (n + prefix == var->len && n == header[i].key.len) {
> -            v->len = header[i].value.len;
> -            v->valid = 1;
> -            v->no_cacheable = 0;
> -            v->not_found = 0;
> -            v->data = header[i].value.data;
> -
> -            return NGX_OK;
> +        if (n != var->len - prefix) {
> +            continue;
>          }
> +
> +        len += header[i].value.len + 2;
> +
> +        *ph = &header[i];
> +        ph = &header[i].next;
>      }
>  
> -    v->not_found = 1;
> +    *ph = NULL;
> +
> +    if (h == NULL) {
> +        v->not_found = 1;
> +        return NGX_OK;
> +    }
> +
> +    len -= 2;
> +
> +    if (h->next == NULL) {
> +
> +        v->len = h->value.len;

could be just len for consistency with common case

> +        v->valid = 1;
> +        v->no_cacheable = 0;
> +        v->not_found = 0;
> +        v->data = h->value.data;
> +
> +        return NGX_OK;
> +    }
> +
> +    p = ngx_pnalloc(r->pool, len);
> +    if (p == NULL) {
> +        return NGX_ERROR;
> +    }
> +
> +    v->len = len;
> +    v->valid = 1;
> +    v->no_cacheable = 0;
> +    v->not_found = 0;
> +    v->data = p;
> +
> +    for ( ;; ) {
> +
> +        p = ngx_copy(p, h->value.data, h->value.len);
> +
> +        if (h->next == NULL) {
> +            break;
> +        }
> +
> +        *p++ = ','; *p++ = ' ';

what about Set-Cookie separator ";" support?
looks like can be easly integrated (on top off):

diff --git a/src/http/ngx_http_variables.c b/src/http/ngx_http_variables.c
--- a/src/http/ngx_http_variables.c
+++ b/src/http/ngx_http_variables.c
@@ -950,7 +950,7 @@ ngx_http_variable_unknown_header(ngx_htt
     ngx_http_variable_value_t *v, ngx_str_t *var,
     ngx_list_part_t *part, size_t prefix)
 {
-    u_char           *p, ch;
+    u_char           *p, ch, sep;
     size_t            len;
     ngx_uint_t        i, n;
     ngx_table_elt_t  *header, *h, **ph;
@@ -1018,7 +1018,7 @@ ngx_http_variable_unknown_header(ngx_htt
 
     if (h->next == NULL) {
 
-        v->len = h->value.len;
+        v->len = len;
         v->valid = 1;
         v->no_cacheable = 0;
         v->not_found = 0;
@@ -1038,6 +1038,17 @@ ngx_http_variable_unknown_header(ngx_htt
     v->not_found = 0;
     v->data = p;
 
+    if (h->key.len == sizeof("Set-Cookie") - 1
+        && ngx_strncasecmp(h->key.data, (u_char *) "Set-Cookie",
+                           sizeof("Cookie") - 1)
+           == 0)
+    {
+        sep = ';';
+
+    } else {
+        sep = ',';
+    }
+
     for ( ;; ) {
 
         p = ngx_copy(p, h->value.data, h->value.len);
@@ -1046,7 +1057,7 @@ ngx_http_variable_unknown_header(ngx_htt
             break;
         }
 
-        *p++ = ','; *p++ = ' ';
+        *p++ = sep; *p++ = ' ';
 
         h = h->next;
     }

> +
> +        h = h->next;
> +    }
>  
>      return NGX_OK;
>  }
> @@ -1879,7 +1931,7 @@ ngx_http_variable_sent_location(ngx_http
>  
>      ngx_str_set(&name, "sent_http_location");
>  
> -    return ngx_http_variable_unknown_header(v, &name,
> +    return ngx_http_variable_unknown_header(r, v, &name,
>                                              &r->headers_out.headers.part,
>                                              sizeof("sent_http_") - 1);
>  }
> diff --git a/src/http/ngx_http_variables.h b/src/http/ngx_http_variables.h
> --- a/src/http/ngx_http_variables.h
> +++ b/src/http/ngx_http_variables.h
> @@ -57,8 +57,9 @@ ngx_http_variable_value_t *ngx_http_get_
>  ngx_http_variable_value_t *ngx_http_get_variable(ngx_http_request_t *r,
>      ngx_str_t *name, ngx_uint_t key);
>  
> -ngx_int_t ngx_http_variable_unknown_header(ngx_http_variable_value_t *v,
> -    ngx_str_t *var, ngx_list_part_t *part, size_t prefix);
> +ngx_int_t ngx_http_variable_unknown_header(ngx_http_request_t *r,
> +    ngx_http_variable_value_t *v, ngx_str_t *var, ngx_list_part_t *part,
> +    size_t prefix);
>  
>  
>  #if (NGX_PCRE)
> 



More information about the nginx-devel mailing list