A segment error occurred in ngx_http_variable_headers_internal

[bullerdu@…]

Nginx is compiled with lua-nginx-module(V0.10.23), and runs ngx_lua case TEST 46: clear all and re-insert(​https://github.com/openresty/lua-nginx-module/blob/v0.10.23rc1/t/028-req-header.t) , then crashes in line if (th->hash == 0) {.

static ngx_int_t
ngx_http_variable_headers_internal(ngx_http_request_t *r,
    ngx_http_variable_value_t *v, uintptr_t data, u_char sep)
{
    size_t            len;
    u_char           *p;
    ngx_table_elt_t  *h, *th;

    h = *(ngx_table_elt_t **) ((char *) r + data);

    len = 0;

    for (th = h; th; th = th->next) {

        if (th->hash == 0) {
            continue;
        }

        len += th->value.len + 2;
    }

    if (len == 0) {
        v->not_found = 1;
        return NGX_OK;
    }

    len -= 2;

    v->valid = 1;
    v->no_cacheable = 0;
    v->not_found = 0;

    if (h->next == NULL) {
        v->len = h->value.len;
        v->data = h->value.data;

        return NGX_OK;
    }

    p = ngx_pnalloc(r->pool, len);
    if (p == NULL) {
        return NGX_ERROR;
    }

    v->len = len;
    v->data = p;

    for (th = h; th; th = th->next) {

        if (th->hash == 0) {
            continue;
        }

        p = ngx_copy(p, th->value.data, th->value.len);

        if (th->next == NULL) {
            break;
        }

        *p++ = sep; *p++ = ' ';
    }

    return NGX_OK;
}

The reason was that nginx modified ngx_table_elt_t data structure in commit(f8f6b9fee66a), and reworked multi headers to use linked lists in commit(ef6a3a99a81a), but lua-nginx-module was not set NULL in ngx_table_elt_t next field.

So, Can ngx_table_elt_t be initialized, Since r->headers_in.headers

alloc in ngx_list_init, or one header alloc in ngx_list_push?

Replace ngx_palloc with ngx_pcalloc, so ngx_table_elt_t next field is NULL, and do not manually set next field in commit(ef6a3a99a81a).

This reduces the probability of forgetting to set the Next field.

static ngx_inline ngx_int_t
ngx_list_init(ngx_list_t *list, ngx_pool_t *pool, ngx_uint_t n, size_t size)
{
    list->part.elts = ngx_palloc(pool, n * size);
    if (list->part.elts == NULL) {
        return NGX_ERROR;
    }

    list->part.nelts = 0;
    list->part.next = NULL;
    list->last = &list->part;
    list->size = size;
    list->nalloc = n;
    list->pool = pool;

    return NGX_OK;
}

void *
ngx_list_push(ngx_list_t *l)
{
    void             *elt;
    ngx_list_part_t  *last;

    last = l->last;

    if (last->nelts == l->nalloc) {

        /* the last part is full, allocate a new list part */

        last = ngx_palloc(l->pool, sizeof(ngx_list_part_t));
        if (last == NULL) {
            return NULL;
        }

        last->elts = ngx_palloc(l->pool, l->nalloc * l->size);
        if (last->elts == NULL) {
            return NULL;
        }

        last->nelts = 0;
        last->next = NULL;

        l->last->next = last;
        l->last = last;
    }

    elt = (char *) last->elts + l->size * last->nelts;
    last->nelts++;

    return elt;
}

[Maxim Dounin]

The 3rd party module in question needs to be modified to correctly work with headers following the change in nginx 1.23.0, as outlined in ​CHANGES:

    *) Change in internal API: now header lines are represented as linked
       lists.

The segmentation fault in question is not the only issue which is expected to happen if the API change in question is not followed.

Further, the r->headers_in.headers list should not be modified at all. It represents the request headers as received from the client, and not expected to be modified after reading the request. The lua module author's informed decision is to ignore this and modify the r->headers_in.headers list anyway, so segmentation faults caused by the module is the expected outcome.

Correctly written modules which are affected by the API change in question are expected to fail during compilation, such as when modifying r->headers_out.cache_control or reading r->headers_in.cookies. The only existing exception are modules which set r->headers_out.www_authenticate, these might not fail during compilation but instead fail at runtime when used in subrequests with auth_request and need to be modified to work properly (see d26db4f82d7d and 8272c823a7d0 for details).

Closing this as duplicate of #2423.