Closed Bug 632061 Opened 13 years ago Closed 13 years ago

Firefox 4 does not always properly handle HTTP/0.9 responses

Categories

(Core :: Networking: HTTP, defect)

Product:
Core
Component:
Networking: HTTP
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- final+

People

(Reporter: bugzilla, Assigned: mcmanus)

References

(
URL
)

Details

(Keywords: regression, Whiteboard: [hardblocker])

Attachments

(1 file)

632061 patch v1
3.13 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110207 Firefox/4.0b12pre
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110207 Firefox/4.0b12pre

Found this when i was trying to log in on the admin interface of my Sitecom router. 

In some cases (unsure what exactly triggers it, but i do have an example page that triggers it) Firefox 4 doesn't show web pages when the server replies with a HTTP/0.9 response. (= no response headers, just the HTML content)

Example (simple test server written in perl): http://oele.net:9000/0 (does not work in FF4, but does work in FF3, Opera 11 and Chrome 8)

The same page with a "HTTP/1.1 200 OK" header added: http://oele.net:9000/1 (*does* work in FF4). 

Reproducible: Always

Steps to Reproduce:
1. Visit http://oele.net:9000/0 in FF4, FF3, Opera and Chrome
2. Visit http://oele.net:9000/1 in FF4, FF3, Opera and Chrome
Actual Results:  
On http://oele.net:9000/0, the 'test' text is not displayed in FF4 but it does display in the other browsers. 

http://oele.net:9000/1 works fine on all browsers. 

Expected Results:  
FF4 should correctly handle HTTP/0.9 responses

I'm unsure what the exact trigger is. If i change small parts of the HTML code, it sometimes works but i can't seem to track it down to a specific tag or something.
blocking2.0: --- → ?
simple enough.. I'll look at it later today.

/home/mcmanus/src/mozilla2/wd/pipemq/netwerk/protocol/http>telnet oele.net 9000
Trying 82.94.175.160...
Connected to oele.net.
Escape character is '^]'.
GET /0 HTTP/1.0
<html><head></head><body>test</body></html>
Connection closed by foreign host.
Assignee: nobody → mcmanus
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Bug 363109 might be relevant. There, we stopped allowing HTTP/0.9 responses if we had previously received any HTTP/1.0+ responses on the connection.
(In reply to comment #2)
> Bug 363109 might be relevant. There, we stopped allowing HTTP/0.9 responses if
> we had previously received any HTTP/1.0+ responses on the connection.

363109 is the culprit, but it isn't because of the 1.0 interactions..

we now do this:
  char *p = LocateHttpStart(buf, PR_MIN(count, 8), PR_TRUE);

which says "look for HTTP/1. in (up to) the first 8 bytes of the input... and accept any partial match of that case insensitively at the end of the buffer"..

The first 8 bytes of input is "<html><h", which I bet is pretty common for a 0.9 response :).. and that last "h" is treated as a partial match - so the response is considered http/1.0 and breaks.

The old algorithm did not allow a partial match unless less than 4 bytes were consumed in the first read from the network.
Attached patch 632061 patch v1 — — Splinter Review 
Return the partial HTTP match behavior to the 3.6 algorithm.

* Accept partial matches only in the case where we have such a short read that we can't search the full buffer

* Accept up to 4 bytes of junk before the search string. (3.6 searched in 8 bytes for a 4 byte string, 4.0 now searches in 11 bytes for a 7 byte string) - this accommodates 2 CRLFs which ought to be enough (or at least is the same as 3.6).

xpcshell tests are happy, michal's tests are happy, the url from this bug is happy, the recent bassdrive test is happy.. all is happy!
Attachment #510364 - Flags: review?(bzbarsky)
sander - excellent STR!
Keywords: regression
Comment on attachment 510364 [details] [diff] [review]
632061 patch v1

r=me
Attachment #510364 - Flags: review?(bzbarsky) → review+
Attachment #510364 - Flags: approval2.0?
I filed Bug 632210 to track the investigation of whether we must allow arbitrary data to prefix the status line, or whether we should be more strict (e.g. allow only whitespace prefixes).
(In reply to comment #4)
> xpcshell tests are happy, michal's tests are happy, the url from this bug is
> happy, the recent bassdrive test is happy.. all is happy!

I've just noticed that the iframe test isn't happy. Some iframes are duplicated and some are missing when pressing F5. But this isn't caused by this patch. It behaves the same even when only the patch from bug 363109 or bug 628832 is applied. I'll file a new bug for this issue.
(In reply to comment #8)

> I've just noticed that the iframe test isn't happy. Some iframes are duplicated
> and some are missing when pressing F5. But this isn't caused by this patch. It

that's weird - because I ran that test. but then I found I can see what you say if I have a clean cache.. or maybe if I let some pconns timeout (not sure about that one).

As you say, its the same with or without this patch, so it should not be effected. approval-2.0? is still appropos.

What is the new bug #? So far I've confirmed the problem exists 5000 changesets backwards.. so it cannot be that bad :)
blocking2.0: ? → final+
Whiteboard: [hardblocker]
Blocks: 363109
http://hg.mozilla.org/mozilla-central/rev/3872a1e4a881
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
After installing today's nightly build, the web interface of my Sitecom router is now happy too! 

Thanks!
(In reply to comment #9)
> What is the new bug #?

632835
Comment on attachment 510364 [details] [diff] [review]
632061 patch v1

Has landed already, so clearing redundant approval2.0?, since it's causing this fixed bug to erroneously appear under approval requests here:
http://office.smedbergs.us/blocker-report/
Attachment #510364 - Flags: approval2.0?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: