refactor(utils) clean up and flesh out body args reader

This commit expands the functionality of the request body reader
convenience function to return a table of args for JSON, multipart,
and form body requests. It also refactors away several existing
duplicate implementations.

kong/plugins/aws-lambda/handler.lua

@@ -4,18 +4,12 @@ local BasePlugin = require "kong.plugins.base_plugin"
 local aws_v4 = require "kong.plugins.aws-lambda.v4"
 local responses = require "kong.tools.responses"
 local utils = require "kong.tools.utils"
-local Multipart = require "multipart"
 local http = require "resty.http"
 local cjson = require "cjson.safe"
 local public_utils = require "kong.tools.public"
 
-local string_find = string.find
-local ngx_req_get_headers = ngx.req.get_headers
 local ngx_req_read_body = ngx.req.read_body
 local ngx_req_get_uri_args = ngx.req.get_uri_args
-local ngx_req_get_body_data = ngx.req.get_body_data
-
-local CONTENT_TYPE = "content-type"
 
 local AWS_PORT = 443
 
@@ -27,20 +21,8 @@ end
 
 local function retrieve_parameters()
   ngx_req_read_body()
-  local body_parameters, err
-  local content_type = ngx_req_get_headers()[CONTENT_TYPE]
-  if content_type and string_find(content_type:lower(), "multipart/form-data", nil, true) then
-    body_parameters = Multipart(ngx_req_get_body_data(), content_type):get_all()
-  elseif content_type and string_find(content_type:lower(), "application/json", nil, true) then
-    body_parameters, err = cjson.decode(ngx_req_get_body_data())
-    if err then
-      body_parameters = {}
-    end
-  else
-    body_parameters = public_utils.get_post_args()
-  end
 
-  return utils.table_merge(ngx_req_get_uri_args(), body_parameters)
+  return utils.table_merge(ngx_req_get_uri_args(), public_utils.get_body_args())
 end
 
 function AWSLambdaHandler:access(conf)

kong/plugins/key-auth/handler.lua

@@ -75,7 +75,7 @@ local function do_authentication(conf)
   -- read in the body if we want to examine POST args
   if conf.key_in_body then
     ngx_req_read_body()
-    body_data = public_tools.get_post_args()
+    body_data = public_tools.get_body_args()
   end
 
   -- search in headers & querystring

kong/plugins/oauth2/access.lua

@@ -1,8 +1,6 @@
 local url = require "socket.url"
-local cjson = require "cjson.safe"
 local utils = require "kong.tools.utils"
 local cache = require "kong.tools.database_cache"
-local Multipart = require "multipart"
 local responses = require "kong.tools.responses"
 local constants = require "kong.constants"
 local timestamp = require "kong.tools.timestamp"
@@ -94,19 +92,9 @@ end
 
 local function retrieve_parameters()
   ngx.req.read_body()
-  -- OAuth2 parameters could be in both the querystring or body
-  local body_parameters, err
-  local content_type = req_get_headers()[CONTENT_TYPE]
-  if content_type and string_find(content_type:lower(), "multipart/form-data", nil, true) then
-    body_parameters = Multipart(ngx.req.get_body_data(), content_type):get_all()
-  elseif content_type and string_find(content_type:lower(), "application/json", nil, true) then
-    body_parameters, err = cjson.decode(ngx.req.get_body_data())
-    if err then body_parameters = {} end
-  else
-    body_parameters = public_utils.get_post_args()
-  end
 
-  return utils.table_merge(ngx.req.get_uri_args(), body_parameters)
+  -- OAuth2 parameters could be in both the querystring or body
+  return utils.table_merge(ngx.req.get_uri_args(), public_utils.get_body_args())
 end
 
 local function retrieve_scopes(parameters, conf)
@@ -440,7 +428,7 @@ local function parse_access_token(conf)
 
       if ngx.req.get_method() ~= "GET" and is_form_post then -- Remove from body
         ngx.req.read_body()
