Opened 3 years ago
Closed 3 years ago
#2215 closed defect (wontfix)
Maybe a minior bug in $request_completion code
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | minor | Milestone: | nginx-1.21 |
Component: | documentation | Version: | 1.19.x |
Keywords: | variable | Cc: | |
uname -a: | Linux | ||
nginx -V: | nginx 1.19.10 |
Description
Hi,
When I do some test on NGINX variables, I found some strange behavior about $request_completion.
Give the config:
log_format resp_demo '$request_completion';
location /resp {
access_log /var/log/nginx/access.log resp_demo;
return 200 '$request_completion\n';
}
Then I run curl 127.1/resp, we get (empty string).
Because $request_completion is defined as CACHEABLE, so when we log it in access.log ,it will be the same empty value, not the right value 'OK'.
I think $request_completion should act as other variables like $is_arg, Shall we change the flag to NGX_HTTP_VAR_NOCACHEABLE?
Here is my patch:
--- ngx_http_variables.c.old 2021-07-12 20:11:59.985843678 +0800
+++ ngx_http_variables.c 2021-07-12 20:12:22.881285689 +0800
@@ -282,7 +282,7 @@
{ ngx_string("request_completion"), NULL,
ngx_http_variable_request_completion,
- 0, 0, 0 },
+ 0, NGX_HTTP_VAR_NOCACHEABLE, 0 },
{ ngx_string("request_body"), NULL,
ngx_http_variable_request_body,
Attachments (1)
Change History (5)
by , 3 years ago
Attachment: | request_completion.patch added |
---|
comment:1 by , 3 years ago
comment:2 by , 3 years ago
OK, I got it.But I have another question:
What the proper meaning of $request_completion. It is only useful in log phase, but at this time the request is done, what can I do with $request_completion?
comment:3 by , 3 years ago
Or maybe the empty value of $request_completion is the right meaning of it.
In the phases before log, we get empty value, this will tell us we are still processing request, then in the log phase, the request is definitely completed, so we should not care about $request_completion.
comment:4 by , 3 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
The $request_completion
variable is documented as follows:
$request_completion
“OK” if a request has completed, or an empty string otherwise
The "request has completed" means that all the response data has been sent to the client socket. That is, it is always empty before the logging phase (and hence it does not make sense to use it anywhere before logging). At the logging phase, it can be either "OK" (if all the response data has been successfully sent) or an empty string (if an error occurred during sending, for example, if the client closed connection).
Closing this as "wontfix" as it looks like there are no significant reasons to make the variable non-cacheable. As outlined above, a better solution is to avoid using the variable before the logging phase where the variable make sense.
This is more or less a general problem: there are a lot of variables which can be in certain cases accessed before they have a known meaningful value, and if accessed before they are usable the wrong value can be cached. On the other hand, the value does not change once it is properly set. Examples include
$sent_http_*
variables before the response is generated,$remote_addr
(if updated by the realip module, see #603),$request_body
before the request body is read, and many others.The
$request_completion
variable is one of such variables. It doesn't make sense to use it anywhere but during logging, though an attempt to anyway use it, as in your configuration snippet, will result in the wrong value being cached. While it is possible to fix this by making the variable non-cacheable, it is probably not the best possible solution, as this makes proper use of the variable less effective. A better solution might be to avoid incorrect use of the variable.If you think that using
$request_completion
make sense before the logging phase, please elaborate.