Opened 12 years ago

Closed 12 years ago

#4547 closed defect (fixed)

[with patch, positive review] The Sage Notebook doesn't specify the Content-Type header in its responses

Reported by: mhansen Owned by: mhansen
Priority: major Milestone: sage-3.4.1
Component: notebook Keywords:
Cc: jason Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

This causes Apache rewriting for example to just display plain text instead of HTML. This hasn't been an issue because browsers are relatively smart about dealing with unspecified Content-Types.

Attachments (1)

trac_4547.patch (34.8 KB) - added by mabshoff 12 years ago.
Update version of mhansen's patch that does pass doctests

Download all attachments as: .zip

Change History (15)

comment:1 Changed 12 years ago by mhansen

  • Summary changed from The Sage Notebook doesn't specify the Content-Type header in its responses to [with patch, needs work] The Sage Notebook doesn't specify the Content-Type header in its responses

I attached a patch (which may or may not apply to the current version) which only fixes the problem for HTML pages. CSS, Javascript, etc. should also be handled.

comment:2 Changed 12 years ago by was

This *has* been an issue. I have been working around by putting this in my httpd.conf:

<VirtualHost *>
        ServerName sagenb.org
        ProxyPass    / http://sagenb.org:8000/
        ProxyPassReverse / http://sagenb.org:8000/

        <Location />
           DefaultType text/html
        </Location>
</VirtualHost>


comment:3 Changed 12 years ago by ddrake

I rebased Mike's patch against 3.3.alpha6, and added a "charset: utf-8" bit; this ticket is in some sense the other half of #5211.

This ticket was originally motivated by Apache rewriting; I'm here because some users have UTF-8 text in their worksheets and browsers are not always figuring out the correct encoding. If my patch fixes the encoding/browser problems, perhaps we should merge this ticket and open another for adding Content-Type headers to CSS, Javascript, and so on.

comment:4 Changed 12 years ago by mabshoff

  • Summary changed from [with patch, needs work] The Sage Notebook doesn't specify the Content-Type header in its responses to [with patch, needs review] The Sage Notebook doesn't specify the Content-Type header in its responses

Since you rebased the patch please change the summary to "needs review" as I just did. Otherwise it will just bitrot since no one will know it is even there :(

Cheers,

Michael

comment:5 Changed 12 years ago by mabshoff

Well, looking around further into the ticket history it seems that additional work is needed. But if this patch does get a positive review I would recommend to open another ticket to deal with CSS and javasscript encoding settings, even though this is somewhat less pressing (who adds utf-8 characters into CSS or javasscript?).

Mike: any take on this?

Cheers,

Michael

comment:6 Changed 12 years ago by jason

  • Cc jason added

comment:7 Changed 12 years ago by ddrake

I've rebased the patch against 3.4.alpha0 -- that's "trac_4547-ddrake.2.patch", which is the only one of the three patches which needs to be applied.

comment:8 Changed 12 years ago by TimothyClemans

  • Summary changed from [with patch, needs review] The Sage Notebook doesn't specify the Content-Type header in its responses to [with patch, positive review] The Sage Notebook doesn't specify the Content-Type header in its responses

apply trac-4547_3.patch

comment:9 Changed 12 years ago by TimothyClemans

Depends on #4135

comment:10 Changed 12 years ago by ddrake

I've noticed that some javascript and css doesn't have content-type headers being sent (but some do); should we open separate tickets for those as well?

Also, Timothy, can you take a look at #5211 as well? That adds a http-equiv header to the html itself, just to make sure browsers really do understand that we're sending utf8.

comment:11 Changed 12 years ago by mhansen

  • Milestone changed from sage-3.4.2 to sage-3.4.1
  • Owner changed from boothby to mhansen
  • Status changed from new to assigned

I've attached trac_4547.patch which applies cleanly against 3.4. Note that this patch DOES NOT depend on #4135.

We should open new tickets for CSS / Javascript / etc.

comment:12 Changed 12 years ago by mabshoff

  • Summary changed from [with patch, positive review] The Sage Notebook doesn't specify the Content-Type header in its responses to [with patch, positive review, needs doctest fix] The Sage Notebook doesn't specify the Content-Type header in its responses

This patch requires two trivial fixes unless these are provided via some other patch as a dependency:

sage-3.4.1.alpha0$ ./sage -t -long devel/sage/sage/server/notebook/twist.py 
sage -t -long "devel/sage/sage/server/notebook/twist.py"    
**********************************************************************
File "/scratch/mabshoff/sage-3.4.1.alpha0/devel/sage/sage/server/notebook/twist.py", line 139:
    sage: response.headers
Expected:
    <Headers: Raw: {'content-type': ['text/html']} Parsed: {'content-type': <RecalcNeeded>}>
Got:
    <Headers: Raw: {'content-type': ['text/html; charset=utf-8']} Parsed: {'content-type': <RecalcNeeded>}>
**********************************************************************
File "/scratch/mabshoff/sage-3.4.1.alpha0/devel/sage/sage/server/notebook/twist.py", line 1511:
    sage: E.render(None)
Expected:
    <twisted.web2.HTMLResponse code=200, streamlen=...>
Got:
    <twisted.web2.http.Response code=200, streamlen=240>
**********************************************************************

Cheers,

Michael

Changed 12 years ago by mabshoff

Update version of mhansen's patch that does pass doctests

comment:13 Changed 12 years ago by mabshoff

  • Summary changed from [with patch, positive review, needs doctest fix] The Sage Notebook doesn't specify the Content-Type header in its responses to [with patch, positive review] The Sage Notebook doesn't specify the Content-Type header in its responses

How on earth can this patch get a positive review when it doesn't even pass doctests for the file it patches?

I have update mhansen's patch to pass doctests.

Cheers,

Michael

comment:14 Changed 12 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from assigned to closed

Merged in Sage 3.4.1.alpha0.

Cheers,

Michael

Note: See TracTickets for help on using tickets.