Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#21627 closed defect (fixed)

HTTP 304 responses not handled correctly

Reported by: mcs Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-torbutton, TorBrowserTeam201706R
Cc: brade, gk Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

This is a spinoff from #21396. While testing gk's fix for that bug, Kathy and I noticed messages like the following on the error console:

15:09:57.836 NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIHttpChannel.getResponseHeader]1 content-policy.js:99:0

It turns out that this is caused by responses that have an HTTP status code of 304 ("Not Modified"); we saw them consistently when we reloaded the browserleaks.com page (which makes sense, since the browser sends conditional GET requests in that situation). These kinds of responses do not have a Location header.

We should modify the Torbutton code in src/components/content-policy.js to add a try/catch around the call to getResponseHeader() and not log anything if the response's status code is 304 (we probably want to log something in all other cases).

This is not urgent since it is not a new problem, but it would be good to fix it soon.

Child Tickets

Change History (6)

comment:1 Changed 2 years ago by gk

Keywords: TorBrowserTeam201704 added; TorBrowserTeam201703 removed

Remmove remaining tickets over to April

comment:2 Changed 2 years ago by gk

Keywords: TorBrowserTeam201705 added; TorBrowserTeam201704 removed

Moving our tickets to May 2017.

comment:3 Changed 2 years ago by mcs

Keywords: TorBrowserTeam201706R added; TorBrowserTeam201705 removed
Status: newneeds_review

Here is a patch:
https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug21627-01
Rather than adding a try/catch, it seems easiest to simply ignore 304 responses.

comment:4 Changed 2 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good to me. Applied to master: commit ad937183266423425e166b417659b1db14aaeced.

comment:5 Changed 2 years ago by cypherpunks

Why have you left

16:11:36.613 NS_ERROR_MALFORMED_URI: Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIIOService.newURI] 1 content-policy.js:157

comment:6 in reply to:  5 Changed 2 years ago by mcs

Replying to cypherpunks:

Why have you left

16:11:36.613 NS_ERROR_MALFORMED_URI: Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIIOService.newURI] 1 content-policy.js:157

Because we did not see that error.

I was able to reproduce this problem by visiting http://httpstat.us/304, but the error is actually unrelated to the 304 response code. What happens in this case is that a relative URL is returned in the Location header when a GET http://httpstat.us/favicon.ico is done, and the code at content-policy.js:157 does not handle relative URLs. The response code is a 302. I opened #22525 for this issue.

Note: See TracTickets for help on using tickets.