membphis commented on PR #13840:
URL: https://github.com/apache/apisix/pull/13840#issuecomment-5325573561
Reviewed head: `d4483b2495d2f072841cadd16546bfedaee88a57`
I found three P1 merge blockers:
### [P1] Fragment DAGs can make query-cost computation expand exponentially
`each_field` recursively expands every `fragmentSpread`. `state.visiting`
prevents cycles, but it neither memoizes nor bounds acyclic repeated spreads. A
compact acyclic chain in which each fragment spreads the previous fragment
twice causes exponential work in the access phase before rate limiting is
applied. Please memoize fragment cost using the necessary traversal state, or
enforce a strict expanded-node/work limit and reject oversized expansion with a
4xx response. Add a regression proving that an acyclic repeated-fragment DAG
has bounded processing cost.
### [P1] Fragment type conditions do not update the schema type cursor
Both `inlineFragment` and `fragmentSpread` recurse with the parent
`sel_type`; the fragment `typeCondition` is ignored. For an interface or union
query, `... on Product { expensiveField }` therefore still looks up the field
under the parent abstract type, so a `Product.expensiveField` decoration can
fail to match and the query is undercharged. Please update the type cursor from
`typeCondition.name.value` when entering either fragment form, validate
compatibility, and add tests where the parent abstract type differs from the
concrete decorated type.
### [P1] Default variable handling lets clients bypass argument-based cost
`resolve_variables` defaults to `false`, and variable nodes are treated as
missing arguments. A client can move the same fan-out value from `first: 10000`
to `first: $n` and avoid the configured `add_arguments` or `mul_arguments`,
causing `max_cost` and quota charging to underestimate the request. Please make
variable resolution the safe default for the new cost strategies, or
conservatively reject/charge unresolved decorated variables. Add
literal-versus-variable equivalence tests for both cost strategies.
--
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]
