On Thu, Apr 21, 2022 at 01:18:42AM +0300, Maxim Dounin wrote:
> # HG changeset patch
> # User Maxim Dounin <mdounin@mdounin.ru>
> # Date 1650492316 -10800
> # Thu Apr 21 01:05:16 2022 +0300
> # Node ID 61b29233a55216c6fa72e23b93a4a28d76a9fb94
> # Parent e70fb0fdfbc0fb7b7e9f493cc2eb65de617b115a
> FastCGI: combining headers with identical names (ticket #1724).
>
> FastCGI responder is expected to receive CGI/1.1 environment variables
> in the parameters (see section "6.2 Responder" of the FastCGI specification).
> Obviously enough, there cannot be multiple environment variables with
> the same name.
>
> Further, CGI specification (RFC 3875, section "4.1.18. Protocol-Specific
> Meta-Variables") explicitly requires to combine headers: "If multiple
> header fields with the same field-name are received then the server MUST
> rewrite them as a single value having the same semantics".
>
> diff --git a/src/core/ngx_hash.h b/src/core/ngx_hash.h
> --- a/src/core/ngx_hash.h
> +++ b/src/core/ngx_hash.h
> @@ -89,12 +89,15 @@ typedef struct {
> } ngx_hash_keys_arrays_t;
>
>
> -typedef struct {
> +typedef struct ngx_table_elt_s ngx_table_elt_t;
> +
> +struct ngx_table_elt_s {
> ngx_uint_t hash;
> ngx_str_t key;
> ngx_str_t value;
> u_char *lowcase_key;
> -} ngx_table_elt_t;
> + ngx_table_elt_t *next;
> +};
>
>
> void *ngx_hash_find(ngx_hash_t *hash, ngx_uint_t key, u_char *name, size_t len);
> diff --git a/src/http/modules/ngx_http_fastcgi_module.c b/src/http/modules/ngx_http_fastcgi_module.c
> --- a/src/http/modules/ngx_http_fastcgi_module.c
> +++ b/src/http/modules/ngx_http_fastcgi_module.c
> @@ -835,14 +835,14 @@ static ngx_int_t
> ngx_http_fastcgi_create_request(ngx_http_request_t *r)
> {
> off_t file_pos;
> - u_char ch, *pos, *lowcase_key;
> + u_char ch, sep, *pos, *lowcase_key;
> size_t size, len, key_len, val_len, padding,
> allocated;
> ngx_uint_t i, n, next, hash, skip_empty, header_params;
> ngx_buf_t *b;
> ngx_chain_t *cl, *body;
> ngx_list_part_t *part;
> - ngx_table_elt_t *header, **ignored;
> + ngx_table_elt_t *header, *hn, **ignored;
> ngx_http_upstream_t *u;
> ngx_http_script_code_pt code;
> ngx_http_script_engine_t e, le;
> @@ -900,7 +900,11 @@ ngx_http_fastcgi_create_request(ngx_http
> allocated = 0;
> lowcase_key = NULL;
>
> - if (params->number) {
> + if (ngx_http_link_multi_headers(r) != NGX_OK) {
> + return NGX_ERROR;
> + }
> +
> + if (params->number || r->headers_in.multi) {
> n = 0;
> part = &r->headers_in.headers.part;
>
> @@ -930,6 +934,12 @@ ngx_http_fastcgi_create_request(ngx_http
> i = 0;
> }
>
> + for (n = 0; n < header_params; n++) {
> + if (&header[i] == ignored[n]) {
> + goto next_length;
> + }
> + }
> +
> if (params->number) {
> if (allocated < header[i].key.len) {
> allocated = header[i].key.len + 16;
> @@ -959,15 +969,23 @@ ngx_http_fastcgi_create_request(ngx_http
> ignored[header_params++] = &header[i];
> continue;
> }
> -
> - n += sizeof("HTTP_") - 1;
> -
> - } else {
> - n = sizeof("HTTP_") - 1 + header[i].key.len;
> }
>
> - len += ((n > 127) ? 4 : 1) + ((header[i].value.len > 127) ? 4 : 1)
> - + n + header[i].value.len;
> + key_len = sizeof("HTTP_") - 1 + header[i].key.len;
> +
> + val_len = header[i].value.len;
> +
> + for (hn = header[i].next; hn; hn = hn->next) {
> + val_len += hn->value.len + 2;
> + ignored[header_params++] = hn;
> + }
> +
> + len += ((key_len > 127) ? 4 : 1) + key_len
> + + ((val_len > 127) ? 4 : 1) + val_len;
> +
> + next_length:
> +
> + continue;
> }
> }
>
> @@ -1109,7 +1127,7 @@ ngx_http_fastcgi_create_request(ngx_http
>
> for (n = 0; n < header_params; n++) {
> if (&header[i] == ignored[n]) {
> - goto next;
> + goto next_value;
> }
> }
>
> @@ -1125,6 +1143,11 @@ ngx_http_fastcgi_create_request(ngx_http
> }
>
> val_len = header[i].value.len;
> +
> + for (hn = header[i].next; hn; hn = hn->next) {
> + val_len += hn->value.len + 2;
> + }
> +
> if (val_len > 127) {
> *b->last++ = (u_char) (((val_len >> 24) & 0x7f) | 0x80);
> *b->last++ = (u_char) ((val_len >> 16) & 0xff);
> @@ -1150,13 +1173,34 @@ ngx_http_fastcgi_create_request(ngx_http
> *b->last++ = ch;
> }
>
> - b->last = ngx_copy(b->last, header[i].value.data, val_len);
> + b->last = ngx_copy(b->last, header[i].value.data,
> + header[i].value.len);
> +
> + if (header[i].next) {
> +
> + if (header[i].key.len == sizeof("Cookie") - 1
> + && ngx_strncasecmp(header[i].key.data, (u_char *) "Cookie",
> + sizeof("Cookie") - 1)
> + == 0)
> + {
> + sep = ';';
> +
> + } else {
> + sep = ',';
> + }
> +
> + for (hn = header[i].next; hn; hn = hn->next) {
> + *b->last++ = sep;
> + *b->last++ = ' ';
> + b->last = ngx_copy(b->last, hn->value.data, hn->value.len);
> + }
> + }
>
> ngx_log_debug4(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> "fastcgi param: \"%*s: %*s\"",
> key_len, b->last - (key_len + val_len),
> val_len, b->last - val_len);
> - next:
> + next_value:
>
> continue;
> }

Overall, fastcgi/uwsgi/scgi parts look good, some observations below:

- fastcgi_param (still) overrides client headers with the same name
e.g., "fastcgi_param header value;" overrides client header "header: value"

- multiple directives itself aren't linked emitting separate params:
fastcgi_param FOO BAR;
fastcgi_param FOO BAZ;

I don't think it's a major point though, it can be handled
in configuration in principle.

BTW, what about proxy modifications? they aren't required but could be useful

> diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c
> +++ b/src/http/ngx_http_core_module.c
> @@ -2802,6 +2802,78 @@ ngx_http_get_forwarded_addr_internal(ngx
> }
>
>
> +ngx_int_t
> +ngx_http_link_multi_headers(ngx_http_request_t *r)
> +{
> + ngx_uint_t i, j;
> + ngx_list_part_t *part, *ppart;
> + ngx_table_elt_t *header, *pheader, **ph;
> +
> + if (r->headers_in.multi_linked) {
> + return NGX_OK;
> + }

multi_linked is never set, was the intension to avoid doing the work
twice on repetitive invocation? it doesn't seem to be possible.
I'd just axe it.

> +
> + part = &r->headers_in.headers.part;
> + header = part->elts;
> +
> + for (i = 0; /* void */; i++) {
> +
> + if (i >= part->nelts) {
> + if (part->next == NULL) {
> + break;
> + }
> +
> + part = part->next;
> + header = part->elts;
> + i = 0;
> + }
> +
> + header[i].next = NULL;
> +
> + /*
> + * search for previous headers with the same name;
> + * if there are any, link to them
> + */
> +
> + ppart = &r->headers_in.headers.part;
> + pheader = part->elts;

pheader = ppart->elts;

> +
> + for (j = 0; /* void */; j++) {
> +
> + if (j >= ppart->nelts) {
> + if (ppart->next == NULL) {
> + break;
> + }
> +
> + ppart = ppart->next;
> + pheader = ppart->elts;
> + i = 0;

j = 0;

> + }
> +
> + if (part == ppart && i == j) {
> + break;
> + }
> +
> + if (header[i].key.len == pheader[j].key.len
> + && ngx_strncasecmp(header[i].key.data, pheader[j].key.data,
> + header[i].key.len)
> + == 0)
> + {
> + ph = &pheader[j].next;
> + while (*ph) { ph = &(*ph)->next; }
> + *ph = &header[i];
> +
> + r->headers_in.multi = 1;
> +
> + break;
> + }
> + }
> + }
> +
> + return NGX_OK;
> +}
> +
> +
> static char *
> ngx_http_core_server(ngx_conf_t *cf, ngx_command_t *cmd, void *dummy)
> {
> diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h
> --- a/src/http/ngx_http_core_module.h
> +++ b/src/http/ngx_http_core_module.h
> @@ -532,6 +532,8 @@ ngx_int_t ngx_http_get_forwarded_addr(ng
> ngx_array_t *headers, ngx_str_t *value, ngx_array_t *proxies,
> int recursive);
>
> +ngx_int_t ngx_http_link_multi_headers(ngx_http_request_t *r);
> +
>
> extern ngx_module_t ngx_http_core_module;
>
> diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
> --- a/src/http/ngx_http_request.h
> +++ b/src/http/ngx_http_request.h
> @@ -242,6 +242,8 @@ typedef struct {
>
> unsigned connection_type:2;
> unsigned chunked:1;
> + unsigned multi:1;
> + unsigned multi_linked:1;
> unsigned msie:1;
> unsigned msie6:1;
> unsigned opera:1;
>
> _______________________________________________
> nginx-devel mailing list -- nginx-devel@nginx.org
> To unsubscribe send an email to nginx-devel-leave@nginx.org
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org