From 2a8439a52fa73fa1ad3d56aea8e1018851364ca5 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 09:53:24 -0400
Subject: [PATCH 01/16] feat: implement pool_only_after_response option to
 restrict keep alive

---
 lib/resty/http.lua         | 68 ++++++++++++++++++++++++++++----------
 lib/resty/http_connect.lua | 21 +++++++-----
 2 files changed, 63 insertions(+), 26 deletions(-)

diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index 70c3bee9..c730f24d 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -134,7 +134,9 @@ function _M.new(_)
     if not sock then
         return nil, err
     end
-    return setmetatable({ sock = sock, keepalive = true }, mt)
+    return setmetatable({
+        sock = sock, keepalive_supported = true, keepalive_ready = false, pool_only_after_response = false
+    }, mt)
 end
 
 
@@ -195,7 +197,7 @@ function _M.tcp_only_connect(self, ...)
         self.port = nil
     end
 
-    self.keepalive = true
+    self.keepalive_supported = true
     self.ssl = false
 
     return sock:connect(...)
@@ -208,7 +210,11 @@ function _M.set_keepalive(self, ...)
         return nil, "not initialized"
     end
 
-    if self.keepalive == true then
+    if self.keepalive_supported == true then
+        if self.pool_only_after_response and not self.keepalive_ready then
+            return nil, "response not fully read"
+        end
+
         return sock:setkeepalive(...)
     else
         -- The server said we must close the connection, so we cannot setkeepalive.
@@ -429,7 +435,18 @@ end
 _M.transfer_encoding_is_chunked = transfer_encoding_is_chunked
 
 
-local function _chunked_body_reader(sock, default_chunk_size)
+local function _reader_keepalive_ready_mark(http_client)
+    return co_wrap(function()
+        http_client.keepalive_ready = true
+    end)
+end
+
+local function _reader_keepalive_ready_no_op()
+    return co_wrap(function() end)
+end
+
+
+local function _chunked_body_reader(keepalive_ready_callback, sock, default_chunk_size)
     return co_wrap(function(max_chunk_size)
         local remaining = 0
         local length
@@ -487,11 +504,13 @@ local function _chunked_body_reader(sock, default_chunk_size)
             end
 
         until length == 0
+
+        keepalive_ready_callback()
     end)
 end
 
 
-local function _body_reader(sock, content_length, default_chunk_size)
+local function _body_reader(keepalive_ready_callback, sock, content_length, default_chunk_size)
     return co_wrap(function(max_chunk_size)
         max_chunk_size = max_chunk_size or default_chunk_size
 
@@ -521,6 +540,7 @@ local function _body_reader(sock, content_length, default_chunk_size)
         elseif not max_chunk_size then
             -- We have a length and potentially keep-alive, but want everything.
             co_yield(sock:receive(content_length))
+            keepalive_ready_callback()
 
         else
             -- We have a length and potentially a keep-alive, and wish to stream
@@ -549,6 +569,7 @@ local function _body_reader(sock, content_length, default_chunk_size)
                 end
 
             until length == 0
+            keepalive_ready_callback()
         end
     end)
 end
@@ -587,9 +608,10 @@ local function _read_body(res)
 end
 
 
-local function _trailer_reader(sock)
+local function _trailer_reader(keepalive_ready_callback, sock)
     return co_wrap(function()
         co_yield(_receive_headers(sock))
+        keepalive_ready_callback()
     end)
 end
 
@@ -781,7 +803,8 @@ function _M.read_response(self, params)
     end
 
 
-    local res_headers, err = _receive_headers(sock)
+    local res_headers
+    res_headers, err = _receive_headers(sock)
     if not res_headers then
         return nil, err
     end
@@ -791,38 +814,48 @@ function _M.read_response(self, params)
     if ok then
         if (version == 1.1 and str_find(connection, "close", 1, true)) or
            (version == 1.0 and not str_find(connection, "keep-alive", 1, true)) then
-            self.keepalive = false
+            self.keepalive_supported = false
         end
     else
         -- no connection header
         if version == 1.0 then
-            self.keepalive = false
+            self.keepalive_supported = false
         end
     end
 
     local body_reader = _no_body_reader
-    local trailer_reader, err
+    local trailer_reader
     local has_body = false
+    local has_trailer = false
+    -- If there are no trailers - fully reading response body means socket is ready to be pooled
+    local body_reader_keepalive_ready_callback = _reader_keepalive_ready_mark(self)
+
+    if res_headers["Trailer"] then
+        has_trailer = true
+        -- If there are trailers - fully reading response body doesn't mean socket is ready to be pooled
+        body_reader_keepalive_ready_callback = _reader_keepalive_ready_no_op()
+    end
 
     -- Receive the body_reader
     if _should_receive_body(params.method, status) then
         has_body = true
 
         if version == 1.1 and transfer_encoding_is_chunked(res_headers) then
-            body_reader, err = _chunked_body_reader(sock)
+            body_reader, err = _chunked_body_reader(body_reader_keepalive_ready_callback, sock)
         else
