Explicitly set comment diff fragments as javascript MIME type
Review Request #7432 — Created June 17, 2015 and submitted
If the HTTP header 'X-Content-Type-Options' is set to 'nosniff,'
compliant browsers (Google Chrome, for example) will not attempt to
MIME-sniff a response content type different from its declared type.The comment diff fragment currently returns an HTTP response with a
default type of 'text/html,' but it is clearly code intended to be run
as a script of type 'application/javascript.'On latest Google Chrome stable, this results in an error and a failure
to render the comment diff box.Set the content type of this response to 'application/javascript,' to
reflect its true nature.
RB version: 2.0.17
Browser: Google Chrome 43.0.2357.124 (64-bit), Mac OS X.
Server: nginx 1.8.0, configured to serve the 'nosniff' header.Manually inspected response headers before and after patch. The content type is 'text/html' before, and fails to render, instead showing the spinning logo forever. Chrome console reports and error:
Refused to execute script from 'https://test.mydomain.com/r/743/fragments/diff-comments/2246/?queue=diff_fragments&container_prefix=comment_container&1234556' because its MIME type ('text/html') is not executable, and strict MIME type checking is enabled.After this patch, the box renders correctly, and no error is encountered.
| Description | From | Last Updated |
|---|---|---|
|
Col: 80 E501 line too long (84 > 79 characters) |
 reviewbot
|
|
|
Col: 80 E501 line too long (84 > 79 characters) |
reviewbot
|
|
|
Both arguments should be aligned. If they don't fit when positioned right after the (, then you can align them … |
 chipx86
|
|
|
Col: 13 E128 continuation line under-indented for visual indent |
reviewbot
|
- Change Summary:
-
Oops. Fix typo in description.
- Description:
-
If the HTTP header 'X-Content-Type-Options' is set to 'nosniff,'
compliant browsers (Google Chrome, for example) will not attempt to MIME-sniff a response content type different from its declared type. The comment diff fragment currently returns an HTTP response with a
default type of 'text/html,' but it is clearly code intended to be run ~ as a script of type 'application/json.' ~ as a script of type 'application/javascript.' On latest Google Chrome stable, this results in an error and a failure
to render the comment diff box. ~ Set the content type of this response to 'application/json,' to reflect
~ its true nature. ~ Set the content type of this response to 'application/javascript,' to
~ reflect its true nature. - Commit:
-
5c76d8fcc280826a1d973031ca709b7513d9375759d70d1745a41c9f04ef49f4f6c6ac2b2c6cbcea
- Diff:
-
Revision 2 (+1 -1)
- Change Summary:
-
-Fix typo in description.
-Fix 80-character violation. - Commit:
-
59d70d1745a41c9f04ef49f4f6c6ac2b2c6cbceae77b67e568e645e627ed5c665024fcbe3645c1ab
- Diff:
-
Revision 3 (+2 -1)
-
Thanks for the patch! I appreciate the informative description and testing. Only one comment below.
Just to clarify, a default install doesn't exhibit this problem, right? I don't see it with the latest Chrome here, but we don't have nosniff set. I'm mostly trying to gauge whether this will require a quick release or not. I suspect it's not super urgent, but would be nice to get out in the next couple of weeks, ideally?
-
Both arguments should be aligned. If they don't fit when positioned right after the
(, then you can align them as:response = HttpResponse( page_content, content_type=...)
- Change Summary:
-
fix alignment
- Commit:
-
e77b67e568e645e627ed5c665024fcbe3645c1ab9c7ce943c0c04d403a6b7a51c2c283692ae8c8b1
- Diff:
-
Revision 4 (+3 -1)
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py