Ensure request body Content-Length is set correctly (#230)

* Avoid sending non-string body values

Fixes #217

* Add newline at EOF

* Document support for table of strings in request body

* Revert change preventing non string request bodies

* Add tests for different request body types

* Ensure request body Content-Length is set correctly

* Don't blindly set request content length

* Add more explicit support for chunked request body iterators

* Use new transfer encoding utility

* Add tests for client body reader

Fails on chunked input until supported by ngx_lua

* Improve transfer encoding check

Co-authored-by: Thijs Schreijer <thijs@thijsschreijer.nl>

Co-authored-by: Thijs Schreijer <thijs@thijsschreijer.nl>

Author: pintsized

README.md

@@ -239,7 +239,7 @@ The `params` table expects the following fields:
 * `path`: The path string. Defaults to `/`.
 * `query`: The query string, presented as either a literal string or Lua table..
 * `headers`: A table of request headers.
-* `body`: The request body as a string, or an iterator function (see [get\_client\_body\_reader](#get_client_body_reader)).
+* `body`: The request body as a string, a table of strings, or an iterator function yielding strings until nil when exhausted. Note that you must specify a `Content-Length` for the request body, or specify `Transfer-Encoding: chunked` and have your function implement the encoding. See also: [get\_client\_body\_reader](#get_client_body_reader)).
 
 When the request is successful, `res` will contain the following fields:
 

lib/resty/http.lua

@@ -398,6 +398,23 @@ local function _receive_headers(sock)
 end
 
 
+local function transfer_encoding_is_chunked(headers)
+    local te = headers["Transfer-Encoding"]
+    if not te then
+        return false
+    end
+
+    -- Handle duplicate headers
+    -- This shouldn't happen but can in the real world
+    if type(te) ~= "string" then
+        te = tbl_concat(te, ",")
+    end
+
+    return str_find(str_lower(te), "chunked", 1, true) ~= nil
+end
+_M.transfer_encoding_is_chunked = transfer_encoding_is_chunked
+
+
 local function _chunked_body_reader(sock, default_chunk_size)
     return co_wrap(function(max_chunk_size)
         local remaining = 0
@@ -575,7 +592,7 @@ end
 
 
 local function _send_body(sock, body)
-    if type(body) == 'function' then
+    if type(body) == "function" then
         repeat
             local chunk, err, partial = body()
 
@@ -627,12 +644,13 @@ function _M.send_request(self, params)
     local body = params.body
     local headers = http_headers.new()
 
-    local params_headers = params.headers or {}
-    -- We assign one by one so that the metatable can handle case insensitivity
+    -- We assign one-by-one so that the metatable can handle case insensitivity
     -- for us. You can blame the spec for this inefficiency.
+    local params_headers = params.headers or {}
     for k, v in pairs(params_headers) do
         headers[k] = v
     end
+
     if not headers["Proxy-Authorization"] then
         -- TODO: next major, change this to always override the provided
         -- header. Can't do that yet because it would be breaking.
@@ -644,12 +662,28 @@ function _M.send_request(self, params)
     -- Ensure minimal headers are set
 
     if not headers["Content-Length"] then
-        if type(body) == 'string' then
-            headers["Content-Length"] = #body
+        local body_type = type(body)
+
+        if body_type == "function" then
+            if not transfer_encoding_is_chunked(headers) then
+                return nil, "Request body is a function but a length or chunked encoding is not specified"
+            end
+
+        elseif body_type == "table" then
+            local length = 0
+            for _, v in ipairs(body) do
+                length = length + #tostring(v)
+            end
+            headers["Content-Length"] = length
+
         elseif body == nil and EXPECTING_BODY[str_upper(params.method)] then
             headers["Content-Length"] = 0
+
+        elseif body ~= nil then
+            headers["Content-Length"] = #tostring(body)
         end
     end
+
     if not headers["Host"] then
         if (str_sub(self.host, 1, 5) == "unix:") then
             return nil, "Unable to generate a useful Host header for a unix domain socket. Please provide one."
@@ -751,22 +785,8 @@ function _M.read_response(self, params)
     if _should_receive_body(params.method, status) then
         has_body = true
 
-        local te = res_headers["Transfer-Encoding"]
-
-        -- Handle duplicate headers
-        -- This shouldn't happen but can in the real world
-        if type(te) == "table" then
-            te = tbl_concat(te, "")
-        end
-
-        local ok, encoding = pcall(str_lower, te)
-        if not ok then
-            encoding = ""
-        end
-
-        if version == 1.1 and str_find(encoding, "chunked", 1, true) ~= nil then
+        if version == 1.1 and transfer_encoding_is_chunked(res_headers) then
             body_reader, err = _chunked_body_reader(sock)
-
         else
             local ok, length = pcall(tonumber, res_headers["Content-Length"])
             if not ok then
@@ -775,9 +795,7 @@ function _M.read_response(self, params)
             end
 
             body_reader, err = _body_reader(sock, length)
-
         end
-
     end
 
     if res_headers["Trailer"] then
@@ -941,10 +959,9 @@ function _M.get_client_body_reader(_, chunksize, sock)
 
     local headers = ngx_req_get_headers()
     local length = headers.content_length
-    local encoding = headers.transfer_encoding
     if length then
         return _body_reader(sock, tonumber(length), chunksize)
-    elseif encoding and str_lower(encoding) == 'chunked' then
+    elseif transfer_encoding_is_chunked(headers) then
         -- Not yet supported by ngx_lua but should just work...
         return _chunked_body_reader(sock, chunksize)
     else

t/02-chunked.t

@@ -238,3 +238,35 @@ table
 --- no_error_log
 [error]
 [warn]
+
+
+=== TEST 5: transfer_encoding_is_chunked utility.
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua_block {
+            local http_headers = require("resty.http_headers")
+            local http = require("resty.http")
+
+            local headers = http_headers:new()
+            assert(http.transfer_encoding_is_chunked(headers) == false,
+                "empty headers should return false")
+
+            headers["Transfer-Encoding"] = "chunked"
+            assert(http.transfer_encoding_is_chunked(headers) == true,
+                "te set to `chunked` should return true`")
+
+            headers["Transfer-Encoding"] = " ChuNkEd "
+            assert(http.transfer_encoding_is_chunked(headers) == true,
+                "te set to ` ChuNkEd ` should return true`")
+
+            headers["Transfer-Encoding"] = { "chunked", " ChuNkEd " }
+            assert(http.transfer_encoding_is_chunked(headers) == true,
+                "te set to table values containing `chunked` should return true`")
+        }
+    }
+--- request
+GET /a
+--- no_error_log
+[error]
+[warn]

t/03-requestbody.t

@@ -203,3 +203,161 @@ Expectation Failed
 --- no_error_log
 [error]
 [warn]
+
+
+=== TEST 5: Non string request bodies are converted with correct length
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua '
+            local httpc = require("resty.http").new()
+
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/b"
+
+            for _, body in ipairs({ 12345,
+                                    true,
+                                    "string",
+                                    { "tab", "le" },
+                                    { "mix", 123, "ed", "tab", "le" } }) do
+
+                local res, err = assert(httpc:request_uri(uri, {
+                    body = body,
+                }))
+
+                ngx.say(res.body)
+            end
+        ';
+    }
+    location = /b {
+        content_by_lua '
+            ngx.req.read_body()
+            ngx.print(ngx.req.get_body_data())
+        ';
+    }
+--- request
+GET /a
+--- response_body
+12345
+true
+string
+table
+mix123edtable
+--- no_error_log
+[error]
+[warn]
+
+
+=== TEST 6: Request body as iterator
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua '
+            local httpc = require("resty.http").new()
+
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/b"
+
+            local res, err = assert(httpc:request_uri(uri, {
+                body = coroutine.wrap(function()
+                    coroutine.yield("foo")
+                    coroutine.yield("bar")
+                end),
+                headers = {
+                    ["Content-Length"] = 6
+                }
+            }))
+
+            ngx.say(res.body)
+        ';
+    }
+    location = /b {
+        content_by_lua '
+            ngx.req.read_body()
+            ngx.print(ngx.req.get_body_data())
+        ';
+    }
+--- request
+GET /a
+--- response_body
+foobar
+--- no_error_log
+[error]
+[warn]
+
+
+=== TEST 7: Request body as iterator, errors with missing length
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua '
+            local httpc = require("resty.http").new()
+
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/b"
+
+            local res, err = httpc:request_uri(uri, {
+                body = coroutine.wrap(function()
+                    coroutine.yield("foo")
+                    coroutine.yield("bar")
+                end),
+            })
+
+            assert(not res)
+            ngx.say(err)
+        ';
+    }
+    location = /b {
+        content_by_lua '
+            ngx.req.read_body()
+            ngx.print(ngx.req.get_body_data())
+        ';
+    }
+--- request
+GET /a
+--- response_body
+Request body is a function but a length or chunked encoding is not specified
+--- no_error_log
+[error]
+[warn]
+
+
+=== TEST 8: Request body as iterator with chunked encoding
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua_block {
+            local httpc = require("resty.http").new()
+            local yield = coroutine.yield
+
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/b"
+
+            local res, err = assert(httpc:request_uri(uri, {
+                body = coroutine.wrap(function()
+                    yield("3\r\n")
+                    yield("foo\r\n")
+
+                    yield("3\r\n")
+                    yield("bar\r\n")
+
+                    yield("0\r\n")
+                    yield("\r\n")
+                end),
+                headers = {
+                    ["Transfer-Encoding"] = "chunked"
+                }
+            }))
+
+            ngx.say(res.body)
+        }
+    }
+    location = /b {
+        content_by_lua '
+            ngx.req.read_body()
+            ngx.print(ngx.req.get_body_data())
+        ';
+    }
+--- request
+GET /a
+--- response_body
+foobar
+--- no_error_log
+[error]
+[warn]

t/10-clientbodyreader.t

@@ -64,3 +64,59 @@ OK
 [error]
 [warn]
 
+
+=== TEST 2: Read request body
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua_block {
+            local httpc = require("resty.http").new()
+
+            local reader, err = assert(httpc:get_client_body_reader())
+
+            repeat
+                local buffer, err = reader()
+                if err then
+                    ngx.log(ngx.ERR, err)
+                end
+
+                if buffer then
+                    ngx.print(buffer)
+                end
+            until not buffer
+        }
+    }
+--- request
+POST /a
+foobar
+--- response_body: foobar
+--- no_error_log
+[error]
+[warn]
+
+
+=== TEST 2: Read chunked request body, errors as not yet supported
+--- http_config eval: $::HttpConfig
+--- config
+    location = /a {
+        content_by_lua_block {
+            local httpc = require("resty.http").new()
+            local _, err = httpc:get_client_body_reader()
+            ngx.log(ngx.ERR, err)
+        }
+    }
+--- more_headers
+Transfer-Encoding: chunked
+--- request eval
+"POST /a
+3\r
+foo\r
+3\r
+bar\r
+0\r
+\r
+"
+--- error_log
+chunked request bodies not supported yet
+--- no_error_log
+[warn]