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

Ciel i at ciel.dev
Fri Nov 4 09:00:33 UTC 2022


Hi Maxim,

Thanks for the quick reply.

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

I encountered this problem building my own website, and spent some hours to
figure out the problem and patch. I didn't guess this bug have such history.
Reference will be in the updated patch.

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

As we only use the main context for variables and stub blocks, it seems appreciable
to have the context created on demand. However, we can't say if the SSI templates
use these commands until we processed them. So I think this left us merely two 
options:

a. create main context for every requests having SSI enabled (previous patch)
b. check main context existence everywhere we use it

>From my purview, most SSI templates have some variables involved, so just go ahead
and create context can save many checks, while not introducing much waste of memory.

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

Totally agree, and this can be checked by `ngx_http_ssi_header_filter` in main request,
to initialize the dummy context if needed. But I wonder shall I add a bit field into 
`ngx_http_ssi_ctx_t` (as there're already some so no memory cost) or add an
`ngx_flag_t` (8B on amd64) instead.

In the issue tracker 6 years ago, you mentioned,

> It was written when there were no subrequests except subrequests created by the SSI
> module itself, and assumes in many places that its context always exists in the main
> request.

Though I'm not 100% familiar with the process of subrequests, this arise some more
questions in my mind: If subrequests are processed by the SSI filter (before postpone)
asynchronously and concurrently, could this introduce some out-of-order execution of
SSI commands among subrequests, then lead to non-determined result? If this really
happens, should we move the SSI filter past the postpone filter? Looking forward to
some enlightenment.

I'll post my updated patch after these discussions settle. And thanks again for attention.

Ciel Zhao



More information about the nginx-devel mailing list