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

fix(ReferenceApiController): Bump rate limit for public resolve endpoint #49801

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/Controller/ReferenceApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public function resolveOne(string $reference): DataResponse {
*/
#[ApiRoute(verb: 'GET', url: '/resolvePublic', root: '/references')]
#[PublicPage]
#[AnonRateLimit(limit: 10, period: 120)]
#[AnonRateLimit(limit: 200, period: 120)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[AnonRateLimit(limit: 200, period: 120)]
#[AnonRateLimit(limit: 200, period: 3600)]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would mean that only 200 requests are allowed in 3600 seconds, which means 60 minutes, no? So reloading a document with 150 previews once would already hit the rate limit. Given that we cache the references internally I would prefer a shorter period.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tricky part is that we do not cache invalid responses, so you can still issue a lot more requests, though I find it hard to come up with a reasonable amount there.

We could also think about doing lazy loading on the frontend side there as an additional step to only fetch the ones that are visible in the browsers viewport, but even with that 10 in 120 seconds might be to low for examples like the one shared in the collectives ticket.

public function resolveOnePublic(string $reference, string $sharingToken): DataResponse {
/** @var ?CoreReference $resolvedReference */
$resolvedReference = $this->referenceManager->resolveReference(trim($reference), true, trim($sharingToken))?->jsonSerialize();
Expand Down
Loading