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

Fetch: test nosniff parsing better #13559

Merged
merged 6 commits into from
Nov 1, 2018
Merged

Fetch: test nosniff parsing better #13559

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3241bd
Fetch: test nosniff parsing better
annevk Oct 17, 2018
7dd984b
test form feeds
annevk Oct 17, 2018
ee3c287
meh, let's just require stripping form feed as well, we already do it…
annevk Oct 17, 2018
18280c2
also test U+000B
annevk Oct 17, 2018
fd3b2fa
use a reusable framework instead
annevk Oct 19, 2018
882ac0b
address feedback and stop treating 0x0C as whitespace
annevk Oct 30, 2018
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
12 changes: 6 additions & 6 deletions fetch/nosniff/parsing-nosniff.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,27 @@
<script src=/resources/testharnessreport.js></script>
<div id=log></div>
<script>
Copy link
Member

Choose a reason for hiding this comment

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

Mind adding a comment, while you're here, on just what this test is doing? It looks to me like an accurate description would be something like:

Tests whether variations of a "X-Content-Type-Options: nosniff" header can be parsed as such. When the response header fails to be parsed as a valid nosniff header, the advertised Content-Type is not recognized as a possible script, and the script's onerror handler is invoked. When the nosniff header is not parsed as a valid nosniff header, the MIME type of the response is sniffed as text, and the onload handler is run.

var fails = ["first", "uppercase"],
passes = ["last", "quoted", "quoted-single", "no-x"]
const fails = ["first", "uppercase", "weird-tokens", "form-feed"],
passes = ["last", "quoted", "quoted-single", "no-x", "first-empty", "first-empty-double", "line-tabulation"];

fails.forEach(function(urlpart) {
async_test(function(t) {
var script = document.createElement("script")
script.onerror = t.step_func_done(function(){})
script.onerror = t.step_func_done()
script.onload = t.unreached_func("Unexpected load event")
script.src = "resources/nosniff-" + urlpart + ".asis"
document.body.appendChild(script)
}, "URL query: " + urlpart)
}, "nosniff parsing (success): " + urlpart)
})

passes.forEach(function(urlpart) {
async_test(function(t) {
var script = document.createElement("script")
script.onerror = t.unreached_func("Unexpected error event")
script.onload = t.step_func_done(function(){})
script.onload = t.step_func_done()
script.src = "resources/nosniff-" + urlpart + ".asis"
document.body.appendChild(script)
}, "URL query: " + urlpart)
}, "nosniff parsing (failure): " + urlpart)
})

</script>
7 changes: 7 additions & 0 deletions fetch/nosniff/resources/nosniff-first-empty-double.asis
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
HTTP/1.1 200 YOU HAVE NO POWER HERE
Content-Length: 22
Content-Type: x/x
X-Content-Type-options:
Copy link
Member

Choose a reason for hiding this comment

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

Is options deliberately lowercase in these tests? Shouldn't hurt, but as that's not what they're aimed at testing, should probably capitalize it.

X-Content-Type-Options: nosniff

// nothing to see here
6 changes: 6 additions & 0 deletions fetch/nosniff/resources/nosniff-first-empty.asis
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
HTTP/1.1 200 YOU HAVE NO POWER HERE
Content-Length: 22
Content-Type: x/x
X-Content-Type-options: ,nosniff

// nothing to see here
6 changes: 6 additions & 0 deletions fetch/nosniff/resources/nosniff-form-feed.asis
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
HTTP/1.1 200 YOU HAVE NO POWER HERE
Content-Length: 22
Content-Type: x/x
X-Content-Type-options: nosniff ,

// nothing to see here
6 changes: 6 additions & 0 deletions fetch/nosniff/resources/nosniff-line-tabulation.asis
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
HTTP/1.1 200 YOU HAVE NO POWER HERE
Content-Length: 22
Content-Type: x/x
X-Content-Type-options: nosniff ,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call this vertical-tab instead of line-tabulation? At least I've always seen it caller vertical tab when looking at ascii tables and the like.

Copy link
Member

Choose a reason for hiding this comment

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

Do we get anything from the comma in this one?


// nothing to see here
6 changes: 6 additions & 0 deletions fetch/nosniff/resources/nosniff-weird-tokens.asis
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
HTTP/1.1 200 YOU HAVE NO POWER HERE
Content-Length: 22
Content-Type: x/x
X-Content-Type-options: nosniff,,@#$#%%&^&^*()()11!
Copy link
Member

Choose a reason for hiding this comment

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

Could also do a weird tokens test with the "@#$#%%&^&^*()()11!" first, to make sure their headers with weird tokens aren't just ignored. Could additionally / alternatively do something similar for the tabulation test as well, adding another nosniff after the comma.


// nothing to see here