shreemaan-abhishek commented on code in PR #14154:
URL: https://github.com/apache/apisix/pull/14154#discussion_r4479505755
##########
apisix/plugins/openapi-to-mcp/tools/handler.lua:
##########
@@ -124,20 +125,133 @@ local function apply_query_defaults(tool, query)
end
-local function build_path(template, path_params)
+-- A separator in a path parameter is percent-encoded, but servers disagree on
+-- whether to preserve it as data or decode it into path syntax. The secure
+-- default is to reject it; a Route may opt in for an API that deliberately
+-- accepts encoded separators.
+local function has_separator(value)
+ -- core.string.find is a plain search; this is a pattern, so it needs
+ -- string.find itself. A backslash is a separator on some servers.
+ return str_find(value, "[/\\]") ~= nil
+end
+
+
+-- Even with encoded separators enabled, refuse values whose components would
+-- be resolved away after a server decodes them. Empty components cover
leading,
+-- trailing and repeated separators. Both slash spellings are considered: a
+-- backslash is a separator on some servers.
+local function resolved_away(segment)
+ return segment == "" or segment == "." or segment == ".."
+end
+
+
+local function separator_creates_unsafe_component(value)
+ if not has_separator(value) then
+ return false
+ end
+
+ for component in str_gmatch(value .. "/", "([^/\\]*)[/\\]") do
+ if resolved_away(component) then
+ return true
+ end
+ end
+ return false
+end
+
+
+-- Return everything before one of the given URI delimiters. A template
+-- expression name may itself contain a delimiter, so only one outside braces
+-- counts. Parameter values cannot introduce one: escape_uri encodes them.
+local function before_uri_delimiter(value, delimiters)
+ local in_expression = false
+ for index = 1, #value do
+ local char = str_sub(value, index, index)
+ if char == "{" then
+ in_expression = true
+ elseif char == "}" then
+ in_expression = false
+ elseif not in_expression and str_find(delimiters, char, 1, true) then
+ return str_sub(value, 1, index - 1)
+ end
+ end
+ return value
+end
+
+
+local function uri_path(value)
+ return before_uri_delimiter(value, "?#")
+end
+
+
+local function without_fragment(value)
+ return before_uri_delimiter(value, "#")
+end
+
+
+-- The segments of a path, with the empty one a leading or trailing slash
+-- leaves: "/items/x" is { "", "items", "x" }.
+local function path_segments(value)
+ local segments = {}
+ local path = uri_path(value)
+ for segment in str_gmatch(path .. "/", "([^/]*)/") do
Review Comment:
You are right on both counts, and I had the spec backwards. Fixed in 972d9fb.
I had taken the name restriction from RFC 6570's `varname` rule, which does
not apply here:
OAS 3.2.1 derives `path-template` from RFC 3986 section 3.3 and defines the
name as
`1*( %x00-7A / %x7C / %x7E-10FFFF )`, every Unicode character except a
brace. `/` is `%x2F`,
so it is admitted, and the slash prohibition in that section governs
parameter values
instead.
All four rows reproduce, the first one included. `template_segments` now
splits only on a
slash outside braces, with the same scan `before_uri_delimiter` uses for `?`
and `#`. The
substituted path keeps `path_segments` as it was, for the reason you gave: a
separator in a
value is either refused or percent-encoded, so it cannot change the segment
count.
Regressions added for your table, all four failing without the change:
- `/x/{scope/key}/{id}/..` with `safe`, `ok` is sent as `/x/safe/ok/..`,
since the literal
tail is the template's own segment
- `/x/{a/b}/{c}` with `..`, with `""`, and `/x/{a/b}/{c}/y` with `..` are
each refused,
naming `{c}`
- plus a safe pair, to pin that the splitter did not simply start refusing
everything
500 cases in `t/plugin/openapi-to-mcp.t` pass and luacheck is clean. The
same fix is on the
other implementation as c4fc61c.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
