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

consider maximum scope size #18

Open
wanderview opened this issue Oct 12, 2020 · 9 comments
Open

consider maximum scope size #18

wanderview opened this issue Oct 12, 2020 · 9 comments
Labels
topic: service worker Related to integration of URL patterns and service workers

Comments

@wanderview
Copy link
Member

At the TPAC 2020 discussion @asutherland suggested that we should consider maximum scope size limits. While its theoretically possible for sites to stick large amounts of info encoded in the scope URL today, the scope list mechanism might encourage further size growth. We should have an interoperable agreement as to what is too much.

@wanderview wanderview added the topic: service worker Related to integration of URL patterns and service workers label Aug 12, 2021
@sisidovski
Copy link
Collaborator

The infra doc says we should not enforce specific limits on algorithm inputs with regards to their size, resource usage, or equivalent. https://infra.spec.whatwg.org/#algorithm-limits

Close this issue, feel free to reopen if needed.

@asutherland
Copy link

asutherland commented Sep 12, 2023

To re-characterize the root of my concern:

  • ServiceWorker registration matching is on the critical path for navigations.
  • This can bias browsers to potentially need to keep all ServiceWorker registrations loaded at all times which can create practical resource considerations.

Note that this is specifically about the potential use of URLPattern by ServiceWorker registrations and since URLPattern is now a proposed primitive that exists outside of the ServiceWorkers spec, not all of the concerns are relevant to URLPattern other than if we wanted to impose limits on ServiceWorker registrations, URLPattern might need to expose the necessary primitive like a computed structured serialization size.

There are 2 concerns that I think I had relating to this, at least the first of which could be relevant for URLPattern on its own:

  1. This can lead to arbitrary implementation-defined limits that can create webcompat problems if all implementations are not in alignment. The section 3.2 that you cite does say "It can also be useful to constrain an implementation-defined limit with a lower limit. I.e., ensuring all implementations can handle inputs of a given minimum size." which seems like a reasonable approach to ensuring interop. It seems appropriate for the URLPattern tests to ensure that all implementations support a minimum size. https://chromium.googlesource.com/chromium/src/+/main/docs/security/url_display_guidelines/url_display_guidelines.md#url-length says that chromium limits URL display to "32 kB". Maybe this is a reasonable minimum value so that URLPattern is a reliable pattern in all the places it is used? edit: To clarify, the limit might be enforced as the sum of all lengths of the USVString values of the URLPatternInit dictionary.
  2. There's a potentially perverse incentive for sites to store extra data in their registration to optimize their own load times versus using the available async storage APIs available to ServiceWorkers. Reasonable upper bounds potentially head off sites adopting such anti-patterns and requiring browser implementations to respond reactively like only loading the registrations for sites with "heavy" registrations on demand. An upside of addressing the previous point by having tests enforce a generous minimum lower bound potentially does free up browser implementations to establish upper bounds to this end.

@asutherland
Copy link

Close this issue, feel free to reopen if needed.

(For clarity, I don't have the ability to do this.)

@wanderview
Copy link
Member Author

Re-opened per Andrew's last comment.

@jeremyroman
Copy link
Collaborator

Do these limits belong in the URL Pattern specification? It seems like they might be specific to the use case and so could be specified in those specifications (in this case, the Service Worker spec, or whichever spec describes using URL patterns to define Service Worker scopes).

@wanderview
Copy link
Member Author

Sure. This original issue is a holdover from the original explainer that proposed a service worker enhancement together with URLPattern. Maybe this issue should be ported over to the service worker static routing area (if applicable to that design). Or if its not an issue for static routing we could close this for now since the patterned scope effort is not being worked on.

@asutherland
Copy link

asutherland commented Sep 13, 2023

I'm fine for this issue to migrate to the static routing proposal. I can re-file there if its authors prefer versus them filing a more pragmatic, smaller issue. I know my comment above is a little dense; I'm also fine with a more terse issue of "enforce a minimum supported static route size" which translates to "have a WPT test that adds several routes with some long strings in it that should not fail". That is, the structured serialization sizing thing is a huge ask, just having a pragmatic test with big strings is fine.

@jeremyroman
Copy link
Collaborator

jeremyroman commented Sep 14, 2023

I think it probably makes sense to move this issue to the applicable downstream spec (where one exists). I could imagine bringing up sort of common minimum limits for consistency, perhaps as part of a future enhancement to #182.

Thanks for your patience. :)

@sisidovski
Copy link
Collaborator

Thanks for elaborating on that. Understood the root concerns. Not sure if 32KB is a reasonable number since that is the displayed size in the Chrome’s omnibox, but 2MB is maybe too large?

bringing up sort of common minimum limits for consistency, perhaps as part of a future enhancement to #182.

That makes sense to me.

Also, I filed the issue on the static routing side. WICG/service-worker-static-routing-api#5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: service worker Related to integration of URL patterns and service workers
Development

No branches or pull requests

4 participants