Merge branch 'master' into feat/graphql-query-cost

master rewrote the query depth walk to memoise each fragment and to
reject fragment cycles (#13809); this branch had moved the same walk
behind query_depth to make room for the cost strategies. Keep both:
node_depth is master's, and query_depth carries the memo and the cycle
report through to the access phase, which still answers 400 with the
same message.

Author: AlinsRan

apisix/plugins/graphql-limit-count.lua

@@ -173,41 +173,54 @@ local check_graphql_request = {
 -- Returns the maximum selection nesting depth of the GraphQL query AST.
 -- Fragment spreads are expanded in place using the provided fragment map;
 -- inline fragments are treated as transparent wrappers over their selections.
--- The visited table guards against fragment definition cycles.
-local function node_depth(node, fragments, visited)
+-- visited tracks the current recursion path: a spread back onto it is a
+-- fragment cycle (invalid GraphQL), flagged via cycle.found so the caller can
+-- reject the request instead of trusting a traversal-order-dependent depth.
+-- memo caches each fragment's resolved depth so a fragment is measured once,
+-- keeping the traversal linear in the document size.
+local function node_depth(node, fragments, visited, memo, cycle)
     if type(node) ~= "table" then
         return 0
     end
 
     if node.kind == "fragmentSpread" then
         local name = node.name and node.name.value
-        if not name or visited[name] then
+        if not name then
             return 0
         end
+        if visited[name] then
+            cycle.found = true
+            return 0
+        end
+        local cached = memo[name]
+        if cached then
+            return cached
+        end
         local frag = fragments[name]
         if not frag or not frag.selectionSet then
             return 0
         end
         visited[name] = true
-        local depth = node_depth(frag.selectionSet.selections, fragments, visited)
+        local depth = node_depth(frag.selectionSet.selections, fragments, visited, memo, cycle)
         visited[name] = nil
+        memo[name] = depth
         return depth
     end
 
     if node.kind == "inlineFragment" then
         if not node.selectionSet then
             return 0
         end
-        return node_depth(node.selectionSet.selections, fragments, visited)
+        return node_depth(node.selectionSet.selections, fragments, visited, memo, cycle)
     end
 
     local depth = 0
     for k, v in pairs(node) do
         local child
         if k == "selections" then
-            child = 1 + node_depth(v, fragments, visited)
+            child = 1 + node_depth(v, fragments, visited, memo, cycle)
         else
-            child = node_depth(v, fragments, visited)
+            child = node_depth(v, fragments, visited, memo, cycle)
         end
         depth = max(depth, child)
     end
@@ -216,13 +229,24 @@ local function node_depth(node, fragments, visited)
 end
 
 
+-- Returns the depth, or nil plus the log line and the client message when the
+-- document is not valid GraphQL to begin with.
 local function query_depth(operations, fragments)
     local depth = 0
+    local memo = {}
+    local cycle = {found = false}
     for _, op in ipairs(operations) do
-        depth = max(depth, node_depth(op, fragments, {}))
+        depth = max(depth, node_depth(op, fragments, {}, memo, cycle))
     end
 
-    return max(depth, 1)
+    if cycle.found then
+        return nil, "invalid graphql request: fragment spreads form a cycle",
+               "Invalid graphql request: fragment spreads must not form cycles"
+    end
+
+    depth = max(depth, 1)
+    core.log.info("graphql node depth: ", depth)
+    return depth
 end
 
 
@@ -334,12 +358,16 @@ function _M.access(conf, ctx)
         end
     end
 
-    local raw_cost
-    raw_cost, err = raw_query_cost(conf, ctx, operations, fragments, variables)
+    local raw_cost, client_msg
+    raw_cost, err, client_msg = raw_query_cost(conf, ctx, operations, fragments, variables)
     if not raw_cost then
-        core.log.error("failed to compute the graphql query cost: ", err)
-        return 400, {message = "Invalid graphql request: failed to introspect the " ..
-                               "upstream graphql schema"}
+        -- a malformed document reports its own message; anything else is the
+        -- introspection failing
+        core.log.error(client_msg and err
+                       or "failed to compute the graphql query cost: " .. err)
+        return 400, {message = client_msg
+                               or "Invalid graphql request: failed to introspect the "
+                                  .. "upstream graphql schema"}
     end
 
     -- The +0.01 floor makes a query whose nodes are all undecorated still cost 1.

t/plugin/graphql-limit-count.t

@@ -507,17 +507,19 @@ X-RateLimit-Remaining: 16
 
 
 
-=== TEST 22: fragment cycle does not cause infinite recursion
+=== TEST 22: fragment cycle is rejected
 --- request
 POST /hello
 {
   "query": "query { user { ...CycleA } } fragment CycleA on User { id { ...CycleA } }"
 }
 --- more_headers
 Content-Type: application/json
---- error_code: 200
---- response_headers_like
-X-RateLimit-Remaining: \d+
+--- error_code: 400
+--- error_log
+invalid graphql request: fragment spreads form a cycle
+--- response_body eval
+qr/fragment spreads must not form cycles/
 
 
 
@@ -633,3 +635,115 @@ Content-Type: application/json
 --- error_code: 200
 --- response_headers
 X-RateLimit-Remaining: 15
+
+
+
+=== TEST 27: set route: nested fragment expansion test
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                    "plugins": {
+                        "graphql-limit-count": {
+                            "count": 100,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr",
+                            "show_limit_quota_header": true
+                        }
+                    },
+                    "upstream": {
+                        "nodes": {
+                            "127.0.0.1:1980": 1
+                        },
+                        "type": "roundrobin"
+                    },
+                    "uri": "/hello"
+                }]]
+                )
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 28: deeply reused fragments are measured once and keep their depth
+--- config
+    location /t {
+        content_by_lua_block {
+            -- f0 nests three field levels (depth 3); every fN spreads the
+            -- previous fragment twice, so a naive expansion is O(2^N) while
+            -- the result stays depth 3. A completed request with the exact
+            -- quota proves the traversal is both linear and depth-correct.
+            local http = require "resty.http"
+            local n = 34
+            local parts = {"fragment f0 on Query { a { b { c } } }"}
+            for i = 1, n do
+                parts[#parts + 1] = string.format(
+                    "fragment f%d on Query { ...f%d ...f%d }", i, i - 1, i - 1)
+            end
+            parts[#parts + 1] = string.format("query { ...f%d }", n)
+            local body = table.concat(parts, " ")
+            local httpc = http.new()
+            local res, err = httpc:request_uri(
+                "http://127.0.0.1:" .. ngx.var.server_port .. "/hello", {
+                    method = "POST",
+                    body = body,
+                    headers = { ["Content-Type"] = "application/graphql" },
+                })
+            if not res then
+                ngx.say(err)
+                return
+            end
+            ngx.say(res.status)
+            ngx.say(res.headers["X-RateLimit-Remaining"])
+        }
+    }
+--- request
+GET /t
+--- timeout: 10
+--- response_body
+200
+97
+
+
+
+=== TEST 29: mutually recursive fragments are rejected
+--- request
+POST /hello
+{
+  "query": "query { ...A } fragment A on Query { x { ...B } } fragment B on Query { y { ...A } }"
+}
+--- more_headers
+Content-Type: application/json
+--- error_code: 400
+--- error_log
+invalid graphql request: fragment spreads form a cycle
+--- response_body eval
+qr/fragment spreads must not form cycles/
+
+
+
+=== TEST 30: mutual recursion reached at a different depth and order is rejected
+--- request
+POST /hello
+{
+  "query": "query { ...B } fragment A on Query { ...B } fragment B on Query { m { n { ...A } } }"
+}
+--- more_headers
+Content-Type: application/json
+--- error_code: 400
+--- error_log
+invalid graphql request: fragment spreads form a cycle
+--- response_body eval
+qr/fragment spreads must not form cycles/