Skip to content

Don't limit page size to 50 or add assumedSize when requiring paging boundaries with cost analysis #8144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cmeeren opened this issue Mar 18, 2025 · 1 comment

Comments

@cmeeren
Copy link
Contributor

cmeeren commented Mar 18, 2025

Product

Hot Chocolate

Version

15.0.3

Link to minimal reproduction

See zip below

Steps to reproduce

Repro solution: HotChocolateBugRepro.zip

If I add [UsePaging(RequirePagingBoundaries = true, AllowBackwardPagination = false)] to a field, the schema contains this directive on that field:

@listSize(
  assumedSize: 50
  slicingArguments: ["first"]
  slicingArgumentDefaultValue: 10
  sizedFields: ["edges", "nodes"]
)

As per the cost spec, since requireOneSlicingArgument is omitted, it is by default true (which matches RequirePagingBoundaries = true in the attribute call).

However, there are two issues:

  1. Given the above, I don't understand what assumedSize does there: The list has the size specified by the client in first (which is required). There is no need for assumedSize; it's just confusing.
  2. Furthermore, I have not specified a max page size, but HotChocolate uses 50 (if I request 51 items, I get a HC0051 error). According to cost analysis, I can request much more than 50 elements without exceeding the cost limits. HotChocolate should not require a max number of items in this case, but instead rely on the cost analysis to block requests with too many items (the exact number would depend on the sub-field selection, i.e., on the cost of what you request for each item). I can only make this work by explicitly setting MaxPageSize to e.g. Int32.MaxValue in the attribute call (or ConfigurePaging), but then my GraphQL spec will be full of assumedSize: 2147483647, which is ugly and confusing.

What is expected?

  1. This spec:
@listSize(
  slicingArguments: ["first"]
  slicingArgumentDefaultValue: 10
  sizedFields: ["edges", "nodes"]
)
  1. Not limiting the query to at most 50 items.

What is actually happening?

  1. This spec:
@listSize(
  assumedSize: 50
  slicingArguments: ["first"]
  slicingArgumentDefaultValue: 10
  sizedFields: ["edges", "nodes"]
)
  1. The query is limited to 50 items.

Relevant log output

Additional context

No response

@michaelstaib
Copy link
Member

michaelstaib commented Mar 26, 2025

This is not a bug—it’s actually two entirely different concerns.

Hot Chocolate will always enforce a maximum page size. Relying solely on query cost analysis can lead to several issues. One important aspect to understand is that Hot Chocolate uses pooled memory for completed values. For example, if a pooled list exceeds a capacity of 500 items, it will abandon pooling, which impacts memory efficiency. In such cases, you’d need to reconfigure memory pooling to accommodate higher capacities.

Additionally, query cost analysis can be disabled on a per-request basis, so the maximum page size remains the final safeguard. If you truly want to override this, you can set the max page size to int.MaxValue. However, we won’t change the paging mechanism to rely on a security feature that is both optional and not the recommended approach to securing GraphQL APIs. Even when using trusted documents, we strongly advise against having unbounded pagination.

Regarding point 1: When first is specified as a variable, we fall back to the assumed size because the actual value is unknown during static analysis. Even though the variable is provided, its value isn’t available at analysis time, so complexity must be computed based on assumptions.

As for slicingArgumentDefaultValue, one could argue that with RequirePagingBoundaries enabled, the default value should never apply. But it’s worth noting that this behavior is a non-standard addition to the IBM cost spec.

If anything ....

@listSize(
  assumedSize: 50
  slicingArguments: ["first"]
  sizedFields: ["edges", "nodes"]
)

... would be expected in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants