[PATCH] SSI: ensure context of main request exists for subrequest using SSI
Ciel
i at ciel.dev
Tue Nov 8 17:31:25 UTC 2022
Hello! Thanks for your detailed explanation, really helps a lot!
> Another option might be to link the first subrequest's context as
> the main one - till the main request context is created (if at
> all). Especially given that we anyway have to upgrade the main
> request context if the main request is seen after the first
> subrequest. This will imply an additional check in the body
> filter along with the flag, but a trivial one.
Yes, you're right, this is a valid *option C* and I have implemented that.
This can truly save bytes, if someone have SSI only on main request disabled,
but use templates without variables or blocks.
However, due to the context stealing, we should mark whether the context is for
main/subrequests, and distinguish that in body filter. Honestly, it's kind of
counter-intuitive for me to share one context and use (type, flag) tuple to
check for validity.
> This makes it possible to add "sequence points" to SSI, resolving
> undefined behaviour due to parallel execution of requests.
But what if the parallel subrequests do not share a common SSI parent, i.e.
introduced concurrently by other modules? The `wait` parameter seems have dealt
with intra-module concurrency, but not inter-module ones. If that truly is a
problem, I'm not going to cover this case in this patch. So answer at your
interest or leave it alone.
I've implemented two patch here, the first is the just-create way fixed, and the
second is the context-stealing way. Up to you to choose, or let me know for
other problems.
********************************************************************************
Patch A:
# HG changeset patch
# User Ciel Zhao <i at ciel.dev>
# Date 1667927876 -28800
# Wed Nov 09 01:17:56 2022 +0800
# Node ID f7046e9deabef8c1d3caa4809a4ed5f93c17cf99
# Parent 17d6a537fb1bb587e4de22961bf5be5f0c648fa8
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.
diff -r 17d6a537fb1b -r f7046e9deabe src/http/modules/ngx_http_ssi_filter_module.c
--- a/src/http/modules/ngx_http_ssi_filter_module.c Wed Nov 02 13:46:16 2022 +0400
+++ b/src/http/modules/ngx_http_ssi_filter_module.c Wed Nov 09 01:17:56 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);
@@ -341,14 +341,26 @@
return ngx_http_next_header_filter(r);
}
- ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_ssi_ctx_t));
+ ctx = ngx_http_get_module_ctx(r, ngx_http_ssi_filter_module);
if (ctx == NULL) {
- return NGX_ERROR;
+ ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_ssi_ctx_t));
+ if (ctx == NULL) {
+ return NGX_ERROR;
+ }
+ ngx_http_set_ctx(r, ctx, ngx_http_ssi_filter_module);
}
- 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) {
+ 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->enabled = 1;
ctx->value_len = slcf->value_len;
ctx->last_out = &ctx->out;
@@ -405,6 +417,7 @@
ctx = ngx_http_get_module_ctx(r, ngx_http_ssi_filter_module);
if (ctx == NULL
+ || !ctx->enabled
|| (in == NULL
&& ctx->buf == NULL
&& ctx->in == NULL
diff -r 17d6a537fb1b -r f7046e9deabe src/http/modules/ngx_http_ssi_filter_module.h
--- a/src/http/modules/ngx_http_ssi_filter_module.h Wed Nov 02 13:46:16 2022 +0400
+++ b/src/http/modules/ngx_http_ssi_filter_module.h Wed Nov 09 01:17:56 2022 +0800
@@ -71,6 +71,7 @@
u_char *captures_data;
#endif
+ unsigned enabled:1;
unsigned conditional:2;
unsigned encoding:2;
unsigned block:1;
********************************************************************************
Patch C:
# HG changeset patch
# User Ciel Zhao <i at ciel.dev>
# Date 1667928380 -28800
# Wed Nov 09 01:26:20 2022 +0800
# Node ID 4b6f88048a6104478709b5bd9a6cc6c0c343b36c
# Parent 17d6a537fb1bb587e4de22961bf5be5f0c648fa8
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 links the first initialized context to the main request, and upgrades
it only when main context is initialized.
diff -r 17d6a537fb1b -r 4b6f88048a61 src/http/modules/ngx_http_ssi_filter_module.c
--- a/src/http/modules/ngx_http_ssi_filter_module.c Wed Nov 02 13:46:16 2022 +0400
+++ b/src/http/modules/ngx_http_ssi_filter_module.c Wed Nov 09 01:26:20 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, *sctx;
ngx_http_ssi_loc_conf_t *slcf;
slcf = ngx_http_get_module_loc_conf(r, ngx_http_ssi_filter_module);
@@ -346,9 +346,21 @@
return NGX_ERROR;
}
+ sctx = ngx_http_get_module_ctx(r, ngx_http_ssi_filter_module);
+ if (sctx != NULL) {
+ // migrate to main context if needed, see below
+ ctx->blocks = sctx->blocks;
+ ctx->variables = sctx->variables;
+ }
+
ngx_http_set_ctx(r, ctx, ngx_http_ssi_filter_module);
-
-
+ if (ngx_http_get_module_ctx(r->main, ngx_http_ssi_filter_module) == NULL) {
+ // set main context to current context if not exists
+ ngx_http_set_ctx(r->main, ctx, ngx_http_ssi_filter_module);
+ }
+
+
+ ctx->is_main = (r == r->main);
ctx->value_len = slcf->value_len;
ctx->last_out = &ctx->out;
@@ -405,6 +417,7 @@
ctx = ngx_http_get_module_ctx(r, ngx_http_ssi_filter_module);
if (ctx == NULL
+ || (!ctx->is_main && r == r->main)
|| (in == NULL
&& ctx->buf == NULL
&& ctx->in == NULL
diff -r 17d6a537fb1b -r 4b6f88048a61 src/http/modules/ngx_http_ssi_filter_module.h
--- a/src/http/modules/ngx_http_ssi_filter_module.h Wed Nov 02 13:46:16 2022 +0400
+++ b/src/http/modules/ngx_http_ssi_filter_module.h Wed Nov 09 01:26:20 2022 +0800
@@ -71,6 +71,7 @@
u_char *captures_data;
#endif
+ unsigned is_main:1;
unsigned conditional:2;
unsigned encoding:2;
unsigned block:1;
More information about the nginx-devel
mailing list