Spec: split up response validation into two stages, and fix a bug in it #1175
Conversation
We will need to do checking in two stages for script bodies to get the proper cross-site seller signals behavior. While at it, make sure we actually check that the response is a 2xx; we are not supposed to be trying to use error pages. Also make the WASM path go through this, too, so it gets this check as well.
|
Ping @qingxinwu? |
spec.bs
Outdated
| "`application/wasm`". | ||
|
|
||
| Note: This was intended to match the behavior of [=compiling a potential WebAssembly | ||
| response=], but diverges by failing to remove leading and trailing HTTP whitespace. |
There was a problem hiding this comment.
Use [=HTTP tab or space=] since that's what [=get, decode, and split=] (called by extract a mime type).
HTTP whitespace contains U+000A LF, U+000D CR, and [=HTTP tab or space=], so it's different.
There was a problem hiding this comment.
In this case, the removal is motivated by https://webassembly.github.io/spec/web-api/#compile-a-potential-webassembly-response step 4 (which probably is motivated by proper parsing)
Changed to http tab or space, thanks.
I am not sure on changing our code simply because we're supposed to be super-conservative about backwards compatibility (and the change can break things by making wasm load when it failed to before), but it is a reasonable thing to consider changing in the code in my book since things that break would break would do so because they were doing something silly, not something reasonable.
spec.bs
Outdated
| 1. If |mimeType| is not "`application/wasm`": | ||
| 1. Let |mimeTypeCharset| be |headerMimeType|'s [=MIME type/parameters=]["`charset`"]. | ||
| 1. Return false if any of the following conditions hold: | ||
| * |mimeTypeCharset| does not [=map/exist=], or |mimeTypeCharset| is "utf-8", and |responseBody| |
There was a problem hiding this comment.
I think it is really weird to use [=map/exist=] anywhere away from the exact location where we pull the entry, even though I can technically understand the intent. For example, in https://infra.spec.whatwg.org/#example-map-get we only ever have map["test"] without "exists" following it after we check "exists" first, so I would try and move the "exists" condition up above to where we try and pull the value out.
There was a problem hiding this comment.
Done. Also noticed this had a logic error in that we don't actually accept things other than utf-8 or us-ascii, and fixed that.
There was a problem hiding this comment.
It might help if I commit and push....
We will need to do checking in two stages for script bodies to get the proper cross-site seller signals behavior.
While at it, make sure we actually check that the response is a 2xx; we are not supposed to be trying to use error pages.
Also fix the charset check, we accept very few.
Make the WASM path go through this, too, so it actually checks its mimetype, and gets the status check as well.
Preview | Diff