Welcome! Log In Create A New Profile

Advanced

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

Maxim Dounin
November 03, 2022 01:08PM
Hello!

On Thu, Nov 03, 2022 at 04:25:52AM +0000, Ciel via nginx-devel wrote:

> # HG changeset patch
> # User Ciel Zhao <i@ciel.dev>
> # Date 1667411069 -28800
> # Thu Nov 03 01:44:29 2022 +0800
> # Node ID 85141e004b5af89a9d443bc0084a34291193567a
> # Parent 1ae25660c0c76edef14121ca64362f28b9d57a70
> SSI: ensure context of main request exists for subrequest using SSI
>
> 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 checks the context of the main request after initializing the context
> for subrequests, and creates one if not exists.

Thanks for the patch.

It looks like an attempt to fix ticket #1263
(https://trac.nginx.org/nginx/ticket/1263). I've linked this
thread to the ticket. It might be a good idea to add a reference
into commit log.

See below for some comments about the code.

>
> diff -r 1ae25660c0c7 -r 85141e004b5a src/http/modules/ngx_http_ssi_filter_module.c
> --- a/src/http/modules/ngx_http_ssi_filter_module.c Wed Oct 19 10:56:21 2022 +0300
> +++ b/src/http/modules/ngx_http_ssi_filter_module.c Thu Nov 03 01:44:29 2022 +0800
> @@ -329,7 +329,7 @@
> 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);
> @@ -348,6 +348,16 @@
>
> ngx_http_set_ctx(r, ctx, ngx_http_ssi_filter_module);
>
> + mctx = ngx_http_get_module_ctx(r->main, ngx_http_ssi_filter_module);
> + if (mctx == NULL && r != r->main) {
> + mctx = ngx_pcalloc(r->main->pool, sizeof(ngx_http_ssi_ctx_t));
> + if (mctx == NULL) {
> + return NGX_ERROR;
> + }
> +
> + ngx_http_set_ctx(r->main, mctx, ngx_http_ssi_filter_module);
> + }
> +
>
> ctx->value_len = slcf->value_len;
> ctx->last_out = &ctx->out;

In many cases the main request context is not needed. It might be
a good idea to create it only when needed, and avoid doing so if
it isn't.

Further, in theory, SSI processing of a (in-memory/background)
subrequest might happen even before main request is actually seen
by the SSI module, so the actual context will be created later.
This probably needs to be taken into account.

> @@ -403,8 +413,12 @@
> ngx_str_t *params[NGX_HTTP_SSI_MAX_PARAMS + 1];
>
> ctx = ngx_http_get_module_ctx(r, ngx_http_ssi_filter_module);
> + slcf = ngx_http_get_module_loc_conf(r, ngx_http_ssi_filter_module);
>
> if (ctx == NULL
> + || !slcf->enable
> + || r->headers_out.content_length_n == 0
> + || ngx_http_test_content_type(r, &slcf->types) == NULL
> || (in == NULL
> && ctx->buf == NULL
> && ctx->in == NULL

This does not look like a good solution to filter out processing
of a main request if SSI is not enabled for it. Rather, there
should be an explicit flag, so it would be possible to avoid
evaluation of complex constructs on each body filter call - there
can be a lot of such calls.

> @@ -450,8 +464,6 @@
> }
> }
>
> - slcf = ngx_http_get_module_loc_conf(r, ngx_http_ssi_filter_module);
> -
> while (ctx->in || ctx->buf) {
>
> if (ctx->buf == NULL) {
>

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

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

Maxim Dounin 338 November 03, 2022 01:08PM

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

Maxim Dounin 29 November 08, 2022 01:52AM

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

Maxim Dounin 23 November 14, 2022 10:00PM

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

Sergey Kandaurov 12 November 21, 2022 04:30AM

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

Maxim Dounin 17 November 21, 2022 09:06AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 133
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready