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.

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

Add X-Content-Type-Options: nosniff header to download_stash.py #41037

Merged

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented Jul 14, 2023

The page serves the "text/html" Content-Type, which triggers sniffing in CFNetwork. As a result, CFNetwork will read 512 bytes before telling the client about the network response.

In the context of this test, 512 bytes means CFNetwork finishes the whole load before even notifying the client of the response and thus allowing the client to cancel. This means that CFNetwork clients like WebKit on Apple platforms finish the load and that the token gets set, even if the client rejected the download and tried to cancel the load.

Add the X-Content-Type-Options: nosniff to disable sniffing, to make sure that the network response doesn't get delayed and so that clients have a chance to cancel the load before it completes.

@cdumez
Copy link
Contributor Author

cdumez commented Jul 14, 2023

Note that https://mimesniff.spec.whatwg.org/#determining-the-computed-mime-type-of-a-resource says to sniff when the content-type is text/html so I am a bit surprised the tests using this script are passing in Chrome and Firefox.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Tracked in whatwg/mimesniff#173 for further investigation.

Comment on lines 8 to 9
// Make sure to disable sniffing because it would read enough bytes to finish
// the load, before even giving the client application a chance to cancel it.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Make sure to disable sniffing because it would read enough bytes to finish
// the load, before even giving the client application a chance to cancel it.
# Make sure to disable sniffing because it would read enough bytes to finish
# the load, before even giving the client application a chance to cancel it.

The page serves the "text/html" Content-Type, which triggers sniffing in
CFNetwork. As a result, CFNetwork will read up to 512 bytes before telling
the client about the network response.

In the context of this test, up to 512 bytes means CFNetwork finishes the
whole load before even notifying the client of the response and thus allowing
the client to cancel. This means that CFNetwork clients like WebKit on
Apple platforms finish the load and that the token gets set, even if the
client rejected the download and tried to cancel the load.

Add the `X-Content-Type-Options: nosniff` to disable sniffing, to make
sure that the network response doesn't get delayed and so that clients
have a chance to cancel the load before it completes.
@cdumez cdumez force-pushed the download_stash_add_nosniff_header branch from aa21682 to cf0df3b Compare July 14, 2023 03:54
@cdumez cdumez enabled auto-merge (squash) July 14, 2023 04:00
@cdumez cdumez merged commit fe4dd46 into web-platform-tests:master Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants