Request Counter Clarification

[Maxim Dounin]

Hello!

On Mon, Dec 21, 2020 at 08:54:54PM +0600, M L wrote:

> I am developing an NGINX module which would check the contents of the
> request and if the key components found, would block it. Currently, it
> seems to be working correctly, but I would like to clarify some parts and
> make sure that I am not hard-coding anything. So, the question is mainly
> about the request counter.
> During the execution of the request handler (which is registered on the
> HTTP_REWRITE_PHASE), the request counter is kept as it is. But once the
> handler finishes the request processing, the counter is changed to 1. But
> changing the counter to 1 does not seem like a right decision, as many
> other modules more often decrease it in the post_handler or call the
> "finalize request" function. However, the use of "finalize" cannot be
> implemented, as neither connection, nor request should not be finalized
> after the handler execution. Instead, the request needs to be handed over
> to the other phase handlers (return NGX_DECLINED). As for the decrementing
> in the post_handler of the ngx_http_read_client_request_body function, on
> the heavy loads, it results in the segfaults. Finally, leaving the counter
> unchanged throughout the process leads to memory leaks. Therefore, the
> above-described value assignment was implemented, but, perhaps, there are
> better ways of handling the request counter issue? And why the change in
> the request counter can cause a segfault in the first place?

In general, you shouldn't touch the request counter yourself 
unless you really understand what you are doing.  Instead, you 
should correctly call ngx_http_finalize_request() to decrease it 
(or make sure to return correct return code if the phase handler 
does this for you, this will properly decrement).  Increasing the 
request counter in most cases is handled by nginx core as well.

In no cases you are expected to set the request counter to a 
specific value.  It is something to be done only during forced 
request termination.  Any attempt to do this in your own module is 
certainly a bug.

Incorrectly adjusting request counter can lead to segfaults or to 
connection/memory leaks, depending on the exact code path.

In the particular module you've described it looks like the 
problem is that you are trying to read the request body from early 
request processing phases (that is, before the content phase), and 
do this incorrectly.  For a correct example see the mirror module
(http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_mirror_module.c).

In particular, to start reading the request body, do something 
like this (note ngx_http_finalize_request(NGX_DONE) call to 
decrement reference counter, and NGX_DONE return code to stop 
further processing of the request with the phase handlers):

        rc = ngx_http_read_client_request_body(r, ngx_http_mirror_body_handler);
        if (rc >= NGX_HTTP_SPECIAL_RESPONSE) {
            return rc;
        }

        ngx_http_finalize_request(r, NGX_DONE);
        return NGX_DONE;

And to continue processing with other phase handlers you should 
do something like this in the body handler:

    r->write_event_handler = ngx_http_core_run_phases;
    ngx_http_core_run_phases(r);

This ensures that appropriate write event handler is set (as it is 
removed by the request body reading code) and resumes phase 
handling by calling ngx_http_core_run_phases().

-- 
Maxim Dounin
http://mdounin.ru/