-            local ok, length = pcall(tonumber, res_headers["Content-Length"])
+            local length
+            ok, length = pcall(tonumber, res_headers["Content-Length"])
             if not ok then
                 -- No content-length header, read until connection is closed by server
                 length = nil
             end
 
-            body_reader, err = _body_reader(sock, length)
+            body_reader, err = _body_reader(body_reader_keepalive_ready_callback, sock, length)
         end
     end
 
-    if res_headers["Trailer"] then
-        trailer_reader, err = _trailer_reader(sock)
+    if has_trailer then
+        trailer_reader, err = _trailer_reader(_reader_keepalive_ready_mark(self), sock)
     end
 
     if err then
@@ -981,13 +1014,14 @@ function _M.get_client_body_reader(_, chunksize, sock)
         end
     end
 
+    local reader_keep_alive_ready_callback = _reader_keepalive_ready_no_op()
     local headers = ngx_req_get_headers()
     local length = headers.content_length
     if length then
-        return _body_reader(sock, tonumber(length), chunksize)
+        return _body_reader(reader_keep_alive_ready_callback, sock, tonumber(length), chunksize)
     elseif transfer_encoding_is_chunked(headers) then
         -- Not yet supported by ngx_lua but should just work...
-        return _chunked_body_reader(sock, chunksize)
+        return _chunked_body_reader(reader_keep_alive_ready_callback, sock, chunksize)
     else
         return nil
     end
diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 4da98bcb..b4fdab04 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -16,19 +16,20 @@ be kept alive.
 Call it with a single options table as follows:
 
 client:connect {
-    scheme = "https"        -- scheme to use, or nil for unix domain socket
-    host = "myhost.com",    -- target machine, or a unix domain socket
-    port = nil,             -- port on target machine, will default to 80/443 based on scheme
-    pool = nil,             -- connection pool name, leave blank! this function knows best!
-    pool_size = nil,        -- options as per: https://github.com/openresty/lua-nginx-module#tcpsockconnect
+    scheme = "https"                  -- scheme to use, or nil for unix domain socket
+    host = "myhost.com",              -- target machine, or a unix domain socket
+    port = nil,                       -- port on target machine, will default to 80/443 based on scheme
+    pool = nil,                       -- connection pool name, leave blank! this function knows best!
+    pool_size = nil,                  -- options as per: https://github.com/openresty/lua-nginx-module#tcpsockconnect
     backlog = nil,
+    pool_only_after_response = false, -- only allow set_keepalive() after http response fully read
 
     -- ssl options as per: https://github.com/openresty/lua-nginx-module#tcpsocksslhandshake
     ssl_reused_session = nil
     ssl_server_name = nil,
     ssl_send_status_req = nil,
-    ssl_verify = true,      -- NOTE: defaults to true
-    ctx = nil,              -- NOTE: not supported
+    ssl_verify = true,                -- NOTE: defaults to true
+    ctx = nil,                        -- NOTE: not supported
 
     -- mTLS options (experimental!)
     --
@@ -43,7 +44,7 @@ client:connect {
     ssl_client_cert = nil,
     ssl_client_priv_key = nil,
 
-    proxy_opts,             -- proxy opts, defaults to global proxy options
+    proxy_opts,                       -- proxy opts, defaults to global proxy options
 }
 ]]
 local function connect(self, options)
@@ -60,6 +61,7 @@ local function connect(self, options)
 
     local poolname = options.pool
     local pool_size = options.pool_size
+    local pool_only_after_response = options.pool_only_after_response
     local backlog = options.backlog
 
     if request_scheme and not request_port then
@@ -261,11 +263,12 @@ local function connect(self, options)
 
     self.host = request_host
     self.port = request_port
-    self.keepalive = true
+    self.keepalive_supported = true
     self.ssl = ssl
     -- set only for http, https has already been handled
     self.http_proxy_auth = request_scheme ~= "https" and proxy_authorization or nil
     self.path_prefix = path_prefix
+    self.pool_only_after_response = pool_only_after_response
 
     return true, nil, ssl_session
 end

From 679b0ecd1edc5325d29af8a7f21ca6c441a94076 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 09:56:48 -0400
Subject: [PATCH 02/16] doc: document pool_only_after_response

---
 README.md                  | 1 +
 lib/resty/http_connect.lua | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 9c97b4b5..11211f07 100644
--- a/README.md
+++ b/README.md
@@ -171,6 +171,7 @@ The options table has the following fields:
 * `pool`: custom connection pool name. Option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsockconnect), except that the default will become a pool name constructed using the SSL / proxy properties, which is important for safe connection reuse. When in doubt, leave it blank!
 * `pool_size`: option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsockconnect)
 * `backlog`: option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsockconnect)
+* `pool_only_after_response`: when enabled, set_keepalive() verifies http response fully read before pooling. Default is `false`
 * `proxy_opts`: sub-table, defaults to the global proxy options set, see [set\_proxy\_options](#set_proxy_options).
 * `ssl_reused_session`: option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsocksslhandshake)
 * `ssl_verify`: option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsocksslhandshake), except that it defaults to `true`.
diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index b4fdab04..699b4d52 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -22,7 +22,7 @@ client:connect {
     pool = nil,                       -- connection pool name, leave blank! this function knows best!
     pool_size = nil,                  -- options as per: https://github.com/openresty/lua-nginx-module#tcpsockconnect
     backlog = nil,
-    pool_only_after_response = false, -- only allow set_keepalive() after http response fully read
+    pool_only_after_response = false, -- set_keepalive() verifies http response fully read before pooling
 
     -- ssl options as per: https://github.com/openresty/lua-nginx-module#tcpsocksslhandshake
     ssl_reused_session = nil

From a428ea85ba0b85ff9b751e7de828a61670e07436 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:01:49 -0400
Subject: [PATCH 03/16] test: add tests to check pool_only_after_response

---
 t/07-keepalive.t | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/t/07-keepalive.t b/t/07-keepalive.t
index 573bbf0d..663f1b8b 100644
--- a/t/07-keepalive.t
+++ b/t/07-keepalive.t
@@ -432,3 +432,93 @@ connection must be closed
 --- no_error_log
 [error]
 [warn]
+
+=== TEST 8 Generic interface, Connection: Keep-alive. pool_only_after_response is on. Test the connection is reused.
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua '
+            local http = require "resty.http"
+            local httpc = http.new()
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port,
+                pool_only_after_response = true
+            })
+
+            local res, err = httpc:request{
+                path = "/b"
+            }
+
+            local body = res:read_body()
+
+            ngx.say(res.headers["Connection"])
+            ngx.say(httpc:set_keepalive())
+
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port
+            })
+            ngx.say(httpc:get_reused_times())
+        ';
+    }
+    location = /b {
+        content_by_lua '
+            ngx.say("OK")
+        ';
+    }
+--- request
+GET /a
+--- response_body
+keep-alive
+1
+1
+--- no_error_log
+[error]
+[warn]
+
+=== TEST 9 Generic interface, Connection: Keep-alive. pool_only_after_response is on. Don't read body and check connection isn't reused
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua '
+            local http = require "resty.http"
+            local httpc = http.new()
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port,
+                pool_only_after_response = true
+            })
+
+            local res, err = httpc:request{
+                path = "/b"
+            }
+
+            ngx.say(res.headers["Connection"])
+            ngx.say(httpc:set_keepalive())
+
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port
+            })
+            ngx.say(httpc:get_reused_times())
+        ';
+    }
+    location = /b {
+        content_by_lua '
+            ngx.say("OK")
+        ';
+    }
+--- request
+GET /a
+--- response_body
+keep-alive
+0
+0
+--- no_error_log
+[error]
+[warn]

From f5075a2792bb0f90aa8a341d321d317e2c7c2792 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:05:44 -0400
Subject: [PATCH 04/16] fix: failing test

---
 t/07-keepalive.t | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/07-keepalive.t b/t/07-keepalive.t
index 663f1b8b..bcc5e7ee 100644
--- a/t/07-keepalive.t
+++ b/t/07-keepalive.t
@@ -498,7 +498,8 @@ keep-alive
             }
 
             ngx.say(res.headers["Connection"])
