On Thu, Apr 21, 2022 at 01:18:46AM +0300, Maxim Dounin wrote:
> # HG changeset patch
> # User Maxim Dounin <mdounin@mdounin.ru>
> # Date 1650492323 -10800
> # Thu Apr 21 01:05:23 2022 +0300
> # Node ID 50fe52f516ff9c148aa9e7dfcc1c31cc6a4929ae
> # Parent 2ea48b5e4643a818cd81179f040f0b36be9050d6
> Reworked multi headers to use linked lists.
>
> Multi headers are now using linked lists instead of arrays. Notably,
> the following fields were changed: r->headers_in.cookies (renamed
> to r->headers_in.cookie), r->headers_in.x_forwarded_for,
> r->headers_out.cache_control, r->headers_out.link, u->headers_in.cache_control
> u->headers_in.cookies (renamed to u->headers_in.set_cookie).

jftr, this implies updating a bunch of 3rd party modules, including njs, lua,
naxsi, nginx-clojure, headers-more, geoip2_module, ngx_http_sticky_module

>
> The r->headers_in.cookies and u->headers_in.cookies fields were renamed
> to r->headers_in.cookie and u->headers_in.set_cookie to match header names.
>
> The ngx_http_parse_multi_header_lines() and ngx_http_parse_set_cookie_lines()
> functions were changed accordingly.
>
> With this change, multi headers are now essentially equivalent to normal
> headers, and following changes will further make them equivalent.
>
> diff --git a/src/http/modules/ngx_http_geo_module.c b/src/http/modules/ngx_http_geo_module.c
> --- a/src/http/modules/ngx_http_geo_module.c
> +++ b/src/http/modules/ngx_http_geo_module.c
> @@ -327,15 +327,15 @@ static ngx_int_t
> ngx_http_geo_addr(ngx_http_request_t *r, ngx_http_geo_ctx_t *ctx,
> ngx_addr_t *addr)
> {
> - ngx_array_t *xfwd;
> + ngx_table_elt_t *xfwd;
>
> if (ngx_http_geo_real_addr(r, ctx, addr) != NGX_OK) {
> return NGX_ERROR;
> }
>
> - xfwd = &r->headers_in.x_forwarded_for;
> + xfwd = r->headers_in.x_forwarded_for;
>
> - if (xfwd->nelts > 0 && ctx->proxies != NULL) {
> + if (xfwd != NULL && ctx->proxies != NULL) {
> (void) ngx_http_get_forwarded_addr(r, addr, xfwd, NULL,
> ctx->proxies, ctx->proxy_recursive);
> }
> diff --git a/src/http/modules/ngx_http_geoip_module.c b/src/http/modules/ngx_http_geoip_module.c
> --- a/src/http/modules/ngx_http_geoip_module.c
> +++ b/src/http/modules/ngx_http_geoip_module.c
> @@ -240,16 +240,16 @@ static u_long
> ngx_http_geoip_addr(ngx_http_request_t *r, ngx_http_geoip_conf_t *gcf)
> {
> ngx_addr_t addr;
> - ngx_array_t *xfwd;
> + ngx_table_elt_t *xfwd;
> struct sockaddr_in *sin;
>
> addr.sockaddr = r->connection->sockaddr;
> addr.socklen = r->connection->socklen;
> /* addr.name = r->connection->addr_text; */
>
> - xfwd = &r->headers_in.x_forwarded_for;
> + xfwd = r->headers_in.x_forwarded_for;
>
> - if (xfwd->nelts > 0 && gcf->proxies != NULL) {
> + if (xfwd != NULL && gcf->proxies != NULL) {
> (void) ngx_http_get_forwarded_addr(r, &addr, xfwd, NULL,
> gcf->proxies, gcf->proxy_recursive);
> }
> @@ -292,7 +292,7 @@ static geoipv6_t
> ngx_http_geoip_addr_v6(ngx_http_request_t *r, ngx_http_geoip_conf_t *gcf)
> {
> ngx_addr_t addr;
> - ngx_array_t *xfwd;
> + ngx_table_elt_t *xfwd;
> in_addr_t addr4;
> struct in6_addr addr6;
> struct sockaddr_in *sin;
> @@ -302,9 +302,9 @@ ngx_http_geoip_addr_v6(ngx_http_request_
> addr.socklen = r->connection->socklen;
> /* addr.name = r->connection->addr_text; */
>
> - xfwd = &r->headers_in.x_forwarded_for;
> + xfwd = r->headers_in.x_forwarded_for;
>
> - if (xfwd->nelts > 0 && gcf->proxies != NULL) {
> + if (xfwd != NULL && gcf->proxies != NULL) {
> (void) ngx_http_get_forwarded_addr(r, &addr, xfwd, NULL,
> gcf->proxies, gcf->proxy_recursive);
> }
> diff --git a/src/http/modules/ngx_http_headers_filter_module.c b/src/http/modules/ngx_http_headers_filter_module.c
> --- a/src/http/modules/ngx_http_headers_filter_module.c
> +++ b/src/http/modules/ngx_http_headers_filter_module.c
> @@ -329,8 +329,7 @@ ngx_http_set_expires(ngx_http_request_t
> time_t now, expires_time, max_age;
> ngx_str_t value;
> ngx_int_t rc;
> - ngx_uint_t i;
> - ngx_table_elt_t *e, *cc, **ccp;
> + ngx_table_elt_t *e, *cc;
> ngx_http_expires_t expires;
>
> expires = conf->expires;
> @@ -371,38 +370,28 @@ ngx_http_set_expires(ngx_http_request_t
> len = sizeof("Mon, 28 Sep 1970 06:00:00 GMT");
> e->value.len = len - 1;
>
> - ccp = r->headers_out.cache_control.elts;
> -
> - if (ccp == NULL) {
> + cc = r->headers_out.cache_control;
>
> - if (ngx_array_init(&r->headers_out.cache_control, r->pool,
> - 1, sizeof(ngx_table_elt_t *))
> - != NGX_OK)
> - {
> - return NGX_ERROR;
> - }
> + if (cc == NULL) {
>
> cc = ngx_list_push(&r->headers_out.headers);
> if (cc == NULL) {
> return NGX_ERROR;
> }
>
> + r->headers_out.cache_control = cc;
> + cc->next = NULL;
> +
> cc->hash = 1;
> ngx_str_set(&cc->key, "Cache-Control");
>
> - ccp = ngx_array_push(&r->headers_out.cache_control);
> - if (ccp == NULL) {
> - return NGX_ERROR;
> + } else {
> + for (cc = cc->next; cc; cc = cc->next) {
> + cc->hash = 0;
> }
>
> - *ccp = cc;
> -
> - } else {
> - for (i = 1; i < r->headers_out.cache_control.nelts; i++) {
> - ccp[i]->hash = 0;
> - }
> -
> - cc = ccp[0];
> + cc = r->headers_out.cache_control;
> + cc->next = NULL;
> }
>
> if (expires == NGX_HTTP_EXPIRES_EPOCH) {
> @@ -564,22 +553,12 @@ static ngx_int_t
> ngx_http_add_multi_header_lines(ngx_http_request_t *r,
> ngx_http_header_val_t *hv, ngx_str_t *value)
> {
> - ngx_array_t *pa;
> ngx_table_elt_t *h, **ph;
>
> if (value->len == 0) {
> return NGX_OK;
> }
>
> - pa = (ngx_array_t *) ((char *) &r->headers_out + hv->offset);
> -
> - if (pa->elts == NULL) {
> - if (ngx_array_init(pa, r->pool, 1, sizeof(ngx_table_elt_t *)) != NGX_OK)
> - {
> - return NGX_ERROR;
> - }
> - }
> -
> h = ngx_list_push(&r->headers_out.headers);
> if (h == NULL) {
> return NGX_ERROR;
> @@ -589,12 +568,12 @@ ngx_http_add_multi_header_lines(ngx_http
> h->key = hv->key;
> h->value = *value;
>
> - ph = ngx_array_push(pa);
> - if (ph == NULL) {
> - return NGX_ERROR;
> - }
> + ph = (ngx_table_elt_t **) ((char *) &r->headers_out + hv->offset);
> +
> + while (*ph) { ph = &(*ph)->next; }
>
> *ph = h;
> + h->next = NULL;
>
> return NGX_OK;
> }
> diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c
> --- a/src/http/modules/ngx_http_proxy_module.c
> +++ b/src/http/modules/ngx_http_proxy_module.c
> @@ -2559,22 +2559,20 @@ static ngx_int_t
> ngx_http_proxy_add_x_forwarded_for_variable(ngx_http_request_t *r,
> ngx_http_variable_value_t *v, uintptr_t data)
> {
> - size_t len;
> - u_char *p;
> - ngx_uint_t i, n;
> - ngx_table_elt_t **h;
> + size_t len;
> + u_char *p;
> + ngx_table_elt_t *h, *xfwd;
>
> v->valid = 1;
> v->no_cacheable = 0;
> v->not_found = 0;
>
> - n = r->headers_in.x_forwarded_for.nelts;
> - h = r->headers_in.x_forwarded_for.elts;
> + xfwd = r->headers_in.x_forwarded_for;
>
> len = 0;
>
> - for (i = 0; i < n; i++) {
> - len += h[i]->value.len + sizeof(", ") - 1;
> + for (h = xfwd; h; h = h->next) {
> + len += h->value.len + sizeof(", ") - 1;
> }
>
> if (len == 0) {

The only minor point in a whole patch (which I won't insist):
a special case for xfwd absense can be slightly reshuffled now
to avoid a do-nothing loop.

diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -2569,18 +2569,18 @@ ngx_http_proxy_add_x_forwarded_for_varia

xfwd = r->headers_in.x_forwarded_for;

+ if (xfwd == NULL) {
+ v->len = r->connection->addr_text.len;
+ v->data = r->connection->addr_text.data;
+ return NGX_OK;
+ }
+
len = 0;

for (h = xfwd; h; h = h->next) {
len += h->value.len + sizeof(", ") - 1;
}

- if (len == 0) {
- v->len = r->connection->addr_text.len;
- v->data = r->connection->addr_text.data;
- return NGX_OK;
- }
-
len += r->connection->addr_text.len;

p = ngx_pnalloc(r->pool, len);

Otherwise, looks good.

[..]
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org