Skip to content

Conversation

@biredel
Copy link

@biredel biredel commented Jan 20, 2026

Share creation was unlimited before #50905 setting to 20/600s. That proved to be too low for (integration tests, and more crucially) some legitimate human actions, e.g. chat messages with attachments in Talk.

Checklist

  • What is partially reverted here was already manually exempt from testing
  • No font-end changes
  • Documentation update not required
  • Backports requested where applicable (ex: critical bugfixes)
  • Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
  • Milestone added for target branch/version (ex: 32.x for stable32)

Was unlimited before nextcloud#50905 setting to 20/600s which proved to be too low for some legitimate human actions, e.g. chat messages with attachments in Talk. Fix that by bumping to some arbitrary higher value, until some clearer justification for having it lower or higher is determined.

Signed-off-by: biredel <67849440+biredel@users.noreply.github.com>
@biredel biredel requested a review from a team as a code owner January 20, 2026 16:41
@biredel biredel requested review from Altahrim, ArtificialOwl, icewind1991 and leftybournes and removed request for a team January 20, 2026 16:41
@susnux susnux requested a review from nickvergessen January 20, 2026 17:21
@susnux susnux added this to the Nextcloud 33 milestone Jan 20, 2026
*/
#[NoAdminRequired]
#[UserRateLimit(limit: 20, period: 600)]
#[UserRateLimit(limit: 200, period: 600)]
Copy link
Member

Choose a reason for hiding this comment

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

Maximum I can see in a code base would be 40 / 600

If you really need more, that's instance specific and you can use the ratelimit overwrite to change it in your installation:
https://github.com/nextcloud/server/blob/master/config/config.sample.php#L476-L498

Copy link
Author

@biredel biredel Jan 23, 2026

Choose a reason for hiding this comment

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

What purpose would failing the cheap 3rd operation serve, after permitting the expensive ones (upload+thumbnail)? This was originally to protect the server from overload, no?

Maximum I can see in a code base would be 40 / 600

Copied into PR, but warrants further discussion.

The scenario where I noticed this is people in a group chat taking a bunch of photos and pasting them into the chat. Because each attachments is shared separately even when multiple are selected in one operation, the new limit on share creation acts as a per-file and not per-message limit. Changing it to use folders would allow for lower limits than what I suggest here without affecting legitimate human usage; but that did not look quite as trivial to implement in the spreed app, so some stop-gap is needed.

no goal stated; just copied from review comment

Signed-off-by: biredel <67849440+biredel@users.noreply.github.com>
@biredel biredel changed the title fix(files_sharing): rate limit share creation 200 times per 10 minutes fix(files_sharing): rate limit share creation 40 times per 10 minutes Jan 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Default share creation rate limit breaks typical human usage, e.g. in 429 in Talk app

3 participants