-            ngx.say(httpc:set_keepalive())
+            local ok, err = httpc:set_keepalive()
+            ngx.say(err)
 
             httpc:connect({
                 scheme = "http",
@@ -517,7 +518,7 @@ keep-alive
 GET /a
 --- response_body
 keep-alive
-0
+response not fully read
 0
 --- no_error_log
 [error]

From 58b02e25c55649d71d01849a4e9b0d9f1211bd41 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:16:21 -0400
Subject: [PATCH 05/16] fix: use keepalive_supported instead of keepalive

---
 t/15-instance-reuse.t | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/15-instance-reuse.t b/t/15-instance-reuse.t
index 414e2cb2..a9515546 100644
--- a/t/15-instance-reuse.t
+++ b/t/15-instance-reuse.t
@@ -53,12 +53,12 @@ location /a {
         assert(res3 ~= res2, "responses should be unique tables")
         assert(res3.headers ~= res2.headers, "headers should be unique tables")
 
-        assert(httpc.keepalive == false, "keepalive flag should be false")
+        assert(httpc.keepalive_supported == false, "keepalive flag should be false")
 
         assert(httpc:connect("127.0.0.1", ngx.var.server_port),
             "connect should return positively")
 
-        assert(httpc.keepalive == true, "keepalive flag should be true")
+        assert(httpc.keepalive_supported == true, "keepalive flag should be true")
 
     }
 }

From b6e776cecfe2dc3a0c01a540fd6af3d15ab6f646 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:26:03 -0400
Subject: [PATCH 06/16] fix: reset keepalive_ready once sending a new request

---
 lib/resty/http.lua | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index c730f24d..8e17f0d6 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -676,6 +676,9 @@ function _M.send_request(self, params)
     -- Apply defaults
     setmetatable(params, { __index = DEFAULT_PARAMS })
 
+    -- Sending a new request makes keepalive disabled until its response is fully read
+    self.keepalive_ready = false
+
     local sock = self.sock
     local body = params.body
     local headers = http_headers.new()

From ab80363c269f00ac0f2b63d02a35440fdd95a647 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:33:02 -0400
Subject: [PATCH 07/16] fix: start with keepalive_ready being true since socket
 is empty before requests are made

---
 lib/resty/http.lua | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index 8e17f0d6..2ce8d015 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -135,7 +135,7 @@ function _M.new(_)
         return nil, err
     end
     return setmetatable({
-        sock = sock, keepalive_supported = true, keepalive_ready = false, pool_only_after_response = false
+        sock = sock, keepalive_supported = true, keepalive_ready = true, pool_only_after_response = false
     }, mt)
 end
 

From af8a92279f367361ccd6d7dd153c0d9752e62ee9 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:33:24 -0400
Subject: [PATCH 08/16] test: add tests to check that client pooling works
 before any requests, and after 2 requests

---
 t/07-keepalive.t | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/t/07-keepalive.t b/t/07-keepalive.t
index bcc5e7ee..9e13fa89 100644
--- a/t/07-keepalive.t
+++ b/t/07-keepalive.t
@@ -523,3 +523,67 @@ response not fully read
 --- no_error_log
 [error]
 [warn]
+
+=== TEST 10 Pooling connection immediately after creation should work
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua '
+            local http = require "resty.http"
+            local httpc = http.new()
+            ngx.say(httpc:set_keepalive())
+        ';
+    }
+--- request
+GET /a
+--- response_body
+1
+--- no_error_log
+[error]
+[warn]
+
+=== TEST 11 Reusing client still checks pooling is ready
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua '
+            local http = require "resty.http"
+            local httpc = http.new()
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port,
+                pool_only_after_response = true
+            })
+
+            local res, err = httpc:request{
+                path = "/b"
+            }
+
+            local body = res:read_body()
+
+            ngx.say(res.headers["Connection"])
+            ngx.say(httpc:set_keepalive())
+
+            res, err = httpc:request{
+                path = "/b"
+            }
+            local ok
+            ok, err = httpc:set_keepalive())
+            ngx.say(err)
+        ';
+    }
+    location = /b {
+        content_by_lua '
+            ngx.say("OK")
+        ';
+    }
+--- request
+GET /a
+--- response_body
+keep-alive
+1
+response not fully read
+--- no_error_log
+[error]
+[warn]

From 01dfb0110763fb9939022b8a14e5c163327c21fc Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:36:21 -0400
Subject: [PATCH 09/16] fix: typo in test

---
 t/07-keepalive.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/07-keepalive.t b/t/07-keepalive.t
index 9e13fa89..87ccd6b5 100644
--- a/t/07-keepalive.t
+++ b/t/07-keepalive.t
@@ -569,7 +569,7 @@ GET /a
                 path = "/b"
             }
             local ok
-            ok, err = httpc:set_keepalive())
+            ok, err = httpc:set_keepalive()
             ngx.say(err)
         ';
     }

From 587299193d0845c9e633178a160910579328bf48 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:38:57 -0400
Subject: [PATCH 10/16] fix: test failing because connect missing

---
 t/07-keepalive.t | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/07-keepalive.t b/t/07-keepalive.t
index 87ccd6b5..40d46282 100644
--- a/t/07-keepalive.t
+++ b/t/07-keepalive.t
@@ -565,6 +565,13 @@ GET /a
             ngx.say(res.headers["Connection"])
             ngx.say(httpc:set_keepalive())
 
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port,
+                pool_only_after_response = true
+            })
+            ngx.say(httpc:get_reused_times())
             res, err = httpc:request{
                 path = "/b"
             }
@@ -583,6 +590,7 @@ GET /a
 --- response_body
 keep-alive
 1
+1
 response not fully read
 --- no_error_log
 [error]

From 6a4075c23671560c8e6fdf54a7cfa43d1058fe8e Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 13 Oct 2021 10:46:19 -0400
Subject: [PATCH 11/16] fix: keepalive_ready mark only after connection

---
 lib/resty/http.lua         | 2 +-
 lib/resty/http_connect.lua | 2 ++
 t/07-keepalive.t           | 8 +++++++-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index 2ce8d015..8e17f0d6 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -135,7 +135,7 @@ function _M.new(_)
         return nil, err
     end
     return setmetatable({
-        sock = sock, keepalive_supported = true, keepalive_ready = true, pool_only_after_response = false
+        sock = sock, keepalive_supported = true, keepalive_ready = false, pool_only_after_response = false
     }, mt)
 end
 
diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 699b4d52..ab68ab5b 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -263,6 +263,8 @@ local function connect(self, options)
 
     self.host = request_host
     self.port = request_port
+    -- Immediately after connection - keepalive should be possible
+    self.keepalive_ready = true
     self.keepalive_supported = true
     self.ssl = ssl
     -- set only for http, https has already been handled
