Re: [w3c/ServiceWorker] Include imported scripts to byte-check (#1023)

jakearchibald commented on this pull request.



> +      1. If |sourceScript| is a [=classic script=], then:
+          1. If |sourceScript|'s [=source text=] is not a byte-for-byte match with |targetScript|'s [=source text=], return false.
+          1. Let |sourceMap| be |sourceScript|'s [=script resource/imported scripts map=].
+          1. [=map/For each=] |url| → |response| of |sourceMap|:
+              1. Let |request| be a new [=/request=] whose [=request/url=] is |url|, [=request/client=] is |job|'s [=job/client=], [=request/type=] is "<code>script</code>", [=request/destination=] is "<code>script</code>", [=request/parser metadata=] is "<code>not parser-inserted</code>", [=request/synchronous flag=] is set, and whose [=request/use-URL-credentials flag=] is set.
+              1. Set |request|'s [=request/cache mode=] to "<code>no-cache</code>" if any of the following are true:
+                  * |registration|'s [=service worker registration/use cache=] is false.
+                  * |job|'s [=force bypass cache flag=] is set.
+                  * |registration|'s [=last update check time=] is not null and the time difference in seconds calculated by the current time minus |registration|’s [=last update check time=] is greater than 86400.
+              1. Let |targetResponse| be the result of [=fetch|fetching=] |request|.
+              1. If |targetResponse|'s <a for="response" href="https://github.com/whatwg/fetch/issues/376">cache state</a> is not "<code>local</code>", set |registration|’s [=last update check time=] to the current time.
+              1. Let |targetResponse| be |targetResponse|'s [=unsafe response=].
+              1. If |targetResponse|'s [=response/type=] is "<code>error</code>", or |targetResponse|'s [=response/status=] is not an [=ok status=], return false.
+              1. If the result of [=UTF-8 decoding=] |response|'s [=response/body=] is not a byte-for-byte match with the result of [=UTF-8 decoding=] |targetResponse|'s [=response/body=], return false.
+      1. If |sourceScript| is a [=module script=], then:
+          1. If |sourceScript|’s [=module script/module record=]'s \[[ECMAScriptCode]] is not a byte-for-byte match with |targetScript|’s [=module script/module record=]'s \[[ECMAScriptCode]], return false.

Hmm, `[[ECMAScriptCode]]` appears to be a "parse result" according to [the spec](https://tc39.github.io/ecma262/#sec-script-records), but the spec doesn't define what that is 😢 . But since it's the result of parsing it doesn't feel appropriate for a byte-for-byte check.

The way [ModuleDeclarationInstantiation](https://tc39.github.io/ecma262/#sec-moduledeclarationinstantiation) performs "For each ImportEntry" makes me think that there isn't any flattening going on. Maybe HTML's [module map](https://html.spec.whatwg.org/multipage/webappapis.html#concept-settings-object-module-map) is an easier way in, since it's at least a flattened view of the imported scripts - although this may contain dynamically imported items that we're not interested in.

If I'm right that `[[ECMAScriptCode]]` isn't the source text, and [source text](https://html.spec.whatwg.org/multipage/webappapis.html#concept-classic-script-source-text) is only available on classic scripts, we need to store this ourselves.

"Perform the fetch" is called for the top level script, but also each module fetch, so we could use this hook to store our own collection of source texts for byte-by-byte comparison.

@domenic does the above sound right at all? Our intent is to refetch the top-level script and all its imports, and see if any are byte-different to the original.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/ServiceWorker/pull/1023