- From: Domenic Denicola <notifications@github.com>
- Date: Mon, 10 Jul 2023 20:07:46 -0700
- To: w3c/ServiceWorker <ServiceWorker@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/ServiceWorker/pull/1686/review/1523327167@github.com>
@domenic requested changes on this pull request.
> +
+ {{InstallEvent/registerRouter(rules)}} registers this service worker the rules to offload simple tasks that the fetch handler does.
+
+ <dfn method for="InstallEvent"><code>registerRouter(|rules|)</code></dfn> method *must* run these steps:
+
+ 1. Let |routerRules| be a list of {{RouterRule}} dictionaries.
+ 1. If |rules| is a {{RouterRule}} dictionary:
+ 1. Run [=VerifyRouterRule=] algorithm with |rules|.
+ 1. Append |rules| to |routerRules|.
+ 1. If |rules| is a sequence of {{RouterRule}} dictionaries.
+ 1. for each |rule| in |rules|:
+ 1. If running [=VerifyRouterRule=] algorithm with |rule| returns false, <a>throw</a> a <code>TypeError</code>.
+ 1. Append |rule| to |routerRules|.
+ 1. Set |routerRules| to [=/service worker=]'s [=static router rules object=].
+
+ <section>
I guess this should probably move to https://pr-preview.s3.amazonaws.com/yoshisatoyanagisawa/ServiceWorker/pull/1686.html#algorithms
> + <dfn method for="InstallEvent"><code>registerRouter(|rules|)</code></dfn> method *must* run these steps:
+
+ 1. Let |routerRules| be a list of {{RouterRule}} dictionaries.
+ 1. If |rules| is a {{RouterRule}} dictionary:
+ 1. Run [=VerifyRouterRule=] algorithm with |rules|.
+ 1. Append |rules| to |routerRules|.
+ 1. If |rules| is a sequence of {{RouterRule}} dictionaries.
+ 1. for each |rule| in |rules|:
+ 1. If running [=VerifyRouterRule=] algorithm with |rule| returns false, <a>throw</a> a <code>TypeError</code>.
+ 1. Append |rule| to |routerRules|.
+ 1. Set |routerRules| to [=/service worker=]'s [=static router rules object=].
+
+ <section>
+ <h5 id="verify-router-rule-algorithm"><dfn>VerifyRouterRule</dfn></h5>
+ : Input
+ :: |rule|, RouterRule
```suggestion
:: |rule|, a {{RouterRule}}
```
> + 1. Run [=VerifyRouterRule=] algorithm with |rules|.
+ 1. Append |rules| to |routerRules|.
+ 1. If |rules| is a sequence of {{RouterRule}} dictionaries.
+ 1. for each |rule| in |rules|:
+ 1. If running [=VerifyRouterRule=] algorithm with |rule| returns false, <a>throw</a> a <code>TypeError</code>.
+ 1. Append |rule| to |routerRules|.
+ 1. Set |routerRules| to [=/service worker=]'s [=static router rules object=].
+
+ <section>
+ <h5 id="verify-router-rule-algorithm"><dfn>VerifyRouterRule</dfn></h5>
+ : Input
+ :: |rule|, RouterRule
+ : Output
+ :: a boolean
+
+ 1. If |rule|'s |condition| is empty, and abort these steps.
To reference dictionary members, use this syntax:
```
|rule|["{{RouterRule/condition}}"]
```
However, this step can be deleted, since `condition` is marked as required.
> + 1. Append |rules| to |routerRules|.
+ 1. If |rules| is a sequence of {{RouterRule}} dictionaries.
+ 1. for each |rule| in |rules|:
+ 1. If running [=VerifyRouterRule=] algorithm with |rule| returns false, <a>throw</a> a <code>TypeError</code>.
+ 1. Append |rule| to |routerRules|.
+ 1. Set |routerRules| to [=/service worker=]'s [=static router rules object=].
+
+ <section>
+ <h5 id="verify-router-rule-algorithm"><dfn>VerifyRouterRule</dfn></h5>
+ : Input
+ :: |rule|, RouterRule
+ : Output
+ :: a boolean
+
+ 1. If |rule|'s |condition| is empty, and abort these steps.
+ 1. If |rule|'s |condition|'s |urlPattern| is empty, return false..
What do you mean by "is empty"?
If you want to require that the IDL member be present, use `required` in the IDL and delete this step.
If you want to require that the value is not the empty string, then check "is the empty string" or `'s [=string/length=] is 0`.
> + 1. If |rules| is a sequence of {{RouterRule}} dictionaries.
+ 1. for each |rule| in |rules|:
+ 1. If running [=VerifyRouterRule=] algorithm with |rule| returns false, <a>throw</a> a <code>TypeError</code>.
+ 1. Append |rule| to |routerRules|.
+ 1. Set |routerRules| to [=/service worker=]'s [=static router rules object=].
+
+ <section>
+ <h5 id="verify-router-rule-algorithm"><dfn>VerifyRouterRule</dfn></h5>
+ : Input
+ :: |rule|, RouterRule
+ : Output
+ :: a boolean
+
+ 1. If |rule|'s |condition| is empty, and abort these steps.
+ 1. If |rule|'s |condition|'s |urlPattern| is empty, return false..
+ 1. If |urlPattern| contains a regular expression, return false.
We need to define this more rigorously somehow. How does it work in the code?
> + 1. If |rules| is a sequence of {{RouterRule}} dictionaries.
+ 1. for each |rule| in |rules|:
+ 1. If running [=VerifyRouterRule=] algorithm with |rule| returns false, <a>throw</a> a <code>TypeError</code>.
+ 1. Append |rule| to |routerRules|.
+ 1. Set |routerRules| to [=/service worker=]'s [=static router rules object=].
+
+ <section>
+ <h5 id="verify-router-rule-algorithm"><dfn>VerifyRouterRule</dfn></h5>
+ : Input
+ :: |rule|, RouterRule
+ : Output
+ :: a boolean
+
+ 1. If |rule|'s |condition| is empty, and abort these steps.
+ 1. If |rule|'s |condition|'s |urlPattern| is empty, return false..
+ 1. If |urlPattern| contains a regular expression, return false.
The |urlPattern| variable is not declared anywhere.
> + 1. Set |routerRules| to [=/service worker=]'s [=static router rules object=].
+
+ <section>
+ <h5 id="verify-router-rule-algorithm"><dfn>VerifyRouterRule</dfn></h5>
+ : Input
+ :: |rule|, RouterRule
+ : Output
+ :: a boolean
+
+ 1. If |rule|'s |condition| is empty, and abort these steps.
+ 1. If |rule|'s |condition|'s |urlPattern| is empty, return false..
+ 1. If |urlPattern| contains a regular expression, return false.
+
+ Note: Since running a user-defined regular expression has a security concern, it is prohibited.
+
+ 1. If |urlPattern| cannot be [=urlpattern/parsed=], return false.
This is not appropriately rigorous. Given a string (e.g. `|rule|["{{RouterRule/condition}}"]["{{RouterCondition/urlPattern}}"]`) we can then feed it to the [parse a pattern string](https://wicg.github.io/urlpattern/#parse-a-pattern-string) algorithm, or maybe the [parse a constructor string](https://wicg.github.io/urlpattern/#parse-a-constructor-string) algorithm. Which one do you want?
You then will get a return value from that. What kind of return value do you consider "cannot be parsed"?
Finally, you probably will want to store the result of parsing, so that you can execute the pattern later.
> + <section>
+ <h5 id="verify-router-rule-algorithm"><dfn>VerifyRouterRule</dfn></h5>
+ : Input
+ :: |rule|, RouterRule
+ : Output
+ :: a boolean
+
+ 1. If |rule|'s |condition| is empty, and abort these steps.
+ 1. If |rule|'s |condition|'s |urlPattern| is empty, return false..
+ 1. If |urlPattern| contains a regular expression, return false.
+
+ Note: Since running a user-defined regular expression has a security concern, it is prohibited.
+
+ 1. If |urlPattern| cannot be [=urlpattern/parsed=], return false.
+ 1. If |rule|'s |source| is empty, return false.
+ 1. If |rule|'s |source| is not "network", return false.
Both of these conditions are impossible because of the Web IDL layer's `required` and `enum` features.
> + <section>
+ <h4 id="register-router-method">{{InstallEvent/registerRouter(rules)|event.registerRouter(rules)}}</h4>
+
+ {{InstallEvent/registerRouter(rules)}} registers this service worker the rules to offload simple tasks that the fetch handler does.
+
+ <dfn method for="InstallEvent"><code>registerRouter(|rules|)</code></dfn> method *must* run these steps:
+
+ 1. Let |routerRules| be a list of {{RouterRule}} dictionaries.
+ 1. If |rules| is a {{RouterRule}} dictionary:
+ 1. Run [=VerifyRouterRule=] algorithm with |rules|.
+ 1. Append |rules| to |routerRules|.
+ 1. If |rules| is a sequence of {{RouterRule}} dictionaries.
+ 1. for each |rule| in |rules|:
+ 1. If running [=VerifyRouterRule=] algorithm with |rule| returns false, <a>throw</a> a <code>TypeError</code>.
+ 1. Append |rule| to |routerRules|.
+ 1. Set |routerRules| to [=/service worker=]'s [=static router rules object=].
I think this is backward.
> @@ -3177,6 +3245,19 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/
1. Return |response|.
</section>
+ <section algorithm>
+ <h3 id="get-router-source-algorithm"><dfn>GetRouterSource</dfn></h3>
+ : Input
+ :: |rules|, a list of {{RouterRule}}
+ :: |request|, a [=/request=]
+ : Output
+ :: {{RouterSourceEnum}} or null
+
+ 1. For each |rule| in |rules|:
+ 1. If |request|'s [=request/URL=] [=urlpattern/matches=] |rule|'s {{urlPattern}}, then returns |rule|'s {{RouterRule/source}}.
rule's urlPattern (better written as `|rule|["{{RouterRule/condition}}"]["{{RouterCondition/urlPattern}}"]`) is a string. https://wicg.github.io/urlpattern/#match takes a `URLPattern` object, not a string. This is related to my issue above about needing to store the parsing result somewhere.
(Probably, you will need a separate hierarchy of parsed router rule [structs](https://infra.spec.whatwg.org/#structs), separate from the raw `RouterRule` dictionaries.)
--
Reply to this email directly or view it on GitHub:
https://github.com/w3c/ServiceWorker/pull/1686#pullrequestreview-1523327167
You are receiving this because you are subscribed to this thread.
Message ID: <w3c/ServiceWorker/pull/1686/review/1523327167@github.com>
Received on Tuesday, 11 July 2023 03:07:53 UTC