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
|
PE
Change Summary:
Oops. Fix typo in description.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+1 -1) |
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py -
PE
Change Summary:
-Fix typo in description.
-Fix 80-character violation.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 3 (+2 -1) |
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py -
reviewboard/reviews/views.py (Diff revision 3) Col: 13 E128 continuation line under-indented for visual indent
-
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?
-
reviewboard/reviews/views.py (Diff revision 3) 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=...)
PE
Change Summary:
fix alignment
Commit: |
|
||||
|---|---|---|---|---|---|
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