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.

Sign up for GitHub

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

Spec: split up response validation into two stages, and fix a bug in it #1175

Merged
merged 5 commits into from
Jun 6, 2024
Merged

Spec: split up response validation into two stages, and fix a bug in it #1175

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ce1fa78
Spec: split up response validation into two stages, and fix a bug in it
May 15, 2024
a2dbb0d
Document WASM behavior.
May 24, 2024
8214851
Elaborate
May 24, 2024
2deb061
Fix link, more precise whitespace.
May 24, 2024
4bedf6b
Fix the exists thing + encoding check
Jun 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 32 additions & 13 deletions spec.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2078,23 +2078,44 @@ The <dfn http-header><code>Ad-Auction-Allowed</code></dfn> HTTP response header
[=structured header=] whose value must be a [=structured header/boolean=].

<div algorithm>
To <dfn>validate fetching response</dfn> given a [=response=] |response|, null, failure, or a
[=byte sequence=] |responseBody|, and a [=string=] |mimeType|:
To <dfn>validate fetching response headers</dfn> given a [=response=] |response|:

1. If |responseBody| is null or failure, return false.
1. If [=header list/getting a structured field value|getting=] [:Ad-Auction-Allowed:] and
"`item`" from |response|'s [=response/header list=] does not return a true value, return false.
"`item`" from |response|'s [=response/header list=] does not return a true value, return false.
1. If |response|'s [=response/status=] is not an [=ok status=], return false.
1. Return true.
</div>

<div algorithm>
To <dfn>validate fetching response mime and body</dfn> given a [=response=] |response|, null,
failure, or a [=byte sequence=] |responseBody|, and a [=string=] |mimeType|:

1. If |responseBody| is null or failure, return false.
1. Let |headerMimeType| be the result of [=header list/extracting a MIME type=] from |response|'s
[=response/header list=].
1. Return false if any of the following conditions hold:
* |headerMimeType| is failure;
* |mimeType| is "`text/javascript`" and |headerMimeType| is not a [=JavaScript MIME type=];
* |mimeType| is "`application/json`" and |headerMimeType| is not a [=JSON MIME type=].
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|
is not [=UTF-8=] encoded;
* |mimeTypeCharset| is "us-ascii", and not all bytes in |responseBody| are [=ASCII bytes=].
* |mimeType| is "`application/wasm`" and |headerMimeType|'s [=MIME type/essence=] is not
qingxinwu marked this conversation as resolved.
Show resolved Hide resolved
"`application/wasm`".
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|
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem done.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might help if I commit and push....

is not [=UTF-8=] encoded;
* |mimeTypeCharset| is "us-ascii", and not all bytes in |responseBody| are [=ASCII bytes=].
1. Return true.
</div>

<div algorithm>
To <dfn>validate fetching response</dfn> given a [=response=] |response|, null, failure, or a
[=byte sequence=] |responseBody|, and a [=string=] |mimeType|:

1. If the result of [=validating fetching response headers=] given |response| is false, then
return false.
1. If the result of [=validating fetching response mime and body=] given |response|,
|responseBody|, |mimeType| is false, the return false.
morlovich marked this conversation as resolved.
Show resolved Hide resolved
1. Return true.
</div>

Expand Down Expand Up @@ -2159,10 +2180,8 @@ To <dfn>fetch WebAssembly</dfn> given a [=URL=] |url|:
1. Let |moduleObject| be null.
1. [=Fetch=] |request| with [=fetch/processResponseConsumeBody=] set to the following steps given
a [=response=] |response| and null, failure, or a [=byte sequence=] |responseBody|:
1. Set |moduleObject| to failure and return, if any of the following conditions hold:
* |responseBody| is null or failure;
* [=header list/getting a structured field value|Getting=] [:Ad-Auction-Allowed:] and "`item`"
from |response|'s [=response/header list=] does not return a true value.
1. If [=validate fetching response=] with |response|, |responseBody| and "`application/wasm`"
returns false, set |moduleObject| to failure and return.
1. Let |module| be the result of [=compiling a WebAssembly module=] |response|.
1. If |module| is [=error=], set |moduleObject| to failure.
1. Otherwise, set |moduleObject| to |module|.
Expand Down
Loading