fix(proxy): honor Accept values weights

honor Accept header values based on their q-values weights

kong/error_handlers.lua

@@ -63,7 +63,7 @@ return function(ctx)
     message = { message = message }
 
   else
-    local mime_type = utils.get_mime_type(accept_header)
+    local mime_type = utils.get_response_type(accept_header)
     message = fmt(utils.get_error_template(mime_type), message)
     headers = { [CONTENT_TYPE] = mime_type }
 

kong/pdk/response.lua

@@ -26,17 +26,14 @@ local find = string.find
 local lower = string.lower
 local error = error
 local pairs = pairs
-local ipairs = ipairs
 local concat = table.concat
-local tonumber = tonumber
 local coroutine = coroutine
 local cjson_encode = cjson.encode
 local normalize_header = checks.normalize_header
 local normalize_multi_header = checks.normalize_multi_header
 local validate_header = checks.validate_header
 local validate_headers = checks.validate_headers
 local check_phase = phase_checker.check
-local split = utils.split
 local add_header
 local is_http_subsystem = ngx and ngx.config.subsystem == "http"
 if is_http_subsystem then
@@ -1061,39 +1058,6 @@ end
   end
 
 
-  local function get_response_type(content_header)
-    local type = CONTENT_TYPE_JSON
-
-    if content_header ~= nil then
-      local accept_values = split(content_header, ",")
-      local max_quality = 0
-      for _, value in ipairs(accept_values) do
-        local mimetype_values = split(value, ";")
-        local name
-        local quality = 1
-        for _, entry in ipairs(mimetype_values) do
-          local m = ngx.re.match(entry, [[^\s*(\S+\/\S+)\s*$]], "ajo")
-          if m then
-            name = m[1]
-          else
-            m = ngx.re.match(entry, [[^\s*q=([0-9]*[\.][0-9]+)\s*$]], "ajoi")
-            if m then
-              quality = tonumber(m[1])
-            end
-          end
-        end
-
-        if name and quality > max_quality then
-          type = utils.get_mime_type(name)
-          max_quality = quality
-        end
-      end
-    end
-
-    return type
-  end
-
-
   ---
   -- This function interrupts the current processing and produces an error
   -- response.
@@ -1195,11 +1159,8 @@ end
       if is_grpc_request() then
         content_type = CONTENT_TYPE_GRPC
       else
-        content_type_header = ngx.req.get_headers()[ACCEPT_NAME]
-        if type(content_type_header) == "table" then
-          content_type_header = content_type_header[1]
-        end
-        content_type = get_response_type(content_type_header)
+        local accept_header = ngx.req.get_headers()[ACCEPT_NAME]
+        content_type = utils.get_response_type(accept_header)
       end
     end
 

kong/tools/utils.lua

@@ -32,6 +32,7 @@ local lower         = string.lower
 local fmt           = string.format
 local find          = string.find
 local gsub          = string.gsub
+local join          = pl_stringx.join
 local split         = pl_stringx.split
 local re_find       = ngx.re.find
 local re_match      = ngx.re.match
@@ -1245,6 +1246,7 @@ end
 
 
 local get_mime_type
+local get_response_type
 local get_error_template
 do
   local CONTENT_TYPE_JSON    = "application/json"
@@ -1324,6 +1326,43 @@ do
     end
   })
 
+
+  get_response_type = function(accept_header)
+    local content_type = MIME_TYPES[CONTENT_TYPE_DEFAULT]
+    if type(accept_header) == "table" then
+      accept_header = join(",", accept_header)
+    end
+
+    if accept_header ~= nil then
+      local accept_values = split(accept_header, ",")
+      local max_quality = 0
+      for _, value in ipairs(accept_values) do
+        local mimetype_values = split(value, ";")
+        local name
+        local quality = 1
+        for _, entry in ipairs(mimetype_values) do
+          local m = ngx.re.match(entry, [[^\s*(\S+\/\S+)\s*$]], "ajo")
+          if m then
+            name = m[1]
+          else
+            m = ngx.re.match(entry, [[^\s*q=([0-9]*[\.][0-9]+)\s*$]], "ajoi")
+            if m then
+              quality = tonumber(m[1])
+            end
+          end
+        end
+
+        if name and quality > max_quality then
+          content_type = get_mime_type(name) or content_type
+          max_quality = quality
+        end
+      end
+    end
+
+    return content_type
+  end
+
+
   get_mime_type = function(content_header, use_default)
     use_default = use_default == nil or use_default
     content_header = _M.strip(content_header)
