bug in resumable upload nginx

[slavaslava56ru]

Hey. I have a problem. When the Internet breaks, nginx gives http code 400. I have set cleanup on it by default. When the file is downloaded, a problem occurs. The upper bytes of the file are empty :( Can you tell me how to fix it? Besides removing 400 code from cleanup

[slavaslava56ru]

All that I found in debug nginx

[coldrift]

I don't see any issue here. You asked the system to clean up the file and it was cleaned up as per your log.

[slavaslava56ru]

I don't see any issue here. You asked the system to clean up the file and it was cleaned up as per your log.

Yes, but nginx throw out 400 when the file was not loaded. I do not know what to do :(
is it possible to replace this 400 with another http code or can you tell me where it comes from.

I reproduce the bug in the following steps

1. Starting to load part of the file
2. Turn off the internet
3. nginx return 400

[slavaslava56ru]

attach screen

[slavaslava56ru]

timeout after 7,5 sec and 400 :( and sleanup 😭
I did not ask nginx to return 400

[vkholodkov]

I see HTTP/2.0 request in your log. upload module does not work with http/2.0. The reason 400 is returned is probably because of a bug in nginx.

[slavaslava56ru]

I see HTTP/2.0 request in your log. upload module does not work with http/2.0. The reason 400 is returned is probably because of a bug in nginx.

😮 not knew. Thank you so much

[fdintino]

@vkholodkov it is not true that the module doesn’t support http 2. There are even unit tests to ensure it functions, and we’ve been using it with http 2 for over a year now, when I added that functionality

[vkholodkov]

@fdintino I did not know that. @slavaslava56ru if so, then the issue is somewhere in your system or nginx itself.

[slavaslava56ru]

@vkholodkov Can you tell, Where to look for a problem?

[vkholodkov]

@slavaslava56ru can I ask you where you from and what do you do with nginx and nginx upload module?

[slavaslava56ru]

[slavaslava56ru]

I am trying to upload a file in parts. I specifically turn off the Internet to check that if the part did not upload, then everything is ok. But sometimes at break the request remains hanging and eventually drops from 400

[fdintino]

Are you uploading in the browser with the javascript File interface? If so, you would want to catch the error and retry the upload range that failed. This is how we have implemented a resumable upload with offline network error handling. If you are simply ignoring the error, it makes sense that the range that errored would be filled with null bytes.

[fdintino]

It might also be helpful to diagnose the issue if you provided a gist of the full debug log.

[slavaslava56ru]

Yes, through the js interface. Byte range is loaded again the same on which it broke. The problem is that nginx gives an error which clear ranges loaded before

[slavaslava56ru]

Oki, Now I will try to get a complete log

[slavaslava56ru]

js

nginx

[slavaslava56ru]

[slavaslava56ru]

debug (7).log

[slavaslava56ru]

may be reason in this directives

reset_timedout_connection on
client_body_timeout
client_header_timeout
send_timeout

[fdintino]

Is there any chance you have the directive upload_cleanup somewhere in your config?

[slavaslava56ru]

upload_cleanup 400 404 500-599

[vkholodkov]

I think what happens is that you get n == NGX_ERROR at this place in ngx_http_do_read_upload_client_request_body:

            if (n == 0 || n == NGX_ERROR) {
                c->error = 1;
                return NGX_HTTP_BAD_REQUEST;
            }

The 400 status codes is not appropriate here, because the request has not been received entirely. More appropriate code is NGX_HTTP_CLIENT_CLOSED_REQUEST.

Please replace NGX_HTTP_BAD_REQUEST with NGX_HTTP_CLIENT_CLOSED_REQUEST and see if it helps your situation.

[fdintino]

Yes, @vkholodkov is right I believe. We're returning the wrong status code in that bit of code. That, combined with the upload_cleanup 400 404 500-599 directive, are causing the file cleanup.

If you were to change the upload_cleanup instead to

upload_cleanup 404 500-599;

that would temporarily work around the bug, without requiring you to recompile nginx. But that's just a temporary hack. I have some time today, I'll implement the above fix and add a test for it.

That said, it would still be helpful to get feedback about whether that is actually the fix that's needed in your case.

[fdintino]

Hmmm.... one other thing. Are you sure you recompiled the module with debug flags? I don't see any of the nginx-upload-module debug messages in the log file you posted.

[slavaslava56ru]

Sorry, that nginx build is gone, so I can’t say for sure

[vkholodkov]

you can use nginx -V to dump the full command line that was used to run your build

[slavaslava56ru]

https://user-images.githubusercontent.com/32861091/70521900-df477000-1b61-11ea-95b6-2cc1f9af0f2f.png

[slavaslava56ru]

Today will fix?

[fdintino]

I only ever get a 408 when I try to replicate your issue. Can you try applying @vkholodkov's patch and see if it fixes things for you?

[slavaslava56ru]

@fdintino ok, now see

[slavaslava56ru]

not fixed :(

[coldrift]

If you still get 400, this means your client sends something wrong, because other places where upload module return 400 are related to request validation.

[SoNWarrioR]

I think what happens is that you get n == NGX_ERROR at this place in ngx_http_do_read_upload_client_request_body:

            if (n == 0 || n == NGX_ERROR) {
                c->error = 1;
                return NGX_HTTP_BAD_REQUEST;
            }

The 400 status codes is not appropriate here, because the request has not been received entirely. More appropriate code is NGX_HTTP_CLIENT_CLOSED_REQUEST.

Please replace NGX_HTTP_BAD_REQUEST with NGX_HTTP_CLIENT_CLOSED_REQUEST and see if it helps your situation.

Hello. Have same problem, and this patch not fixed it, can u help me?