> On 15 Nov 2022, at 06:59, Maxim Dounin <mdounin@mdounin.ru> wrote:
>
> Hello!
>
> [..]
> Below is slightly cleaned up version of this patch, please take a
> look:
>
> # HG changeset patch
> # User Ciel Zhao <i@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

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