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

Inconsistency: limits on browser.alarms.create() #422

Open
bershanskiy opened this issue Jul 12, 2023 · 5 comments
Open

Inconsistency: limits on browser.alarms.create() #422

bershanskiy opened this issue Jul 12, 2023 · 5 comments
Labels
inconsistency Inconsistent behavior across browsers spec clarification Needs clarification when specified topic: alarms

Comments

@bershanskiy
Copy link
Member

bershanskiy commented Jul 12, 2023

Background

browser.alarms.create() lets extensions register alarms, without any documented limits on their number and the extension-provided alarm names. All these names have to be stored somewhere, typically in a browser-internal database/registry. In theory, a buggy looped extension which dynamically generates alarm names can end up exceeding the limits of internal database. Less interestingly in practice, but more amusing, a humorous extension may implement an "unlimited" storage without having a storage or unlimitedStorage permission (by scheduling alarms far into the future so they don't go off and then retrieving their names).

Current behavior

I tested the following:

  • creating many alarms in a loop
  • crating one alarms with a name equal to 10M with chrome.alarms.create('a'.repeat(10_000_000), {when: 1_000_000_000_000_000});

Chromium

Chromium 117 limits each extension to 500 alarms, throwing an error 'An extension cannot have more than 500 active alarms.' if extension attempts to create 501-th alarm.
Chromium does not limit the length of alarm names. In particular, I created an alarm with name equal to 100MB.

Firefox

Firefox does not limit the number of alarms created. If too many alarms are created, then the offending extension hangs entirely (since event loop is swamped), the browser UI becomes unresponsive (or very badly responsive), and even if user closes the window process does not exit, pegging one CPU core at 100% utilization. User has to kill the process manually.
Firefox does not limit the length of alarm names. In particular, I created an alarm with name equal to 100MB.

Desired behavior

I believe all browsers should have clear limits on the amount of data stored by alarms API, and ideally these limits would be identical across all browsers.

Proposed limits:

  • extensions can store no more than 500 alarms at a time (500 is OK, 501 is an error). This behavior matches Chromium limit which was picked based on real-world test
  • extensions can name alarms with names no more than 50(?) characters in length. I just made up this limit, to get the discussion going.
@hanguokai
Copy link
Member

Yes, Chrome announced the max-500 limit. I think it is Ok in practice.

Considering that the number has been limited, the name length is basically not a problem. Considering compatibility and less harm, it should be as large as possible, for example 1000.

@bershanskiy
Copy link
Member Author

bershanskiy commented Jul 17, 2023

Hi, any sufficiently permissive length limit would be fine, as long as it actually exists. My main concern is that currently there is no limit at all and hence it is trivial to accidentally cause large writes to browser storage and hence overwhelm the internal DB. 500 keys each 1000 chars long would still be about 50KB of data which is manageable (really it becomes a problem only when keys get to GBs of data).

@dotproto dotproto added inconsistency Inconsistent behavior across browsers spec clarification Needs clarification when specified and removed needs-triage labels Jul 20, 2023
@xeenon
Copy link
Collaborator

xeenon commented Jul 20, 2023

Safari does not have a size or name length limit.

@zombie
Copy link
Collaborator

zombie commented Jul 20, 2023

We're supportive of aligning with limits here, (500 alarms) as according to Google's stats it covers the 99.9% percentile. Similarly, the max length of say 1000 characters also sounds reasonable.

@zombie
Copy link
Collaborator

zombie commented Jul 20, 2023

I filed a Firefox bug to track this: https://bugzilla.mozilla.org/show_bug.cgi?id=1844605

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inconsistency Inconsistent behavior across browsers spec clarification Needs clarification when specified topic: alarms
Projects
None yet
Development

No branches or pull requests

5 participants