Skip to content

Commit ff9db5c

Browse files
authored
fix: address QA defects found by 10-spec public-API corpus (#8)
1 parent bf4c423 commit ff9db5c

5 files changed

Lines changed: 419 additions & 7 deletions

File tree

lib/resty/openapi_validator/body.lua

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,14 @@ function _M.validate(route, body_str, content_type, opts)
344344
opts = opts or {}
345345
local errs = {}
346346

347+
-- Normalize non-string content_type (e.g. cjson.null sentinel — which is
348+
-- userdata and truthy in Lua, so naive `and content_type` checks would
349+
-- let it through and crash inside str_lower) to nil so downstream code
350+
-- can treat it uniformly with "header absent".
351+
if type(content_type) ~= "string" then
352+
content_type = nil
353+
end
354+
347355
if route.body_required then
348356
if body_str == nil or body_str == "" then
349357
tab_insert(errs, errors.new("body", nil, "request body is required"))

lib/resty/openapi_validator/normalize.lua

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
local _M = {}
66

7+
local cjson = require("cjson.safe")
78
local type = type
89
local pairs = pairs
910
local ipairs = ipairs
@@ -28,16 +29,48 @@ local function normalize_30_schema(schema, warnings)
2829

2930
-- For nullable schemas with enum or const, we cannot simply inject
3031
-- cjson.null into enum (jsonschema can't handle userdata in enum).
31-
-- Use anyOf: [original_schema_without_nullable, {type: "null"}]
32+
-- Use anyOf: [original_schema_without_nullable, {type: "null"}].
33+
--
34+
-- IMPORTANT: if the original enum already contains cjson.null (i.e.
35+
-- the spec author wrote `enum: [..., null]`), the userdata sentinel
36+
-- silently disables the entire enum check inside api7/jsonschema.
37+
-- Strip null entries from the enum here — the {type: "null"} branch
38+
-- of the anyOf wrapper already permits null.
3239
if schema.enum or schema["const"] then
33-
-- save and remove nullable-related fields, wrap in anyOf
3440
local original = {}
3541
for k, v in pairs(schema) do
3642
if k ~= "nullable" then
3743
original[k] = v
3844
end
3945
end
40-
-- clear schema and replace with anyOf
46+
-- A const that is exactly `null` means "must be null", so the
47+
-- nullable wrapper collapses to just `{type:"null"}` — there's
48+
-- no remaining non-null branch to keep.
49+
local null_only_const = original["const"] == cjson.null
50+
if null_only_const then
51+
for k in pairs(schema) do schema[k] = nil end
52+
schema.type = "null"
53+
return
54+
end
55+
-- Otherwise, strip any null entries from the enum. The
56+
-- `{type:"null"}` branch added below already permits null,
57+
-- and leaving cjson.null inside an enum array silently
58+
-- disables the entire enum check inside api7/jsonschema.
59+
if type(original.enum) == "table" then
60+
local cleaned = {}
61+
for _, val in ipairs(original.enum) do
62+
if val ~= cjson.null then
63+
tab_insert(cleaned, val)
64+
end
65+
end
66+
-- An enum containing only `null` similarly collapses.
67+
if #cleaned == 0 and not original["const"] then
68+
for k in pairs(schema) do schema[k] = nil end
69+
schema.type = "null"
70+
return
71+
end
72+
original.enum = cleaned
73+
end
4174
for k in pairs(schema) do
4275
schema[k] = nil
4376
end

lib/resty/openapi_validator/params.lua

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,11 +287,28 @@ local function deserialize_param(raw_value, param, query_args)
287287
local items_schema = schema.items or {}
288288
local values
289289

290+
-- Coerce table values into a single delimited string for the
291+
-- delimiter-based styles. This happens in real callers when, e.g.,
292+
-- ngx.req.get_uri_args returns `{"a","b"}` for `?fields=a&fields=b`
293+
-- but the schema is declared `style:form, explode:false` (comma-
294+
-- separated). Previously this crashed in split() with
295+
-- "bad argument #1 to 'str_find' (string expected, got table)".
296+
local function coerce_to_string(delim)
297+
if type(raw_value) == "table" then
298+
local out = {}
299+
for _, v in ipairs(raw_value) do
300+
tab_insert(out, tostring(v))
301+
end
302+
return table.concat(out, delim)
303+
end
304+
return raw_value
305+
end
306+
290307
if style == "simple" then
291-
values = split(raw_value, ",")
308+
values = split(coerce_to_string(","), ",")
292309
elseif style == "form" then
293310
if not explode then
294-
values = split(raw_value, ",")
311+
values = split(coerce_to_string(","), ",")
295312
else
296313
if type(raw_value) == "table" then
297314
values = raw_value
@@ -300,9 +317,9 @@ local function deserialize_param(raw_value, param, query_args)
300317
end
301318
end
302319
elseif style == "pipeDelimited" then
303-
values = split(raw_value, "|")
320+
values = split(coerce_to_string("|"), "|")
304321
elseif style == "spaceDelimited" then
305-
values = split(raw_value, " ")
322+
values = split(coerce_to_string(" "), " ")
306323
else
307324
values = { raw_value }
308325
end

lib/resty/openapi_validator/router.lua

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,72 @@ local function convert_path(path_template)
3636
end
3737

3838

39+
-- Detect path templates whose segments mix a {var} with literal text or
40+
-- multiple {var}s in the same segment, e.g. "/foo/{id}.json" or
41+
-- "/baz/{name}.{ext}". radixtree's :param syntax cannot extract these
42+
-- correctly because it consumes the entire `/`-bounded segment as one
43+
-- variable; the literal suffix is silently dropped from both the captured
44+
-- value and the param name. For such templates we fall back to a per-route
45+
-- PCRE that re-extracts path params at match time.
46+
local function has_mixed_segment(path_template)
47+
for seg in str_gmatch(path_template, "/([^/]*)") do
48+
local has_var = str_find(seg, "{", 1, true) ~= nil
49+
if has_var then
50+
-- a clean segment is exactly "{name}" with nothing else
51+
if not (str_byte(seg, 1) == str_byte("{")
52+
and str_byte(seg, #seg) == str_byte("}")
53+
and (str_find(seg, "}", 2, true) == #seg)) then
54+
return true
55+
end
56+
end
57+
end
58+
return false
59+
end
60+
61+
62+
local PCRE_META = {
63+
["%"] = true, ["."] = true, ["*"] = true, ["+"] = true, ["?"] = true,
64+
["("] = true, [")"] = true, ["["] = true, ["]"] = true, ["{"] = true,
65+
["}"] = true, ["|"] = true, ["^"] = true, ["$"] = true, ["\\"] = true,
66+
["/"] = true,
67+
}
68+
69+
local function pcre_escape(s)
70+
return (str_gsub(s, ".", function(c)
71+
if PCRE_META[c] then return "\\" .. c end
72+
end))
73+
end
74+
75+
76+
-- Build a PCRE pattern + ordered name list that extracts path params from
77+
-- a request path matching `path_template`. Used as a fallback when
78+
-- has_mixed_segment(path_template) is true.
79+
local function build_param_pcre(path_template)
80+
local names = {}
81+
local out = {}
82+
local i = 1
83+
while i <= #path_template do
84+
local lb = str_find(path_template, "{", i, true)
85+
if not lb then
86+
tab_insert(out, pcre_escape(sub_str(path_template, i)))
87+
break
88+
end
89+
if lb > i then
90+
tab_insert(out, pcre_escape(sub_str(path_template, i, lb - 1)))
91+
end
92+
local rb = str_find(path_template, "}", lb + 1, true)
93+
if not rb then
94+
tab_insert(out, pcre_escape(sub_str(path_template, lb)))
95+
break
96+
end
97+
tab_insert(names, sub_str(path_template, lb + 1, rb - 1))
98+
tab_insert(out, "([^/]+?)")
99+
i = rb + 1
100+
end
101+
return "^" .. table.concat(out) .. "$", names
102+
end
103+
104+
39105
-- Extract param names from {param} in path template.
40106
local function extract_param_names(path_template)
41107
local names = {}
@@ -152,6 +218,11 @@ function _M.new(spec)
152218
local route_id = 0
153219
for path_template, path_item in pairs(paths) do
154220
local param_names = extract_param_names(path_template)
221+
local mixed = has_mixed_segment(path_template)
222+
local param_pcre, pcre_names
223+
if mixed then
224+
param_pcre, pcre_names = build_param_pcre(path_template)
225+
end
155226

156227
for method, operation in pairs(path_item) do
157228
local m = str_upper(method)
@@ -166,6 +237,9 @@ function _M.new(spec)
166237
route_metadata[id] = {
167238
path_template = path_template,
168239
param_names = param_names,
240+
param_pcre = param_pcre,
241+
pcre_names = pcre_names,
242+
base_paths = base_paths,
169243
method = m,
170244
operation = operation,
171245
params = params,
@@ -240,6 +314,40 @@ function _M.match(self, method, path)
240314
path_params[name] = matched["_" .. name]
241315
end
242316

317+
-- Fallback: when the template has mixed segments (e.g. "/foo/{id}.json"
318+
-- or "/baz/{name}.{ext}"), radixtree can match the route but cannot
319+
-- extract the variables (it consumes the whole `/`-bounded segment as
320+
-- one param and silently drops the literal suffix). Re-extract the
321+
-- params here using a per-route PCRE built from the template.
322+
if route.param_pcre then
323+
local re_match = ngx.re.match
324+
local m
325+
local bases = route.base_paths or { "" }
326+
for _, base in ipairs(bases) do
327+
local rel = path
328+
if base ~= "" then
329+
if str_find(path, base, 1, true) == 1 then
330+
rel = sub_str(path, #base + 1)
331+
if rel == "" then rel = "/" end
332+
else
333+
rel = nil
334+
end
335+
end
336+
if rel then
337+
local mm = re_match(rel, route.param_pcre, "jo")
338+
if mm then m = mm; break end
339+
end
340+
end
341+
if not m then
342+
-- radixtree matched but our authoritative regex doesn't:
343+
-- treat as no match so callers get a clean error.
344+
return nil, nil
345+
end
346+
for i, name in ipairs(route.pcre_names or {}) do
347+
path_params[name] = m[i]
348+
end
349+
end
350+
243351
return route, path_params
244352
end
245353

0 commit comments

Comments
 (0)