Explicitly set comment diff fragments as javascript MIME type

Review Request #7432 — Created June 17, 2015 and submitted

Information

Review Board
master
9c7ce94...

Reviewers

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)

reviewbotreviewbot

Col: 80 E501 line too long (84 > 79 characters)

reviewbotreviewbot

Both arguments should be aligned. If they don't fit when positioned right after the (, then you can align them …

chipx86chipx86

Col: 13 E128 continuation line under-indented for visual indent

reviewbotreviewbot
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
    
    
  2. reviewboard/reviews/views.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (84 > 79 characters)
    
  3. 
      
PE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
    
    
  2. reviewboard/reviews/views.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (84 > 79 characters)
    
  3. 
      
PE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
    
    
  2. reviewboard/reviews/views.py (Diff revision 3)
     
     
    Show all issues
    Col: 13
     E128 continuation line under-indented for visual indent
    
  3. 
      
chipx86
  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?

    1. You should only notice this problem if the web server has nosniff set. I only noticed this on Chrome. Internet Explorer apparently does this too, but I did not try.

  2. reviewboard/reviews/views.py (Diff revision 3)
     
     
     
    Show all issues

    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=...)
    
  3. 
      
PE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
    
    
  2. 
      
brennie
  1. Hi, thanks for the patch!

    Please mark issues fixed as you fix them. That way we know what to expect when re-reviewing. We also cannot land this patch with open issues.

  2. 
      
PE
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.0.x (47112e5)