Added an API to set the keepalive property of the request

[aviramc]

Setting the keepalive from Lua is useful when implementing all kinds of protocols above ngx_lua.
Some protocols may require that the connection would be closed or stay open.

The is a part of pull request #286

[aviramc]

Will open an updated pull request

[majek]

What's the status of this patch? Did it ever got merged?

[agentzh]

@majek It's replaced by another pull request. I meant to merge this feature soon.

[jdesgats]

I know it it's been a while now, but can you point out which other PR supersedes this one? I can't find a clean way to force a connection close for a particular request with the current API.

[agentzh]

I wonder if we could just achieve the same result by using ngx.req.set_header("Connection", "close")?

@dndx What's your take on this please?

[jdesgats]

Using ngx.req.set_header("Connection", "close") doesn't seem to do the trick, unfortunately.

    server {
        listen       8080;
        server_name  localhost;
        access_log  off;
        location /req {
            rewrite_by_lua 'ngx.req.set_header("Connection", "close")';
            content_by_lua 'ngx.say("Hello")';
        }
        location /resp {
            rewrite_by_lua 'ngx.header["Connection"] = "close"';
            content_by_lua 'ngx.say("Hello")';
        }
    }

➜  ~ curl -v -H 'Connection: keep-alive' 0:8080/req
*   Trying 0.0.0.0...
* TCP_NODELAY set
* Connected to 0 (127.0.0.1) port 8080 (#0)
> GET /req HTTP/1.1
> Host: 0:8080
> User-Agent: curl/7.54.0
> Accept: */*
> Connection: keep-alive
> 
< HTTP/1.1 200 OK
< Server: openresty/1.11.2.5
< Date: Tue, 29 Aug 2017 11:15:27 GMT
< Content-Type: text/plain
< Transfer-Encoding: chunked
< Connection: keep-alive
< 
Hello
* Connection #0 to host 0 left intact
➜  ~ curl -v -H 'Connection: keep-alive' 0:8080/resp
*   Trying 0.0.0.0...
* TCP_NODELAY set
* Connected to 0 (127.0.0.1) port 8080 (#0)
> GET /resp HTTP/1.1
> Host: 0:8080
> User-Agent: curl/7.54.0
> Accept: */*
> Connection: keep-alive
> 
< HTTP/1.1 200 OK
< Server: openresty/1.11.2.5
< Date: Tue, 29 Aug 2017 11:15:31 GMT
< Content-Type: text/plain
< Transfer-Encoding: chunked
< Connection: keep-alive
< Connection: close
< 
Hello
* Closing connection 0

Maybe a better way to implement that feature would be to use the ngx_http_lua_set_handlers callbacks: https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_headers_out.c#L45 so we could use the regular ngx.header API.

[agentzh]

@jdesgats OK, this is because nginx tests r->headers_in.connection_type before your ngx.req.set_header() call, in the ngx_http_handler function. Seems like we should set r->keepalive directly in our own ngx_http_set_connection_header function of src/ngx_http_lua_headers_in.c. Will you contribute a pull request for it? Thanks!

From your examples above, it seems like ngx.header["Connection"] = "close" already fulfills your needs?

[dndx]

@agentzh Somehow I missed this last week but I agree that setting r->keepalive is necessary for this to work.

Also, we should check and make sure clcf->keepalive_timeout is greater than 0 and return an error telling the user that keepalive can not be enabled if not.

See http://lxr.nginx.org/source/src/http/ngx_http_request.c#2602 for the full conditions.

[dndx]

@jdesgats Yeah adding a special handler for the "Connection" header is probably a better way of achieving it without introducing new APIs. I don't even think the fact that currently we have no special set handler for "Connection" makes sense since AFAIK inside ngx_http_header_filter_module.c it uses r->keepalive to generate the "Connection" header in response.

[jdesgats]

@agentzh no, it doesn't really work as the client get 2 Connection headers in the response (as nginx generates one automatically as @dndx pointed out), and the nginx error logs contain a line saying the client closed a keep-alive connection.

I'll try to find some time to make a PR using a special handler :)