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

[Ciel]

Hello!

On Monday, November 21st, 2022 at 17:28, Sergey Kandaurov <pluknet at nginx.com> wrote:

> 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.

Thanks for pointing out that. I've been scanning this code finding `mctx`s, but
the one in `ngx_http_ssi_get_variable` used `ctx` instead.

Now I've changed that variable to `mctx` to align with other references, and
also made sure that every context of `r->main` gets correct name. If that's not
appropriate, feel free to trim that part.

I'm not familiar with nginx's roll-out strategy, could anyone kindly tell me when
shall I expect to see this usable in a stable/mainline release?

Updated patch below.


# HG changeset patch
# User Ciel Zhao <i at ciel.dev>
# Date 1669029675 -28800
#      Mon Nov 21 19:21:15 2022 +0800
# Node ID a42cc9c1b1fd6c07798c7ad6c6a2a3da24e9cc21
# Parent  17d6a537fb1bb587e4de22961bf5be5f0c648fa8
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 -r 17d6a537fb1b -r a42cc9c1b1fd 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     Mon Nov 21 19:21:15 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,6 +341,8 @@
         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,26 @@
     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;
+
+#if (NGX_PCRE)
+            ctx->ncaptures = mctx->ncaptures;
+            ctx->captures = mctx->captures;
+            ctx->captures_data = mctx->captures_data;
+#endif
+
+            mctx->shared = 0;
+        }
+
         ngx_http_clear_content_length(r);
         ngx_http_clear_accept_ranges(r);
 
@@ -379,6 +401,10 @@
         } 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 +431,7 @@
     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
@@ -1559,9 +1586,9 @@
     ngx_uint_t           i;
     ngx_list_part_t     *part;
     ngx_http_ssi_var_t  *var;
-    ngx_http_ssi_ctx_t  *ctx;
-
-    ctx = ngx_http_get_module_ctx(r->main, ngx_http_ssi_filter_module);
+    ngx_http_ssi_ctx_t  *mctx;
+
+    mctx = ngx_http_get_module_ctx(r->main, ngx_http_ssi_filter_module);
 
 #if (NGX_PCRE)
     {
@@ -1570,7 +1597,7 @@
     if (key >= '0' && key <= '9') {
         i = key - '0';
 
-        if (i < ctx->ncaptures) {
+        if (i < mctx->ncaptures) {
             value = ngx_palloc(r->pool, sizeof(ngx_str_t));
             if (value == NULL) {
                 return NULL;
@@ -1578,8 +1605,8 @@
 
             i *= 2;
 
-            value->data = ctx->captures_data + ctx->captures[i];
-            value->len = ctx->captures[i + 1] - ctx->captures[i];
+            value->data = mctx->captures_data + mctx->captures[i];
+            value->len = mctx->captures[i + 1] - mctx->captures[i];
 
             return value;
         }
@@ -1587,11 +1614,11 @@
     }
 #endif
 
-    if (ctx->variables == NULL) {
+    if (mctx->variables == NULL) {
         return NULL;
     }
 
-    part = &ctx->variables->part;
+    part = &mctx->variables->part;
     var = part->elts;
 
     for (i = 0; /* void */ ; i++) {
diff -r 17d6a537fb1b -r a42cc9c1b1fd 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     Mon Nov 21 19:21:15 2022 +0800
@@ -71,6 +71,7 @@
     u_char                   *captures_data;
 #endif
 
+    unsigned                  shared:1;
     unsigned                  conditional:2;
     unsigned                  encoding:2;
     unsigned                  block:1;