Request Counter Clarification

[M L]

Dear NGINX community,

I had some questions regarding the module development. The module I am
developing processes the request and sometimes (especially when there is a
large body) the request processing takes long enough to disrupt Nginx
lifecycle. To handle this problem I've added a feature of adding a posted
event if the processing exceeds given time. To read the body of the
request, I use "read client req body" function, and after its execution, I
was advised to finalize the request with ngx_http_finalize_request(r,
NGX_DONE). But won't it disrupt the working of the posted event? I post it
with ngx_post_event function. Or maybe there is some else solution, and I
don't even need to add a posted event? For example, I could make smth like
a for loop in the post handler of the ngx_http_read_client_request_body
which would check if the request processing finished by my handler, and
exit if it did, or some error occurred.

With best regards,
doughnut

On Fri, Dec 25, 2020 at 8:47 PM Maxim Dounin <mdounin at mdounin.ru> wrote:

> 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/
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20210116/ed43d5b0/attachment.htm>