<div dir="ltr"><div>Dear NGINX community, <br><br>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. <br></div><div><br>With best regards, <br>doughnut</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Dec 25, 2020 at 8:47 PM Maxim Dounin <<a href="mailto:mdounin@mdounin.ru">mdounin@mdounin.ru</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello!<br>
<br>
On Mon, Dec 21, 2020 at 08:54:54PM +0600, M L wrote:<br>
<br>
> I am developing an NGINX module which would check the contents of the<br>
> request and if the key components found, would block it. Currently, it<br>
> seems to be working correctly, but I would like to clarify some parts and<br>
> make sure that I am not hard-coding anything. So, the question is mainly<br>
> about the request counter.<br>
> During the execution of the request handler (which is registered on the<br>
> HTTP_REWRITE_PHASE), the request counter is kept as it is. But once the<br>
> handler finishes the request processing, the counter is changed to 1. But<br>
> changing the counter to 1 does not seem like a right decision, as many<br>
> other modules more often decrease it in the post_handler or call the<br>
> "finalize request" function. However, the use of "finalize" cannot be<br>
> implemented, as neither connection, nor request should not be finalized<br>
> after the handler execution. Instead, the request needs to be handed over<br>
> to the other phase handlers (return NGX_DECLINED). As for the decrementing<br>
> in the post_handler of the ngx_http_read_client_request_body function, on<br>
> the heavy loads, it results in the segfaults. Finally, leaving the counter<br>
> unchanged throughout the process leads to memory leaks. Therefore, the<br>
> above-described value assignment was implemented, but, perhaps, there are<br>
> better ways of handling the request counter issue? And why the change in<br>
> the request counter can cause a segfault in the first place?<br>
<br>
In general, you shouldn't touch the request counter yourself <br>
unless you really understand what you are doing.  Instead, you <br>
should correctly call ngx_http_finalize_request() to decrease it <br>
(or make sure to return correct return code if the phase handler <br>
does this for you, this will properly decrement).  Increasing the <br>
request counter in most cases is handled by nginx core as well.<br>
<br>
In no cases you are expected to set the request counter to a <br>
specific value.  It is something to be done only during forced <br>
request termination.  Any attempt to do this in your own module is <br>
certainly a bug.<br>
<br>
Incorrectly adjusting request counter can lead to segfaults or to <br>
connection/memory leaks, depending on the exact code path.<br>
<br>
In the particular module you've described it looks like the <br>
problem is that you are trying to read the request body from early <br>
request processing phases (that is, before the content phase), and <br>
do this incorrectly.  For a correct example see the mirror module<br>
(<a href="http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_mirror_module.c" rel="noreferrer" target="_blank">http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_mirror_module.c</a>).<br>
<br>
In particular, to start reading the request body, do something <br>
like this (note ngx_http_finalize_request(NGX_DONE) call to <br>
decrement reference counter, and NGX_DONE return code to stop <br>
further processing of the request with the phase handlers):<br>
<br>
        rc = ngx_http_read_client_request_body(r, ngx_http_mirror_body_handler);<br>
        if (rc >= NGX_HTTP_SPECIAL_RESPONSE) {<br>
            return rc;<br>
        }<br>
<br>
        ngx_http_finalize_request(r, NGX_DONE);<br>
        return NGX_DONE;<br>
<br>
And to continue processing with other phase handlers you should <br>
do something like this in the body handler:<br>
<br>
    r->write_event_handler = ngx_http_core_run_phases;<br>
    ngx_http_core_run_phases(r);<br>
<br>
This ensures that appropriate write event handler is set (as it is <br>
removed by the request body reading code) and resumes phase <br>
handling by calling ngx_http_core_run_phases().<br>
<br>
-- <br>
Maxim Dounin<br>
<a href="http://mdounin.ru/" rel="noreferrer" target="_blank">http://mdounin.ru/</a><br>
_______________________________________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" rel="noreferrer" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
</blockquote></div>