Skip to content
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

Constrain the maximum number of results in a query #14917

Closed
0Tech opened this issue Feb 6, 2023 · 6 comments · Fixed by #16041
Closed

Constrain the maximum number of results in a query #14917

0Tech opened this issue Feb 6, 2023 · 6 comments · Fixed by #16041
Assignees

Comments

@0Tech
Copy link
Contributor

0Tech commented Feb 6, 2023

Summary

Query server should be able to constrain the maximum number of results in a single page.

Problem Definition

For now, end-users can request a query with a pagination of any limit. It means a huge size of the response.

This change would not introduce a break-change, and the clients already expect this behavior (refer: https://cloud.google.com/apis/design/design_patterns#list_pagination).

Proposal

  1. Use MaxLimit to constrain the maximum number of results.
  • Add logic to constrain limit in types.query.Paginate() and types.query.CollectionFilteredPaginate().
  1. Change MaxLimit into variable (it's a constant now), so app developers can change the value.
  • It would be better to stop using global.
@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Feb 6, 2023
@alexanderbez
Copy link
Contributor

Hi @0Tech. I'm not understanding the summary or the problem definition. What's wrong with the current pagination mechanism? If limits are kept reasonable, which are set by clients, then how can responses be huge?

@0Tech
Copy link
Contributor Author

0Tech commented Feb 7, 2023

If limits are kept reasonable, which are set by clients, then how can responses be huge?

There are always malicious clients and end users. And we cannot stop them using our services. We could deal with it on other layers (e.g. transport layer), but it would be much nicer to have it on application layer too. Because the server must prepare the response of a huge size prior to transmission, and also gRPC already has the related design.

@tac0turtle
Copy link
Member

This makes sense to me, we should have a max limit of what is possible to get per page.

@tac0turtle tac0turtle added T:Sprint T: Client UX and removed needs-triage Issue that needs to be triaged labels Feb 7, 2023
@alexanderbez
Copy link
Contributor

There are always malicious clients and end users.

Sure, I agree.

And we cannot stop them using our services.

Yes, you can. You can use firewalls.

I'm not opposed to the idea though.

@0Tech
Copy link
Contributor Author

0Tech commented Feb 8, 2023

Yes, you can. You can use firewalls.

You're right. We can always let firewalls replace the limit in the request. Thank you for the correction.

I'm not opposed to the idea though.

😄

@alexanderbez
Copy link
Contributor

The proposal looks good. We'll add it to our roadmap 👍

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

Successfully merging a pull request may close this issue.

3 participants