@@ -1334,7 +1373,7 @@ do
     if #entries > 1 then
       if entries[2] == CONTENT_TYPE_ANY then
         if entries[1] == CONTENT_TYPE_ANY then
-          mime_type = MIME_TYPES["default"]
+          mime_type = MIME_TYPES[CONTENT_TYPE_DEFAULT]
         else
           mime_type = MIME_TYPES[entries[1]]
         end
@@ -1344,7 +1383,7 @@ do
     end
 
     if mime_type or use_default then
-      return mime_type or MIME_TYPES["default"]
+      return mime_type or MIME_TYPES[CONTENT_TYPE_DEFAULT]
     end
 
     return nil, "could not find MIME type"
@@ -1374,6 +1413,7 @@ do
 
 end
 _M.get_mime_type = get_mime_type
+_M.get_response_type = get_response_type
 _M.get_error_template = get_error_template
 
 

spec/02-integration/05-proxy/12-error_default_type_spec.lua

@@ -307,6 +307,37 @@ for _, strategy in helpers.each_strategy() do
           local xml_message = string.format(custom_template, RESPONSE_MESSAGE)
           assert.equal(xml_message, body)
         end)
+
+        describe("with q-values", function()
+          it("defaults to 1 when q-value is not specified", function()
+            local res = assert(proxy_client:send {
+              method  = "GET",
+              path    = "/",
+              headers = {
+                accept = "application/json;q=0.9,text/html,text/plain;q=0.9",
+              }
+            })
+
+            local body = assert.res_status(RESPONSE_CODE, res)
+            local custom_template = pl_file.read(html_template_path)
+            local html_message = string.format(custom_template, RESPONSE_MESSAGE)
+            assert.equal(html_message, body)
+          end)
+
+          it("picks highest q-value (json)", function()
+            local res = assert(proxy_client:send {
+              method  = "GET",
+              path    = "/",
+              headers = {
+                accept = "text/plain;q=0.7,application/json;q=0.8,text/html;q=0.5",
+              }
+            })
+
+            local body = assert.res_status(RESPONSE_CODE, res)
+            local json = cjson.decode(body)
+            assert.equal(RESPONSE_MESSAGE, json.custom_template_message)
+          end)
+        end)
       end)
     end)
   end)

t/01-pdk/08-response/13-error.t

@@ -444,3 +444,43 @@ grpc-status: 8
 grpc-message: ResourceExhausted
 --- no_error_log
 [error]
+
+
+=== TEST 15: service.response.error() honors values of multiple Accept headers
+--- http_config eval: $t::Util::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            kong = {
+              configuration = {},
+            }
+
+            local PDK = require "kong.pdk"
+            local pdk = PDK.new()
+            return pdk.response.error(502)
+        }
+    }
+
+--- request
+GET /t
+--- more_headers
+Accept: text/plain;q=0.2, text/*;q=0.1
+Accept: text/css;q=0.7, text/html;q=0.9, */*;q=0.5
+Accept: application/xml;q=0.2, application/json;q=0.3
+--- error_code: 502
+--- response_headers_like
+Content-Type: text/html; charset=utf-8
+--- response_body
+<!doctype html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Kong Error</title>
+  </head>
+  <body>
+    <h1>Kong Error</h1>
+    <p>An invalid response was received from the upstream server.</p>
+  </body>
+</html>
+--- no_error_log
+[error]