shreemaan-abhishek commented on code in PR #14154:
URL: https://github.com/apache/apisix/pull/14154#discussion_r4476562533
##########
apisix/plugins/openapi-to-mcp/tools/handler.lua:
##########
@@ -124,19 +124,72 @@ local function apply_query_defaults(tool, query)
end
+-- A path parameter fills part of one segment of the operation's path, so its
+-- value cannot carry a separator. A "/" goes out as "%2F", which a server that
+-- accepts an encoded separator decodes back into one, and the call would then
+-- address a path the operation does not describe. This reads the value as it
+-- arrived, because the encoding hides the separator from the finished path.
+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
+
+
+-- The segments of a path, with the empty one a leading or trailing slash
+-- leaves: "/items/x" is { "", "items", "x" }.
+local function path_segments(path)
+ local segments = {}
+ for segment in str_gmatch(path .. "/", "([^/]*)/") do
+ segments[#segments + 1] = segment
+ end
+ return segments
+end
+
+
+-- A server receiving the request resolves "." and ".." away, but only where
+-- one is a whole segment. A template expression may fill part of a segment,
+-- and "{name}.json" filled with ".." comes out as "...json", which addresses
+-- nothing else, so the value alone does not say whether it moves the request.
+-- The finished path does.
+--
+-- Only the segments an expression helped build are checked: one the template
+-- contributes by itself is not the caller's to shift. Rejecting a separator
+-- above keeps both segment lists the same length, so they line up by index.
+local function resolved_away(segment)
+ return segment == "" or segment == "." or segment == ".."
+end
+
+
local function build_path(template, path_params)
local path = template
for name, value in pairs(path_params) do
+ local text = tostring(value)
+ if has_separator(text) then
Review Comment:
openapi-to-mcp forwards LLM-controlled arguments using configured upstream
credentials and cannot know the upstream’s decoding policy, rejecting / and \
is a reasonable security default but it is stricter than ordinary OpenAPI
client behavior. The best compatibility design would be default rejection with
an explicit opt-in for APIs known to preserve encoded separators safely.
--
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]
