diff --git a/lib/resty/openapi_validator/body.lua b/lib/resty/openapi_validator/body.lua index c8052cd..03ce7df 100644 --- a/lib/resty/openapi_validator/body.lua +++ b/lib/resty/openapi_validator/body.lua @@ -344,6 +344,14 @@ function _M.validate(route, body_str, content_type, opts) opts = opts or {} local errs = {} + -- Normalize non-string content_type (e.g. cjson.null sentinel — which is + -- userdata and truthy in Lua, so naive `and content_type` checks would + -- let it through and crash inside str_lower) to nil so downstream code + -- can treat it uniformly with "header absent". + if type(content_type) ~= "string" then + content_type = nil + end + if route.body_required then if body_str == nil or body_str == "" then tab_insert(errs, errors.new("body", nil, "request body is required")) diff --git a/lib/resty/openapi_validator/normalize.lua b/lib/resty/openapi_validator/normalize.lua index f83d1dd..e5df37b 100644 --- a/lib/resty/openapi_validator/normalize.lua +++ b/lib/resty/openapi_validator/normalize.lua @@ -4,6 +4,7 @@ local _M = {} +local cjson = require("cjson.safe") local type = type local pairs = pairs local ipairs = ipairs @@ -28,16 +29,48 @@ local function normalize_30_schema(schema, warnings) -- For nullable schemas with enum or const, we cannot simply inject -- cjson.null into enum (jsonschema can't handle userdata in enum). - -- Use anyOf: [original_schema_without_nullable, {type: "null"}] + -- Use anyOf: [original_schema_without_nullable, {type: "null"}]. + -- + -- IMPORTANT: if the original enum already contains cjson.null (i.e. + -- the spec author wrote `enum: [..., null]`), the userdata sentinel + -- silently disables the entire enum check inside api7/jsonschema. + -- Strip null entries from the enum here — the {type: "null"} branch + -- of the anyOf wrapper already permits null. if schema.enum or schema["const"] then - -- save and remove nullable-related fields, wrap in anyOf local original = {} for k, v in pairs(schema) do if k ~= "nullable" then original[k] = v end end - -- clear schema and replace with anyOf + -- A const that is exactly `null` means "must be null", so the + -- nullable wrapper collapses to just `{type:"null"}` — there's + -- no remaining non-null branch to keep. + local null_only_const = original["const"] == cjson.null + if null_only_const then + for k in pairs(schema) do schema[k] = nil end + schema.type = "null" + return + end + -- Otherwise, strip any null entries from the enum. The + -- `{type:"null"}` branch added below already permits null, + -- and leaving cjson.null inside an enum array silently + -- disables the entire enum check inside api7/jsonschema. + if type(original.enum) == "table" then + local cleaned = {} + for _, val in ipairs(original.enum) do + if val ~= cjson.null then + tab_insert(cleaned, val) + end + end + -- An enum containing only `null` similarly collapses. + if #cleaned == 0 and not original["const"] then + for k in pairs(schema) do schema[k] = nil end + schema.type = "null" + return + end + original.enum = cleaned + end for k in pairs(schema) do schema[k] = nil end diff --git a/lib/resty/openapi_validator/params.lua b/lib/resty/openapi_validator/params.lua index 2a60b83..6ca39dc 100644 --- a/lib/resty/openapi_validator/params.lua +++ b/lib/resty/openapi_validator/params.lua @@ -287,11 +287,28 @@ local function deserialize_param(raw_value, param, query_args) local items_schema = schema.items or {} local values + -- Coerce table values into a single delimited string for the + -- delimiter-based styles. This happens in real callers when, e.g., + -- ngx.req.get_uri_args returns `{"a","b"}` for `?fields=a&fields=b` + -- but the schema is declared `style:form, explode:false` (comma- + -- separated). Previously this crashed in split() with + -- "bad argument #1 to 'str_find' (string expected, got table)". + local function coerce_to_string(delim) + if type(raw_value) == "table" then + local out = {} + for _, v in ipairs(raw_value) do + tab_insert(out, tostring(v)) + end + return table.concat(out, delim) + end + return raw_value + end + if style == "simple" then - values = split(raw_value, ",") + values = split(coerce_to_string(","), ",") elseif style == "form" then if not explode then - values = split(raw_value, ",") + values = split(coerce_to_string(","), ",") else if type(raw_value) == "table" then values = raw_value @@ -300,9 +317,9 @@ local function deserialize_param(raw_value, param, query_args) end end elseif style == "pipeDelimited" then - values = split(raw_value, "|") + values = split(coerce_to_string("|"), "|") elseif style == "spaceDelimited" then - values = split(raw_value, " ") + values = split(coerce_to_string(" "), " ") else values = { raw_value } end diff --git a/lib/resty/openapi_validator/router.lua b/lib/resty/openapi_validator/router.lua index 2a30d42..e6217dd 100644 --- a/lib/resty/openapi_validator/router.lua +++ b/lib/resty/openapi_validator/router.lua @@ -36,6 +36,72 @@ local function convert_path(path_template) end +-- Detect path templates whose segments mix a {var} with literal text or +-- multiple {var}s in the same segment, e.g. "/foo/{id}.json" or +-- "/baz/{name}.{ext}". radixtree's :param syntax cannot extract these +-- correctly because it consumes the entire `/`-bounded segment as one +-- variable; the literal suffix is silently dropped from both the captured +-- value and the param name. For such templates we fall back to a per-route +-- PCRE that re-extracts path params at match time. +local function has_mixed_segment(path_template) + for seg in str_gmatch(path_template, "/([^/]*)") do + local has_var = str_find(seg, "{", 1, true) ~= nil + if has_var then + -- a clean segment is exactly "{name}" with nothing else + if not (str_byte(seg, 1) == str_byte("{") + and str_byte(seg, #seg) == str_byte("}") + and (str_find(seg, "}", 2, true) == #seg)) then + return true + end + end + end + return false +end + + +local PCRE_META = { + ["%"] = true, ["."] = true, ["*"] = true, ["+"] = true, ["?"] = true, + ["("] = true, [")"] = true, ["["] = true, ["]"] = true, ["{"] = true, + ["}"] = true, ["|"] = true, ["^"] = true, ["$"] = true, ["\\"] = true, + ["/"] = true, +} + +local function pcre_escape(s) + return (str_gsub(s, ".", function(c) + if PCRE_META[c] then return "\\" .. c end + end)) +end + + +-- Build a PCRE pattern + ordered name list that extracts path params from +-- a request path matching `path_template`. Used as a fallback when +-- has_mixed_segment(path_template) is true. +local function build_param_pcre(path_template) + local names = {} + local out = {} + local i = 1 + while i <= #path_template do + local lb = str_find(path_template, "{", i, true) + if not lb then + tab_insert(out, pcre_escape(sub_str(path_template, i))) + break + end + if lb > i then + tab_insert(out, pcre_escape(sub_str(path_template, i, lb - 1))) + end + local rb = str_find(path_template, "}", lb + 1, true) + if not rb then + tab_insert(out, pcre_escape(sub_str(path_template, lb))) + break + end + tab_insert(names, sub_str(path_template, lb + 1, rb - 1)) + tab_insert(out, "([^/]+?)") + i = rb + 1 + end + return "^" .. table.concat(out) .. "$", names +end + + -- Extract param names from {param} in path template. local function extract_param_names(path_template) local names = {} @@ -152,6 +218,11 @@ function _M.new(spec) local route_id = 0 for path_template, path_item in pairs(paths) do local param_names = extract_param_names(path_template) + local mixed = has_mixed_segment(path_template) + local param_pcre, pcre_names + if mixed then + param_pcre, pcre_names = build_param_pcre(path_template) + end for method, operation in pairs(path_item) do local m = str_upper(method) @@ -166,6 +237,9 @@ function _M.new(spec) route_metadata[id] = { path_template = path_template, param_names = param_names, + param_pcre = param_pcre, + pcre_names = pcre_names, + base_paths = base_paths, method = m, operation = operation, params = params, @@ -240,6 +314,40 @@ function _M.match(self, method, path) path_params[name] = matched["_" .. name] end + -- Fallback: when the template has mixed segments (e.g. "/foo/{id}.json" + -- or "/baz/{name}.{ext}"), radixtree can match the route but cannot + -- extract the variables (it consumes the whole `/`-bounded segment as + -- one param and silently drops the literal suffix). Re-extract the + -- params here using a per-route PCRE built from the template. + if route.param_pcre then + local re_match = ngx.re.match + local m + local bases = route.base_paths or { "" } + for _, base in ipairs(bases) do + local rel = path + if base ~= "" then + if str_find(path, base, 1, true) == 1 then + rel = sub_str(path, #base + 1) + if rel == "" then rel = "/" end + else + rel = nil + end + end + if rel then + local mm = re_match(rel, route.param_pcre, "jo") + if mm then m = mm; break end + end + end + if not m then + -- radixtree matched but our authoritative regex doesn't: + -- treat as no match so callers get a clean error. + return nil, nil + end + for i, name in ipairs(route.pcre_names or {}) do + path_params[name] = m[i] + end + end + return route, path_params end diff --git a/t/conformance/test_qa_bugs_v103.lua b/t/conformance/test_qa_bugs_v103.lua new file mode 100644 index 0000000..07d9f16 --- /dev/null +++ b/t/conformance/test_qa_bugs_v103.lua @@ -0,0 +1,246 @@ +#!/usr/bin/env resty +--- Regression tests for the bugs surfaced by the v1.0.3 multi-spec QA pass. +-- See qa/lua-resty-openapi-validator-v1.0.3.md (in api7ee workspace). +dofile("t/lib/test_bootstrap.lua") + +local T = require("test_helper") +local cjson = require("cjson.safe") +local ov = require("resty.openapi_validator") + +local function compile(spec) + local v, err = ov.compile(cjson.encode(spec)) + assert(v, "compile failed: " .. tostring(err)) + return v +end + +-- Bug 1: path templates with literal extension (e.g. /users/{id}.json) used +-- to be silently misrouted by lua-resty-radixtree, which treated the whole +-- segment ":{id}.json" as a single param named "{id}.json". The validator now +-- detects mixed segments at compile time and re-extracts path params with PCRE. +T.describe("Bug 1: path with literal .json extension after param", function() + local v = compile({ + openapi = "3.0.0", + info = { title = "t", version = "0" }, + paths = { + ["/users/{id}.json"] = { + get = { + parameters = { + { + ["in"] = "path", name = "id", required = true, + schema = { type = "string", minLength = 1 }, + }, + }, + responses = { ["200"] = { description = "ok" } }, + }, + }, + }, + }) + local ok, err = v:validate_request({ method = "GET", path = "/users/abc.json" }) + T.ok(ok, "valid id extracted: " .. tostring(err)) + + -- and the param value really came through (not nil/empty under the wrong name) + local ok2 = v:validate_request({ method = "GET", path = "/users/x.json" }) + T.ok(ok2, "single-char id 'x' accepted by minLength=1") +end) + +-- Bug 1b: dotted suffix shouldn't be greedy-matched +T.describe("Bug 1b: param value must not consume the literal extension", function() + local v = compile({ + openapi = "3.0.0", + info = { title = "t", version = "0" }, + paths = { + ["/files/{name}.txt"] = { + get = { + parameters = { + { + ["in"] = "path", name = "name", required = true, + schema = { type = "string", pattern = "^[a-z]+$" }, + }, + }, + responses = { ["200"] = { description = "ok" } }, + }, + }, + }, + }) + local ok, err = v:validate_request({ method = "GET", path = "/files/report.txt" }) + T.ok(ok, "name 'report' (not 'report.txt') extracted: " .. tostring(err)) +end) + +-- Bug 2: nullable + enum with explicit null in the enum list used to +-- silently disable the enum check inside api7/jsonschema (cjson.null +-- userdata leaked into the cloned schema). After normalization, any string +-- could pass as a "valid enum" value. +T.describe("Bug 2: nullable + enum with null still enforces enum", function() + local v = compile({ + openapi = "3.0.0", + info = { title = "t", version = "0" }, + paths = { + ["/x"] = { + post = { + requestBody = { + required = true, + content = { + ["application/json"] = { + schema = { + type = "object", + properties = { + status = { + type = "string", + nullable = true, + enum = { "free", "paid", cjson.null }, + }, + }, + required = { "status" }, + }, + }, + }, + }, + responses = { ["200"] = { description = "ok" } }, + }, + }, + }, + }) + local ok = v:validate_request({ + method = "POST", path = "/x", + content_type = "application/json", + body = '{"status":"free"}', + }) + T.ok(ok, "free is in enum") + + local ok2 = v:validate_request({ + method = "POST", path = "/x", + content_type = "application/json", + body = '{"status":null}', + }) + T.ok(ok2, "null is allowed via nullable") + + local ok3, err3 = v:validate_request({ + method = "POST", path = "/x", + content_type = "application/json", + body = '{"status":"bogus"}', + }) + T.ok(not ok3, "bogus is NOT in enum, must be rejected") + -- with nullable, schema is wrapped as anyOf [original, {type:null}], so + -- the error wording mentions "matches none of the required" rather than + -- "enum"; either way, the request must be rejected (the regression here + -- was silent acceptance). + T.ok(err3 and #err3 > 0, "error message present: " .. tostring(err3)) +end) + +-- Bug 4: array-style query params with delimiter styles used to crash when +-- the raw value arrived as a Lua table (multi-value form input). Now coerced +-- to a delimiter-joined string before splitting. +T.describe("Bug 4: pipeDelimited param with table raw value does not crash", function() + local v = compile({ + openapi = "3.0.0", + info = { title = "t", version = "0" }, + paths = { + ["/q"] = { + get = { + parameters = { + { + ["in"] = "query", name = "ids", required = true, + style = "pipeDelimited", explode = false, + schema = { type = "array", items = { type = "string" } }, + }, + }, + responses = { ["200"] = { description = "ok" } }, + }, + }, + }, + }) + local ok, err = v:validate_request({ + method = "GET", path = "/q", + query = { ids = { "a|b", "c" } }, + }) + T.ok(ok, "table raw value handled: " .. tostring(err)) +end) + +-- Bug 5: cjson.null in content_type (e.g. caller passed a parsed JSON value +-- through verbatim) is userdata, which is truthy in Lua. The body validator +-- entered the content-type branch and crashed. Now normalized to nil at the +-- top of body.validate so all downstream sites (find_body_schema_for_content_type, +-- is_json_content_type, ...) treat it like an absent header. +T.describe("Bug 5: cjson.null content_type does not crash (with body present)", function() + local v = compile({ + openapi = "3.0.0", + info = { title = "t", version = "0" }, + paths = { + ["/p"] = { + post = { + requestBody = { + required = false, + content = { + ["application/json"] = { + schema = { type = "object" }, + }, + }, + }, + responses = { ["200"] = { description = "ok" } }, + }, + }, + }, + }) + -- Non-empty body so the body validator actually evaluates content_type + -- (an empty body short-circuits before the content-type branch). + local pcall_ok, ok, err = pcall(v.validate_request, v, { + method = "POST", path = "/p", + content_type = cjson.null, + body = '{"hello":"world"}', + }) + T.ok(pcall_ok, "cjson.null content_type didn't crash: " .. tostring(ok)) + -- With content_type normalized to nil, treated as no declared CT match; + -- since the request has a body but the validator can't find a schema for + -- the (absent) content-type, it should not crash. Outcome (ok or not) is + -- secondary; the regression is the crash. + T.ok(ok ~= nil, "validator returned a value, not a crash: ok=" .. tostring(ok) .. " err=" .. tostring(err)) +end) + +-- Bug 2b: nullable + const = null collapses to just {type:"null"}, which +-- means anything other than null must be rejected. +T.describe("Bug 2b: nullable + const=null only accepts null", function() + local v = compile({ + openapi = "3.0.0", + info = { title = "t", version = "0" }, + paths = { + ["/c"] = { + post = { + requestBody = { + required = true, + content = { + ["application/json"] = { + schema = { + type = "object", + properties = { + v = { + type = "string", + nullable = true, + ["const"] = cjson.null, + }, + }, + required = { "v" }, + }, + }, + }, + }, + responses = { ["200"] = { description = "ok" } }, + }, + }, + }, + }) + local ok = v:validate_request({ + method = "POST", path = "/c", + content_type = "application/json", + body = '{"v":null}', + }) + T.ok(ok, "null is accepted (only allowed value)") + + local ok2 = v:validate_request({ + method = "POST", path = "/c", + content_type = "application/json", + body = '{"v":"anything"}', + }) + T.ok(not ok2, "non-null rejected when const is null") +end) + +T.done()