[PATCH] SSI: ensure context of main request exists for subrequest using SSI

[Sergey Kandaurov]

> On 15 Nov 2022, at 06:59, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> Hello!
> 
> [..]
> Below is slightly cleaned up version of this patch, please take a 
> look:
> 
> # HG changeset patch
> # User Ciel Zhao <i at ciel.dev>
> # Date 1667928380 -28800
> #      Wed Nov 09 01:26:20 2022 +0800
> # Node ID 41c67de61c7a916c01f0a1f5054d11821991ffce
> # Parent  42bc158a47ecb3c2bd0396c723c307c757f2770e
> SSI: handling of subrequests from other modules (ticket #1263).
> 
> As the SSI parser always uses the context from the main request for storing
> variables and blocks, that context should always exist for subrequests using
> SSI, even though the main request does not necessarily have SSI enabled.
> 
> However, `ngx_http_get_module_ctx(r->main, ...)` is getting NULL in such cases,
> resulting in the worker crashing SIGSEGV when accessing its attributes.
> 
> This patch links the first initialized context to the main request, and
> upgrades it only when main context is initialized.
> 
> diff --git a/src/http/modules/ngx_http_ssi_filter_module.c b/src/http/modules/ngx_http_ssi_filter_module.c
> --- a/src/http/modules/ngx_http_ssi_filter_module.c
> +++ b/src/http/modules/ngx_http_ssi_filter_module.c
> @@ -329,7 +329,7 @@ static ngx_http_variable_t  ngx_http_ssi
> static ngx_int_t
> ngx_http_ssi_header_filter(ngx_http_request_t *r)
> {
> -    ngx_http_ssi_ctx_t       *ctx;
> +    ngx_http_ssi_ctx_t       *ctx, *mctx;
>     ngx_http_ssi_loc_conf_t  *slcf;
> 
>     slcf = ngx_http_get_module_loc_conf(r, ngx_http_ssi_filter_module);
> @@ -341,6 +341,8 @@ ngx_http_ssi_header_filter(ngx_http_requ
>         return ngx_http_next_header_filter(r);
>     }
> 
> +    mctx = ngx_http_get_module_ctx(r->main, ngx_http_ssi_filter_module);
> +
>     ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_ssi_ctx_t));
>     if (ctx == NULL) {
>         return NGX_ERROR;
> @@ -367,6 +369,20 @@ ngx_http_ssi_header_filter(ngx_http_requ
>     r->filter_need_in_memory = 1;
> 
>     if (r == r->main) {
> +
> +        if (mctx) {
> +
> +            /*
> +             * if there was a shared context previously used as main,
> +             * copy variables and blocks
> +             */
> +
> +            ctx->variables = mctx->variables;
> +            ctx->blocks = mctx->blocks;
> +
> +            mctx->shared = 0;
> +        }
> +
>         ngx_http_clear_content_length(r);
>         ngx_http_clear_accept_ranges(r);
> 
> @@ -379,6 +395,10 @@ ngx_http_ssi_header_filter(ngx_http_requ
>         } else {
>             ngx_http_weak_etag(r);
>         }
> +
> +    } else if (mctx == NULL) {
> +        ngx_http_set_ctx(r->main, ctx, ngx_http_ssi_filter_module);
> +        ctx->shared = 1;
>     }
> 
>     return ngx_http_next_header_filter(r);
> @@ -405,6 +425,7 @@ ngx_http_ssi_body_filter(ngx_http_reques
>     ctx = ngx_http_get_module_ctx(r, ngx_http_ssi_filter_module);
> 
>     if (ctx == NULL
> +        || (ctx->shared && r == r->main)
>         || (in == NULL
>             && ctx->buf == NULL
>             && ctx->in == NULL
> diff --git a/src/http/modules/ngx_http_ssi_filter_module.h b/src/http/modules/ngx_http_ssi_filter_module.h
> --- a/src/http/modules/ngx_http_ssi_filter_module.h
> +++ b/src/http/modules/ngx_http_ssi_filter_module.h
> @@ -71,6 +71,7 @@ typedef struct {
>     u_char                   *captures_data;
> #endif
> 
> +    unsigned                  shared:1;
>     unsigned                  conditional:2;
>     unsigned                  encoding:2;
>     unsigned                  block:1;
> 

The patch doesn't cover regex positional captures in the "if" command.
They are also used to be stored in the main request in ctx->*captures*.
Upgrading main context makes them unavailable, so the fields need to be
copied under mctx condition as well if we want to care about that.

Otherwise, looks good.

-- 
Sergey Kandaurov