whatwg / urlpattern

URL Pattern Standard
https://urlpattern.spec.whatwg.org/
Other
156 stars 22 forks source link

consider maximum scope size #18

Open wanderview opened 3 years ago

wanderview commented 3 years ago

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.

sisidovski commented 1 year ago

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 commented 1 year ago

To re-characterize the root of my concern:

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 commented 1 year ago

Close this issue, feel free to reopen if needed.

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

wanderview commented 1 year ago

Re-opened per Andrew's last comment.

jeremyroman commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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. https://github.com/WICG/service-worker-static-routing-api/issues/5