openresty / lua-nginx-module

Embed the Power of Lua into NGINX HTTP servers
https://openresty.org/
11.28k stars 2.03k forks source link

echo "HTTP/1.1 100 Continue" before any response data to downstream #1324

Open tokers opened 6 years ago

tokers commented 6 years ago

Hello!

Recently I found some HTTP client (like curl) will complain "Illegal or missing hexadecimal sequence in chunked-encoding" when sending request to OpenResty which has some configurations like this:

location / {
    content_by_lua_block {
        ngx.print("hello")
    }
}

Sends request like this:

curl -v http://127.0.0.1:8080/ --upload-file /path/to/file

When I discard the request body explicitly (ngx.req.discard_body). Everything is OK.

I'm not sure whether this is the callers' responsibility to discard the request body explicitly. Or ngx_lua can do this implicitly (send HTTP/1.1 100 Continue before any data). What's your opinion?

spacewander commented 6 years ago

It is recommened to call ngx.req.discard_body explicitly.

In cases where current request may have a request body and the request body data is not required, The ngx.req.discard_body function must be used to explicitly discard the request body to avoid breaking things under HTTP 1.1 keepalive or HTTP 1.1 pipelining.

BTW, doesn't your client send Expect: 100-continue? I remember that curl will send the header in this case. Nginx will send HTTP/1.1 100 Continue implicitly if Expect: 100-continue is sent by the client.

tokers commented 6 years ago

BTW, doesn't your client send Expect: 100-continue? I remember that curl will send the header in this case. Nginx will send HTTP/1.1 100 Continue implicitly if Expect: 100-continue is sent by the client.

Yes. Curl will send the Expect: 100-continue.

However, the problem is the time to send HTTP/1.1 Continue. When we send some data through ngx.print or ngx.say (chunked way) and flush these data by ngx.flush prior to discard the request body. The client will receive the HTTP response header and response body before it gets HTTP/1.1 Continue.

In this case. Client might complain the Illegal or missing hexadecimal sequence in chunked-encoding problem. You can reproduce this problem by the following configuration:

server {
    server_name _;
     listen 8000;

    location = /t {
        client_max_body_size 10g;
        content_by_lua_block {
             ngx.print("asdasdas")
             ngx.flush(true)
    }
}

The result is:

curl -v http://127.0.0.1:8000/t --upload-file /path/to/file
* Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8000 (#0)
> PUT /t HTTP/1.1
> Host: 127.0.0.1:8000
> User-Agent: curl/7.53.1
> Accept: */*
> Content-Length: 5868163
> Expect: 100-continue
>
< HTTP/1.1 200 OK
< Server: openresty/1.13.6.1
< Date: Fri, 18 May 2018 12:25:07 GMT
< Content-Type: application/octet-stream
< Transfer-Encoding: chunked
< Connection: keep-alive
<
* Illegal or missing hexadecimal sequence in chunked-encoding
* stopped the pause stream!
* Closing connection 0
curl: (56) Illegal or missing hexadecimal sequence in chunked-encoding
hello%

Another case. If we send data with the plain way and flush the header firstly then discard the request body like this:

 server {
     listen       8000;
     server_name  _;

    location = /t {
         client_max_body_size 10g;
         content_by_lua_block {
             ngx.header["Content-Length"] = 5
             ngx.status = 200
             ngx.send_headers()
             ngx.flush(true)
             ngx.print("hello")
    }
}

And the result will be:

curl -v http://127.0.0.1:8000/t --upload-file /path/to/file
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8000 (#0)
> PUT /t HTTP/1.1
> Host: 127.0.0.1:8000
> User-Agent: curl/7.53.1
> Accept: */*
> Content-Length: 5868163
> Expect: 100-continue
>
< HTTP/1.1 200 OK
< Server: openresty/1.13.6.1
< Date: Fri, 18 May 2018 12:30:13 GMT
< Content-Type: application/octet-stream
< Connection: keep-alive
< Content-Length: 5
<
* Excess found in a non pipelined read: excess = 25, size = 5, maxdownload = 5, bytecount = 0
* Connection #0 to host 127.0.0.1 left intact
HTTP/%

So the HTTP/1.1 Continue pollutes the HTTP response data.

So caller must discard body by ngx.req.discard_body before he sends any data. How about reminding it in the document?

Or we can create a Pull Request and test the Expect header before the user wants to send data?

spacewander commented 6 years ago

How about reminding it in the document?

Actually it is already in a corner of the document. In the previous comment I have quoted it out.

test the Expect header before the user wants to send data?

So do you mean testing the Expect header, if Expect exists, response with 100 Continue and discard the body?

Let's wait for @agentzh 's opinion.

spacewander commented 6 years ago

test the Expect header before the user wants to send data?

IMHO, it is not a general solution. What if your client doesn't send Expect header?

tokers commented 6 years ago

So do you mean testing the Expect header, if Expect exists, response with 100 Continue and discard the body?

Just call the ngx_http_test_expect when user is trying to send headers.

niko commented 4 years ago

Is there any news on that? Is it possible to send a 100 continue followed by a 200 ok? This part (two status lines?) of the HTTP spec strikes me as weird, but there are clients expecting that anyway.