diff --git a/t/07-keepalive.t b/t/07-keepalive.t
index 40d46282..61e6ac0c 100644
--- a/t/07-keepalive.t
+++ b/t/07-keepalive.t
@@ -524,13 +524,19 @@ response not fully read
 [error]
 [warn]
 
-=== TEST 10 Pooling connection immediately after creation should work
+=== TEST 10 Pooling connection immediately after connecting should work
 --- http_config eval: $::HttpConfig
 --- config
     location = /a {
         content_by_lua '
             local http = require "resty.http"
             local httpc = http.new()
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port,
+                pool_only_after_response = true
+            })
             ngx.say(httpc:set_keepalive())
         ';
     }

From 2d4feca43c448123c27a46aee0e6db239607011e Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Fri, 10 Dec 2021 15:10:16 -0500
Subject: [PATCH 12/16] fix: mark keepalive_ready if no body and no trailer

---
 lib/resty/http.lua |  5 +++++
 t/07-keepalive.t   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index 8e17f0d6..e1239fd9 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -855,6 +855,11 @@ function _M.read_response(self, params)
 
             body_reader, err = _body_reader(body_reader_keepalive_ready_callback, sock, length)
         end
+    else
+        if not has_trailer then
+            -- If there's no body and no trailer - it's ready for keep-alive
+            self.keepalive_ready = true
+        end
     end
 
     if has_trailer then
diff --git a/t/07-keepalive.t b/t/07-keepalive.t
index 61e6ac0c..0dc661e2 100644
--- a/t/07-keepalive.t
+++ b/t/07-keepalive.t
@@ -601,3 +601,48 @@ response not fully read
 --- no_error_log
 [error]
 [warn]
+
+=== TEST 12 pool_only_after_response is on. Test the connection is reused on non-body requests.
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua '
+            local http = require "resty.http"
+            local httpc = http.new()
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port,
+                pool_only_after_response = true
+            })
+
+            local res, err = httpc:request{
+                method = "HEAD",
+                path = "/b"
+            }
+
+            ngx.say(res.headers["Connection"])
+            ngx.say(httpc:set_keepalive())
+
+            httpc:connect({
+                scheme = "http",
+                host = "127.0.0.1",
+                port = ngx.var.server_port
+            })
+            ngx.say(httpc:get_reused_times())
+        ';
+    }
+    location = /b {
+        content_by_lua '
+            ngx.say("OK")
+        ';
+    }
+--- request
+GET /a
+--- response_body
+keep-alive
+1
+1
+--- no_error_log
+[error]
+[warn]

From 7feec2ee24527e9e24b3b9d601d620da29643842 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 20 Apr 2022 10:57:25 -0400
Subject: [PATCH 13/16] feat: convert coroutines to closures, remove
 unnecessary duplicate call to closures if there are trailers

---
 lib/resty/http.lua | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index e1239fd9..cac9dc59 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -436,13 +436,13 @@ _M.transfer_encoding_is_chunked = transfer_encoding_is_chunked
 
 
 local function _reader_keepalive_ready_mark(http_client)
-    return co_wrap(function()
+    return function()
         http_client.keepalive_ready = true
-    end)
+    end
 end
 
 local function _reader_keepalive_ready_no_op()
-    return co_wrap(function() end)
+    return function() end
 end
 
 
@@ -830,13 +830,15 @@ function _M.read_response(self, params)
     local trailer_reader
     local has_body = false
     local has_trailer = false
-    -- If there are no trailers - fully reading response body means socket is ready to be pooled
-    local body_reader_keepalive_ready_callback = _reader_keepalive_ready_mark(self)
+    local body_reader_keepalive_ready_callback
 
     if res_headers["Trailer"] then
         has_trailer = true
         -- If there are trailers - fully reading response body doesn't mean socket is ready to be pooled
         body_reader_keepalive_ready_callback = _reader_keepalive_ready_no_op()
+    else
+        -- If there are no trailers - fully reading response body means socket is ready to be pooled
+        body_reader_keepalive_ready_callback = _reader_keepalive_ready_mark(self)
     end
 
     -- Receive the body_reader

From 91935c49264ecf4f78f5f069ed7695b11c704ace Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 20 Apr 2022 11:02:58 -0400
Subject: [PATCH 14/16] feat: remove optional flag pool_only_after_response,
 its always on

---
 README.md                  |  1 -
 lib/resty/http.lua         |  4 +--
 lib/resty/http_connect.lua |  3 --
 t/07-keepalive.t           | 67 +++++---------------------------------
 4 files changed, 10 insertions(+), 65 deletions(-)

diff --git a/README.md b/README.md
index 11211f07..9c97b4b5 100644
--- a/README.md
+++ b/README.md
@@ -171,7 +171,6 @@ The options table has the following fields:
 * `pool`: custom connection pool name. Option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsockconnect), except that the default will become a pool name constructed using the SSL / proxy properties, which is important for safe connection reuse. When in doubt, leave it blank!
 * `pool_size`: option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsockconnect)
 * `backlog`: option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsockconnect)
-* `pool_only_after_response`: when enabled, set_keepalive() verifies http response fully read before pooling. Default is `false`
 * `proxy_opts`: sub-table, defaults to the global proxy options set, see [set\_proxy\_options](#set_proxy_options).
 * `ssl_reused_session`: option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsocksslhandshake)
 * `ssl_verify`: option as per [OpenResty docs](https://github.com/openresty/lua-nginx-module#tcpsocksslhandshake), except that it defaults to `true`.
diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index cac9dc59..18aedaff 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -135,7 +135,7 @@ function _M.new(_)
         return nil, err
     end
     return setmetatable({
-        sock = sock, keepalive_supported = true, keepalive_ready = false, pool_only_after_response = false
+        sock = sock, keepalive_supported = true, keepalive_ready = false
     }, mt)
 end
 
@@ -211,7 +211,7 @@ function _M.set_keepalive(self, ...)
     end
 
     if self.keepalive_supported == true then
-        if self.pool_only_after_response and not self.keepalive_ready then
+        if not self.keepalive_ready then
             return nil, "response not fully read"
         end
 
diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index ab68ab5b..3e61c3e0 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -22,7 +22,6 @@ client:connect {
     pool = nil,                       -- connection pool name, leave blank! this function knows best!
     pool_size = nil,                  -- options as per: https://github.com/openresty/lua-nginx-module#tcpsockconnect
     backlog = nil,
-    pool_only_after_response = false, -- set_keepalive() verifies http response fully read before pooling
 
     -- ssl options as per: https://github.com/openresty/lua-nginx-module#tcpsocksslhandshake
     ssl_reused_session = nil
@@ -61,7 +60,6 @@ local function connect(self, options)
 
     local poolname = options.pool
     local pool_size = options.pool_size
-    local pool_only_after_response = options.pool_only_after_response
     local backlog = options.backlog
 
     if request_scheme and not request_port then
@@ -270,7 +268,6 @@ local function connect(self, options)
     -- set only for http, https has already been handled
     self.http_proxy_auth = request_scheme ~= "https" and proxy_authorization or nil
     self.path_prefix = path_prefix
-    self.pool_only_after_response = pool_only_after_response
 
     return true, nil, ssl_session
 end
diff --git a/t/07-keepalive.t b/t/07-keepalive.t
index 0dc661e2..c39bafe7 100644
--- a/t/07-keepalive.t
+++ b/t/07-keepalive.t
@@ -433,65 +433,18 @@ connection must be closed
 [error]
 [warn]
 
-=== TEST 8 Generic interface, Connection: Keep-alive. pool_only_after_response is on. Test the connection is reused.
+=== TEST 8 Generic interface, Connection: Keep-alive. Don't read body and check connection isn't reused
 --- http_config eval: $::HttpConfig
 --- config
     location = /a {
         content_by_lua '
             local http = require "resty.http"
             local httpc = http.new()
-            httpc:connect({
-                scheme = "http",
-                host = "127.0.0.1",
-                port = ngx.var.server_port,
-                pool_only_after_response = true
-            })
-
-            local res, err = httpc:request{
-                path = "/b"
-            }
-
-            local body = res:read_body()
-
-            ngx.say(res.headers["Connection"])
-            ngx.say(httpc:set_keepalive())
-
             httpc:connect({
                 scheme = "http",
                 host = "127.0.0.1",
                 port = ngx.var.server_port
             })
-            ngx.say(httpc:get_reused_times())
-        ';
-    }
-    location = /b {
-        content_by_lua '
-            ngx.say("OK")
-        ';
-    }
---- request
-GET /a
---- response_body
-keep-alive
-1
-1
---- no_error_log
-[error]
-[warn]
-
-=== TEST 9 Generic interface, Connection: Keep-alive. pool_only_after_response is on. Don't read body and check connection isn't reused
---- http_config eval: $::HttpConfig
---- config
-    location = /a {
-        content_by_lua '
-            local http = require "resty.http"
-            local httpc = http.new()
-            httpc:connect({
-                scheme = "http",
-                host = "127.0.0.1",
-                port = ngx.var.server_port,
-                pool_only_after_response = true
-            })
 
             local res, err = httpc:request{
                 path = "/b"
@@ -524,7 +477,7 @@ response not fully read
 [error]
 [warn]
 
-=== TEST 10 Pooling connection immediately after connecting should work
+=== TEST 9 Pooling connection immediately after connecting should work
 --- http_config eval: $::HttpConfig
 --- config
     location = /a {
@@ -534,8 +487,7 @@ response not fully read
             httpc:connect({
                 scheme = "http",
                 host = "127.0.0.1",
-                port = ngx.var.server_port,
-                pool_only_after_response = true
+                port = ngx.var.server_port
             })
             ngx.say(httpc:set_keepalive())
         ';
@@ -548,7 +500,7 @@ GET /a
 [error]
 [warn]
 
-=== TEST 11 Reusing client still checks pooling is ready
+=== TEST 10 Reused client still checks pooling is ready
 --- http_config eval: $::HttpConfig
 --- config
     location = /a {
@@ -558,8 +510,7 @@ GET /a
             httpc:connect({
                 scheme = "http",
                 host = "127.0.0.1",
-                port = ngx.var.server_port,
-                pool_only_after_response = true
+                port = ngx.var.server_port
             })
 
             local res, err = httpc:request{
@@ -574,8 +525,7 @@ GET /a
             httpc:connect({
                 scheme = "http",
                 host = "127.0.0.1",
-                port = ngx.var.server_port,
-                pool_only_after_response = true
+                port = ngx.var.server_port
             })
             ngx.say(httpc:get_reused_times())
             res, err = httpc:request{
@@ -602,7 +552,7 @@ response not fully read
 [error]
 [warn]
 
-=== TEST 12 pool_only_after_response is on. Test the connection is reused on non-body requests.
+=== TEST 11 Test the connection is reused on non-body requests
 --- http_config eval: $::HttpConfig
 --- config
     location = /a {
@@ -612,8 +562,7 @@ response not fully read
             httpc:connect({
                 scheme = "http",
                 host = "127.0.0.1",
-                port = ngx.var.server_port,
-                pool_only_after_response = true
+                port = ngx.var.server_port
             })
 
             local res, err = httpc:request{

From 5d3a82c95ea44b18eacf8b0da2f6ecf8e6fb4bd7 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 20 Apr 2022 11:49:20 -0400
Subject: [PATCH 15/16] feat: rewrite keepalive_ready feature based on
 reader_state table

---
 lib/resty/http.lua         | 67 +++++++++++++++++---------------------
 lib/resty/http_connect.lua |  2 +-
 2 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index 18aedaff..5ed802ba 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -135,7 +135,7 @@ function _M.new(_)
         return nil, err
     end
     return setmetatable({
-        sock = sock, keepalive_supported = true, keepalive_ready = false
+        sock = sock, keepalive_supported = true, reader_state = { keepalive_ready = false, mark_keepalive_ready_on_body_read = true }
     }, mt)
 end
 
@@ -197,6 +197,8 @@ function _M.tcp_only_connect(self, ...)
         self.port = nil
     end
 
+    -- Immediately after connection - keepalive should be possible
+    self.reader_state.keepalive_ready = true
     self.keepalive_supported = true
     self.ssl = false
 
@@ -211,7 +213,7 @@ function _M.set_keepalive(self, ...)
     end
 
     if self.keepalive_supported == true then
-        if not self.keepalive_ready then
+        if not self.reader_state.keepalive_ready then
             return nil, "response not fully read"
         end
 
@@ -435,18 +437,7 @@ end
 _M.transfer_encoding_is_chunked = transfer_encoding_is_chunked
 
 
-local function _reader_keepalive_ready_mark(http_client)
-    return function()
-        http_client.keepalive_ready = true
-    end
-end
-
-local function _reader_keepalive_ready_no_op()
-    return function() end
-end
-
-
-local function _chunked_body_reader(keepalive_ready_callback, sock, default_chunk_size)
+local function _chunked_body_reader(reader_state, sock, default_chunk_size)
     return co_wrap(function(max_chunk_size)
         local remaining = 0
         local length
@@ -505,12 +496,14 @@ local function _chunked_body_reader(keepalive_ready_callback, sock, default_chun
 
         until length == 0
 
-        keepalive_ready_callback()
+        if reader_state.mark_keepalive_ready_on_body_read then
+            reader_state.keepalive_ready = true
+        end
     end)
 end
 
 
-local function _body_reader(keepalive_ready_callback, sock, content_length, default_chunk_size)
+local function _body_reader(reader_state, sock, content_length, default_chunk_size)
     return co_wrap(function(max_chunk_size)
         max_chunk_size = max_chunk_size or default_chunk_size
 
@@ -540,8 +533,9 @@ local function _body_reader(keepalive_ready_callback, sock, content_length, defa
         elseif not max_chunk_size then
             -- We have a length and potentially keep-alive, but want everything.
             co_yield(sock:receive(content_length))
-            keepalive_ready_callback()
-
+            if reader_state.mark_keepalive_ready_on_body_read then
+                reader_state.keepalive_ready = true
+            end
         else
             -- We have a length and potentially a keep-alive, and wish to stream
             -- the response.
@@ -569,7 +563,9 @@ local function _body_reader(keepalive_ready_callback, sock, content_length, defa
                 end
 
             until length == 0
-            keepalive_ready_callback()
+            if reader_state.mark_keepalive_ready_on_body_read then
+                reader_state.keepalive_ready = true
+            end
         end
     end)
 end
@@ -608,10 +604,11 @@ local function _read_body(res)
 end
 
 
-local function _trailer_reader(keepalive_ready_callback, sock)
+local function _trailer_reader(reader_state, sock)
     return co_wrap(function()
         co_yield(_receive_headers(sock))
-        keepalive_ready_callback()
+        -- We can always pool after reading trailers
+        reader_state.keepalive_ready = true
     end)
 end
 
@@ -677,7 +674,7 @@ function _M.send_request(self, params)
     setmetatable(params, { __index = DEFAULT_PARAMS })
 
     -- Sending a new request makes keepalive disabled until its response is fully read
-    self.keepalive_ready = false
+    self.reader_state.keepalive_ready = false
 
     local sock = self.sock
     local body = params.body
@@ -830,23 +827,16 @@ function _M.read_response(self, params)
     local trailer_reader
     local has_body = false
     local has_trailer = false
-    local body_reader_keepalive_ready_callback
 
-    if res_headers["Trailer"] then
-        has_trailer = true
-        -- If there are trailers - fully reading response body doesn't mean socket is ready to be pooled
-        body_reader_keepalive_ready_callback = _reader_keepalive_ready_no_op()
-    else
-        -- If there are no trailers - fully reading response body means socket is ready to be pooled
-        body_reader_keepalive_ready_callback = _reader_keepalive_ready_mark(self)
-    end
+    has_trailer = (res_headers["Trailer"] ~= nil)
+    self.reader_state.mark_keepalive_ready_on_body_read = not has_trailer
 
     -- Receive the body_reader
     if _should_receive_body(params.method, status) then
         has_body = true
 
         if version == 1.1 and transfer_encoding_is_chunked(res_headers) then
-            body_reader, err = _chunked_body_reader(body_reader_keepalive_ready_callback, sock)
+            body_reader, err = _chunked_body_reader(self.reader_state, sock)
         else
             local length
             ok, length = pcall(tonumber, res_headers["Content-Length"])
@@ -855,17 +845,17 @@ function _M.read_response(self, params)
                 length = nil
             end
 
-            body_reader, err = _body_reader(body_reader_keepalive_ready_callback, sock, length)
+            body_reader, err = _body_reader(self.reader_state, sock, length)
         end
     else
         if not has_trailer then
             -- If there's no body and no trailer - it's ready for keep-alive
-            self.keepalive_ready = true
+            self.reader_state.keepalive_ready = true
         end
     end
 
     if has_trailer then
-        trailer_reader, err = _trailer_reader(_reader_keepalive_ready_mark(self), sock)
+        trailer_reader, err = _trailer_reader(self.reader_state, sock)
     end
 
     if err then
@@ -1024,14 +1014,15 @@ function _M.get_client_body_reader(_, chunksize, sock)
         end
     end
 
-    local reader_keep_alive_ready_callback = _reader_keepalive_ready_no_op()
+    -- Reading the request body has nothing to do with pooling the upstream server socket
+    local request_body_reader_state = { mark_keepalive_ready_on_body_read = false }
     local headers = ngx_req_get_headers()
     local length = headers.content_length
     if length then
-        return _body_reader(reader_keep_alive_ready_callback, sock, tonumber(length), chunksize)
+        return _body_reader(request_body_reader_state, sock, tonumber(length), chunksize)
     elseif transfer_encoding_is_chunked(headers) then
         -- Not yet supported by ngx_lua but should just work...
-        return _chunked_body_reader(reader_keep_alive_ready_callback, sock, chunksize)
+        return _chunked_body_reader(request_body_reader_state, sock, chunksize)
     else
         return nil
     end
diff --git a/lib/resty/http_connect.lua b/lib/resty/http_connect.lua
index 3e61c3e0..997e38bc 100644
--- a/lib/resty/http_connect.lua
+++ b/lib/resty/http_connect.lua
@@ -262,7 +262,7 @@ local function connect(self, options)
     self.host = request_host
     self.port = request_port
     -- Immediately after connection - keepalive should be possible
-    self.keepalive_ready = true
+    self.reader_state.keepalive_ready = true
     self.keepalive_supported = true
     self.ssl = ssl
     -- set only for http, https has already been handled

From 948d6521f4ef73feb3f088ab5ba204340146dd71 Mon Sep 17 00:00:00 2001
From: Guy Lewin <guy@lewin.co.il>
Date: Wed, 20 Apr 2022 11:52:38 -0400
Subject: [PATCH 16/16] fix: luacheck errors

---
 lib/resty/http.lua | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/resty/http.lua b/lib/resty/http.lua
index 5ed802ba..5e051716 100644
--- a/lib/resty/http.lua
+++ b/lib/resty/http.lua
@@ -135,7 +135,9 @@ function _M.new(_)
         return nil, err
     end
     return setmetatable({
-        sock = sock, keepalive_supported = true, reader_state = { keepalive_ready = false, mark_keepalive_ready_on_body_read = true }
+        sock = sock,
+        keepalive_supported = true,
+        reader_state = { keepalive_ready = false, mark_keepalive_ready_on_body_read = true }
     }, mt)
 end
 
@@ -826,9 +828,7 @@ function _M.read_response(self, params)
     local body_reader = _no_body_reader
     local trailer_reader
     local has_body = false
-    local has_trailer = false
-
-    has_trailer = (res_headers["Trailer"] ~= nil)
+    local has_trailer = (res_headers["Trailer"] ~= nil)
     self.reader_state.mark_keepalive_ready_on_body_read = not has_trailer
 
     -- Receive the body_reader