-        parameters = public_utils.get_post_args()
+        parameters = public_utils.get_body_args()
         parameters[ACCESS_TOKEN] = nil
         local encoded_args = ngx.encode_args(parameters)
         ngx.req.set_header(CONTENT_LENGTH, #encoded_args)

kong/plugins/runscope/handler.lua

@@ -27,7 +27,7 @@ function RunscopeLogHandler:access(conf)
     local headers = req_get_headers()
     local content_type = headers["content-type"]
     if content_type and string_find(content_type:lower(), "application/x-www-form-urlencoded", nil, true) then
-      req_post_args = public_utils.get_post_args()
+      req_post_args = public_utils.get_body_args()
     end
   end
 

kong/tools/public.lua

@@ -7,18 +7,67 @@ local _M = {}
 
 
 do
+  local multipart = require "multipart"
+  local cjson     = require "cjson.safe"
+
+
+  local str_find              = string.find
   local ngx_req_get_post_args = ngx.req.get_post_args
+  local ngx_req_get_body_data = ngx.req.get_body_data
+
+
+  local content_type_map = {
+    [1] = function(content_type) -- multipart
+      return multipart(ngx_req_get_body_data(), content_type):get_all()
+    end,
+
+    [2] = function() -- json
+      local body, err = cjson.decode(ngx_req_get_body_data())
+      if err then
+        ngx_log(ERR, "could not decode JSON body args: ", err)
+        return {}
+      end
+
+      return body
+    end,
+
+    [3] = function() -- encoded form
+      local ok, res, err = pcall(ngx_req_get_post_args)
+      if not ok or err then
+        local msg = res and res or err
+        ngx_log(ERR, "could not get body args: ", msg)
+        return {}
+      end
+
+      return res
+    end,
+  }
+
+  function _M.get_body_args()
+    local content_type = ngx.var.http_content_type
+
+    if not content_type or content_type == "" then
+      return {}
+    end
+
+    local map_type
+
+    if str_find(content_type, "multipart/form-data", nil, true) then
+      map_type = 1
+
+    elseif str_find(content_type, "application/json", nil, true) then
+      map_type = 2
 
-  function _M.get_post_args()
-    local ok, res, err = pcall(ngx_req_get_post_args)
+    elseif str_find(content_type, "application/www-form-urlencoded", nil, true) or
+      str_find(content_type, "application/x-www-form-urlencoded", nil, true) then
+      map_type = 3
 
-    if not ok or err then
-      local msg = res and res or err
-      ngx_log(ERR, "could not get body args: ", msg)
-      return {} -- TODO return an immutable table here
+    else
+      ngx_log(ERR, "don't know how to parse request body of Content-Type: '", content_type, "'")
+      return {}
     end
 
-    return res
+    return content_type_map[map_type](content_type)
   end
 end
 

spec/03-plugins/10-key-auth/02-access_spec.lua

@@ -166,64 +166,57 @@ describe("Plugin: key-auth (access)", function()
   end)
 
   describe("key in request body", function()
-    it("authenticates valid credentials", function()
-      local res = assert(client:send {
-        path = "/request",
-        headers = {
-          ["Host"] = "key-auth5.com",
-          ["Content-Type"] = "application/www-form-urlencoded",
-        },
-        body = {
-          apikey = "kong",
-        }
-      })
-      assert.res_status(200, res)
-    end)
-    it("returns 403 Forbidden on invalid key", function()
-      local res = assert(client:send {
-        path = "/status/200",
-        headers = {
-          ["Host"] = "key-auth5.com",
-          ["Content-Type"] = "application/www-form-urlencoded",
-        },
-        body = {
-          apikey = "123",
-        }
-      })
-      local body = assert.res_status(403, res)
-      local json = cjson.decode(body)
-      assert.same({ message = "Invalid authentication credentials" }, json)
-    end)
-    it("handles duplicated key", function()
-      local res = assert(client:send {
-        path = "/status/200",
-        headers = {
-          ["Host"] = "key-auth5.com",
-          ["Content-Type"] = "application/www-form-urlencoded",
-        },
-        body = {
-          apikey = { "kong", "kong" },
-        },
-      })
-      local body = assert.res_status(401, res)
-      local json = cjson.decode(body)
-      assert.same({ message = "Duplicate API key found" }, json)
-    end)
-    it("only handles application/www-form-urlencoded bodies", function()
-      local res = assert(client:send {
-        path = "/status/200",
-        headers = {
-          ["Host"] = "key-auth5.com",
-          ["Content-Type"] = "application/json",
-        },
-        body = {
-          apikey = { "kong", "kong" },
-        },
-      })
-      local body = assert.res_status(401, res)
-      local json = cjson.decode(body)
-      assert.same({ message = "No API key found in request" }, json)
-    end)
+    for _, type in pairs({ "application/www-form-urlencoded", "application/json", "multipart/form-data" }) do
+      describe(type, function()
+        it("authenticates valid credentials", function()
+          local res = assert(client:send {
+            path = "/request",
+            headers = {
+              ["Host"] = "key-auth5.com",
+              ["Content-Type"] = type,
+            },
+            body = {
+              apikey = "kong",
+            }
+          })
+          assert.res_status(200, res)
+        end)
+        it("returns 403 Forbidden on invalid key", function()
+          local res = assert(client:send {
+            path = "/status/200",
+            headers = {
+              ["Host"] = "key-auth5.com",
+              ["Content-Type"] = type,
+            },
+            body = {
+              apikey = "123",
+            }
+          })
+          local body = assert.res_status(403, res)
+          local json = cjson.decode(body)
+          assert.same({ message = "Invalid authentication credentials" }, json)
+        end)
+
+        -- lua-multipart doesn't currently handle duplicates in the same method
+        -- that json/form-urlencoded handlers do
+        local test = type == "multipart/form-data" and pending or it
+        test("handles duplicated key", function()
+          local res = assert(client:send {
+            path = "/status/200",
+            headers = {
+              ["Host"] = "key-auth5.com",
+              ["Content-Type"] = type,
+            },
+            body = {
+              apikey = { "kong", "kong" },
+            },
+          })
+          local body = assert.res_status(401, res)
+          local json = cjson.decode(body)
+          assert.same({ message = "Duplicate API key found" }, json)
+        end)
+      end)
+    end
   end)
 
   describe("key in headers", function()