Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(pdk): increase the precision of JSON number encoding from 14 to 16 decimals #12019

Merged
merged 7 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Increase the precision of JSON number encoding from 14 to 16 decimals
type: feature
scope: PDK
1 change: 1 addition & 0 deletions kong-3.6.0-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ build = {
["kong.tools.module"] = "kong/tools/module.lua",
["kong.tools.ip"] = "kong/tools/ip.lua",
["kong.tools.http"] = "kong/tools/http.lua",
["kong.tools.cjson"] = "kong/tools/cjson.lua",

["kong.runloop.handler"] = "kong/runloop/handler.lua",
["kong.runloop.events"] = "kong/runloop/events.lua",
Expand Down
1 change: 1 addition & 0 deletions kong/constants.lua
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ for k in pairs(key_formats_map) do
end

local constants = {
CJSON_MAX_PRECISION = 16,
BUNDLED_PLUGINS = plugin_map,
DEPRECATED_PLUGINS = deprecated_plugin_map,
BUNDLED_VAULTS = vault_map,
Expand Down
9 changes: 6 additions & 3 deletions kong/db/strategies/postgres/init.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
local arrays = require "pgmoon.arrays"
local json = require "pgmoon.json"
local cjson = require "cjson"
local cjson_safe = require "cjson.safe"
local utils = require "kong.tools.utils"
local new_tab = require "table.new"
Expand Down Expand Up @@ -180,6 +179,10 @@ local function escape_literal(connector, literal, field)
return concat { "TO_TIMESTAMP(", connector:escape_literal(tonumber(fmt("%.3f", literal))), ") AT TIME ZONE 'UTC'" }
end

if field.type == "integer" then
return fmt("%16.f", literal)
end

if field.type == "array" or field.type == "set" then
if not literal[1] then
return connector:escape_literal("{}")
Expand Down Expand Up @@ -211,7 +214,7 @@ local function escape_literal(connector, literal, field)
elseif et == "map" or et == "record" or et == "json" then
local jsons = {}
for i, v in ipairs(literal) do
jsons[i] = cjson.encode(v)
jsons[i] = cjson_safe.encode(v)
end
return encode_array(jsons) .. '::JSONB[]'

Expand Down Expand Up @@ -522,7 +525,7 @@ local function page(self, size, token, foreign_key, foreign_entity_name, options
insert(offset, row[field_name])
end

offset = cjson.encode(offset)
offset = cjson_safe.encode(offset)
offset = encode_base64(offset, true)

return rows, nil, offset
Expand Down
3 changes: 1 addition & 2 deletions kong/db/strategies/postgres/tags.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
local cjson = require "cjson"
local cjson_safe = require "cjson.safe"


Expand Down Expand Up @@ -118,7 +117,7 @@ local function page(self, size, token, options, tag)
last_ordinality
}

offset = cjson.encode(offset)
offset = cjson_safe.encode(offset)
offset = encode_base64(offset, true)

return rows, nil, offset
Expand Down
9 changes: 6 additions & 3 deletions kong/globalpatches.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
local constants = require "kong.constants"

local ran_before


Expand All @@ -15,15 +17,16 @@ return function(options)
local meta = require "kong.meta"


local cjson = require("cjson.safe")
cjson.encode_sparse_array(nil, nil, 2^15)
local cjson_safe = require("cjson.safe")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we replace it with kong.tools.cjson?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt you can, since this is in globalpatches, which gets loaded very early on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't simply replace it, because currently, our global patch doesn't set cjson.decode_array_with_array_mt(true), whereas kong.tools.cjson does.

cjson.decode_array_with_array_mt(true)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got. The code makes me a bit confusing.

cjson_safe.encode_sparse_array(nil, nil, 2^15)
cjson_safe.encode_number_precision(constants.CJSON_MAX_PRECISION)

local pb = require "pb"

-- let pb decode arrays to table cjson.empty_array_mt metatable
-- so empty arrays are encoded as `[]` instead of `nil` or `{}` by cjson.
pb.option("decode_default_array")
pb.defaults("*array", cjson.empty_array_mt)
pb.defaults("*array", cjson_safe.empty_array_mt)

if options.cli then
-- disable the _G write guard alert log introduced in OpenResty 1.15.8.1
Expand Down
6 changes: 2 additions & 4 deletions kong/pdk/request.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
-- @module kong.request


local cjson = require "cjson.safe".new()
local cjson = require "kong.tools.cjson"
local multipart = require "multipart"
local phase_checker = require "kong.pdk.private.phases"
local normalize = require("kong.tools.uri").normalize
Expand Down Expand Up @@ -44,8 +44,6 @@ local decode_args = ngx.decode_args
local PHASES = phase_checker.phases


cjson.decode_array_with_array_mt(true)


local function new(self)
local _REQUEST = {}
Expand Down Expand Up @@ -832,7 +830,7 @@ local function new(self)
return nil, err, CONTENT_TYPE_JSON
end

local json = cjson.decode(body)
local json = cjson.decode_with_array_mt(body)
if type(json) ~= "table" then
return nil, "invalid json body", CONTENT_TYPE_JSON
end
Expand Down
6 changes: 2 additions & 4 deletions kong/pdk/service/response.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
-- @module kong.service.response


local cjson = require "cjson.safe".new()
local cjson = require "kong.tools.cjson"
local multipart = require "multipart"
local phase_checker = require "kong.pdk.private.phases"
local string_tools = require "kong.tools.string"
Expand All @@ -23,8 +23,6 @@ local setmetatable = setmetatable
local check_phase = phase_checker.check


cjson.decode_array_with_array_mt(true)


local replace_dashes = string_tools.replace_dashes
local replace_dashes_lower = string_tools.replace_dashes_lower
Expand Down Expand Up @@ -356,7 +354,7 @@ local function new(pdk, major_version)

elseif find(content_type_lower, CONTENT_TYPE_JSON, 1, true) == 1 then
local body = response.get_raw_body()
local json = cjson.decode(body)
local json = cjson.decode_with_array_mt(body)
if type(json) ~= "table" then
return nil, "invalid json body", CONTENT_TYPE_JSON
end
Expand Down
21 changes: 21 additions & 0 deletions kong/tools/cjson.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
local cjson = require "cjson.safe".new()
local constants = require "kong.constants"

cjson.decode_array_with_array_mt(true)
cjson.encode_sparse_array(nil, nil, 2^15)
cjson.encode_number_precision(constants.CJSON_MAX_PRECISION)

local _M = {}


function _M.encode(json_text)
return cjson.encode(json_text)
end

function _M.decode_with_array_mt(json_text)
return cjson.decode(json_text)
end

_M.array_mt = cjson.array_mt

return _M
110 changes: 110 additions & 0 deletions spec/02-integration/04-admin_api/25-max_safe_integer_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
local helpers = require "spec.helpers"

local LMDB_MAP_SIZE = "10m"

for _, strategy in helpers.each_strategy() do
if strategy ~= "off" then
describe("Admin API #" .. strategy, function()
local bp
local client, route

lazy_setup(function()
bp = helpers.get_db_utils(strategy, {
"routes",
"services",
})

route = bp.routes:insert({
paths = { "/route_with_max_safe_integer_priority"},
regex_priority = 9007199254740992,
})

assert(helpers.start_kong({
database = strategy,
}))
end)

lazy_teardown(function()
helpers.stop_kong(nil, true)
end)

before_each(function()
client = assert(helpers.admin_client())
end)

after_each(function()
if client then
client:close()
end
end)

it("the maximum safe integer can be accurately represented as a decimal number", function()
local res = assert(client:send {
method = "GET",
path = "/routes/" .. route.id
})
assert.res_status(200, res)
assert.match_re(res:read_body(), "9007199254740992")
end)
end)
end

if strategy == "off" then
describe("Admin API #off", function()
local client

lazy_setup(function()
assert(helpers.start_kong({
database = "off",
lmdb_map_size = LMDB_MAP_SIZE,
stream_listen = "127.0.0.1:9011",
nginx_conf = "spec/fixtures/custom_nginx.template",
}))
end)

lazy_teardown(function()
helpers.stop_kong(nil, true)
end)

before_each(function()
client = assert(helpers.admin_client())
end)

after_each(function()
if client then
client:close()
end
end)

it("the maximum safe integer can be accurately represented as a decimal number", function()
local res = assert(client:send {
method = "POST",
path = "/config",
body = {
config = [[
_format_version: "1.1"
services:
- name: my-service
id: 0855b320-0dd2-547d-891d-601e9b38647f
url: https://localhost
routes:
- name: my-route
id: 481a9539-f49c-51b6-b2e2-fe99ee68866c
paths:
- /
regex_priority: 9007199254740992
]],
},
headers = {
["Content-Type"] = "application/json"
}
})

assert.response(res).has.status(201)
local res = client:get("/routes/481a9539-f49c-51b6-b2e2-fe99ee68866c")
assert.res_status(200, res)
assert.match_re(res:read_body(), "9007199254740992")
end)
end)
end
end
25 changes: 25 additions & 0 deletions t/01-pdk/08-response/11-exit.t
Original file line number Diff line number Diff line change
Expand Up @@ -1155,3 +1155,28 @@ X-test: test
manually setting Transfer-Encoding. Ignored.



=== TEST 45: response.exit() json encoding of numbers with a precision of 16 decimals
--- http_config eval: $t::Util::HttpConfig
--- config
location = /t {
default_type 'text/test';
access_by_lua_block {
require("kong.globalpatches")()
local PDK = require "kong.pdk"
local pdk = PDK.new()

pdk.response.exit(200, { n = 9007199254740992 })
}
}
--- request
GET /t
--- error_code: 200
--- response_headers_like
Content-Type: application/json; charset=utf-8
--- response_body chop
{"n":9007199254740992}
--- no_error_log
[error]


Loading