Hello! On Thu, Feb 22, 2018 at 11:53:14AM +1100, Lawrence Stewart via nginx-devel wrote:
> Greetings nginx developers, > > The root cause analysis for a bug I observed during development of some > custom nginx module code turns out to be an unfortunate interaction between > sub-requests and indexed variable lookups using > ngx_http_get_indexed_variable(). We're based on nginx 1.13.6. > > The sequence of events is: > > - Main request arrives and triggers an access-phase sub-request. Main > request includes request variable "x=blah", but sub-request does not. > > - Sub-request arrives at custom module's header filter > > - Custom module looks up variable "x" with ngx_http_get_indexed_variable(), > and it is not found in the sub-request. This causes the "not_found" flag to > be set in the sr->variables var cache for variable "x", but > because ngx_http_subrequest() shallow copies r->variables from parent > request to sub-request, the not_found flag change affects the main > request's r->variables too. > > - Sub-request completes, main request is allowed to progress and arrives at > custom module's header filter > > - Custom module again looks up variable "x" > with ngx_http_get_indexed_variable(), and even though "x" is present in > r->args and I've confirmed can be found by > ngx_http_arg(), ngx_http_get_indexed_variable() sees the not_found flag, > does not refresh the variable cache and returns the poisoned > ngx_http_variable_value_t. > > There are ways to deal with this in our custom code, but it seems like a > fundamental problem in the current design of the sub-request mechanism and > I'm thinking that it may be appropriate to address this in the nginx core > code e.g. by deep copying r->variables in ngx_http_subrequest(), or by > somehow tracking when to invalidate the ngx_http_variable_value_t flags and > refresh the r->variables cache. > > Thoughts? Variables are expected to be shared between the main request and subrequests. If a variable can have different values in the main request and subrequests, consider marking it non-cacheable. For example, the $arg_* variables are defined as follows: { ngx_string("arg_"), NULL, ngx_http_variable_argument, 0, NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_PREFIX, 0 }, This is because request arguments can be changed, and, in particular, can be different in a subrequest. While the approach with shared variables might not be convenient in some scenarious, it is certainly is in many of them - for example, you don't need to recalculate various map{} results, you can freely use variables in SSI includes (the actual use case subrequests were created for), and so on. Given the above, I don't really think we need to change things here. Suggested changes are likely to break more things they will fix. -- Maxim Dounin http://mdounin.ru/ _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel