upgrade to 1-9.15 caused block requests

[Maxim Dounin]

Hello!

On Thu, Nov 22, 2018 at 07:15:41AM -0500, Ortal wrote:

> This is a small example of my code, I changed the code and in the current
> version on post/put request trying to read the buffer and print it, on any
> other requests just send response (I tried to minimal it as much as I
> could)
> On the current flow when  running a post request with 1MB data the
> ngx_http_read_client_request_body return NGX_AGAIN and not call the
> post_handler
> I am not getting the ngx_http_block_reading for the request but I still do
> not get the request buffer, and the post_handler is not called

In no particular order:

1. In ngx_v3io_test_req_handler() you call 
ngx_v3io_test_send_response() directly if the method is not PUT or 
POST, and return NGX_OK.  And in ngx_v3io_test_send_response() you 
call ngx_http_finalize_request(req, NGX_DONE).  This will result 
in wrong request reference count as the request will be finalized 
twice - first with ngx_http_finalize_request(req, NGX_DONE), and 
then with NGX_OK returned from the handler.  This will basically 
preclude your code from working in non-POST/PUT codepath.

2. You ignore return code from the ngx_http_send_header().  While 
unlikely, this can cause problems if anything different from 
NGX_OK is returned.

3. In ngx_v3io_http_test_handle_post_req(), you call 
ngx_handle_read_event().  This is not something you should do 
unless you understand why you are doing this - usually, this is 
something you should call only in low-level event handling 
functions, when you are reading from the socket yourself.  Doing 
this in your code is certainly not needed and can cause problems.

4. In ngx_v3io_http_test_read_client_request_body(), you decrement 
r->main->count manually.  This is not something you are expected 
to do, and will result in non-working code.

5. The return code of ngx_http_read_client_request_body() is not 
handled correct.  You are expected to do something like this:

    rc = ngx_http_read_client_request_body(r, ngx_http_upstream_init);

    if (rc >= NGX_HTTP_SPECIAL_RESPONSE) {
        return rc;
    }

    return NGX_DONE;

in the request content handler, see, for example, PUT handling in 
the DAV module code, or the development guide here:

http://nginx.org/en/docs/dev/development_guide.html#http_request_body

Your code seems to return NGX_AGAIN in most common cases of NGX_OK 
and NGX_AGAIN instead, and this will lead to undefined results.  
And this is what causes your immediate problems as seen in the 
debug logs.

6. After reading the request body, you finalize the request in 
ngx_v3io_test_send_response() with NGX_DONE.  This is incorrect, 
as due to NGX_DONE nginx will assume that something else is going 
to happen on the request.  Instead, you should use the code 
returned by ngx_http_send_header().

Overall, it looks like there are too many problems in the module 
to work.  It may be a good idea to re-write it from scratch, 
using an nginx module as an example (and looking into the 
development guide).

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