According to rfc2616, persistent connections are the default for HTTP/1.1, but Mozilla sends a "Connection: close" header unless the preference "network.http.keep-alive" is set. Reproducible: Always Steps to Reproduce: 1. In Preferences/Debug/Networking: 2. Deselect "Enable Keep-Alive" 3. Set "HTTP Version" to "1.1". 4. Make any HTTP GET request and examine headers. Expected Results: When Mozilla is sending an HTTP/1.1 request it should never send "Connection: close". The "Connection: keep-alive" and "Keep-alive" headers are unnecessary and undesirable in HTTP 1.1 - see rfc2616 19.6.2, "Compatibility with HTTP/1.0 Persistent Connections".

The keep-alive pref is a debug-only pref designed for diagnosing and isolating problems with persistent http transactions. When the pref is off (and it is on by default), we don't want persistent connections at all, which is why we send Connection: close. The pref could possibly be renamed to network.http.persistent-connection but since its only purpose is for debugging there's not much point. We send Connection: Keep-Alive, and the Keep-Alive: header for compatablility with older servers. You do have a point that 19.6.2 points out what is wrong with doing so (although it does not actually forbid it, or in fact mention http/1.1 clients at all). However, since ns4 and ie also send these headers, I'm not sure that any proxy server will break in this way, especially when combined with the rules for http/1.1 servers in 14.10. Are you aware of anything which breaks in this case? ->darin

I agree that there is no point renaming a debugging pref - but what about other prefs such as network.http.keep-alive.max-connections? (I'll be commenting on bug 83526 soon) I also understand and agree with your decision to send the keep-alive headers for compatibility with older servers. The case I would like to see fixed is when using HTTP/1.1 together with a proxy. If the proxy is 1.1-compliant, the headers are superfluous; if not, the problem described in 19.6.2 could occur. Sorry, I don't have an actual example. As a point of information, MSIE 5.5 doesn't send "Connection: Keep-Alive" and "Keep-Alive" when using HTTP/1.1 together with a proxy - it sends "Proxy- Connection: Keep-Alive", which I understand was originally a Netscape "extension" to HTTP :-)

Well, we probably should fix it for proxies, although, as I mentioned to darin last night, that won't help us for tranparent proxies. Changing aummary, and confirming.

with this patch we obviously fail to ever open keep-alive connections to HTTP/1.0 servers. i'm only submitting it for the sake of testing, etc.

does bug 84264 really depend on this?

bbaetz, can you review darin's patch?

dougt: Its not complete - see darin's comments. I could make it conditional on us talking to a proxy server - would that work? We then wouldn't do keep-alive to a 1.0 proxy server that doesn't understand 1.1 (ie squid works) I'll check what IE does when neeti's machine finishes installing SP2, and I can run the packet sniffer.

OK, so IE uses proxy-connection: keep-alive when it talks to a proxy, and honors proxy-connection: close. That header is non-standard, and I can't find any docs on it, except a few mailing list archives which point out its flaws (basically the same as are in the bottom of the HTTP/1.1 RFC, applied to multi-hop proxies) So my choice would be to send proxy-connection instead of connection, when talking to a proxy. This will mean that (compared to the current situation) we won't have keep-alives if we talk to an HTTP/1.0 proxy which doesn't understand HTTP/1.1 and which understands connection: keep-alive, but not the proxy-connection: keep-alive. I don't know of any servers which fall into that category, however. Does that seem reasonable?

Seems very reasonable to me - servers which fall into the category you describe (if they exist) will have the same problem with IE.

So, it turns out that Darin's patch does more that I thought it did. I've just changed it to send Connection/Proxy-Connection headers as appropriate. And, we now use keep-alives to lots of sites. Like those using netscape enterprise server. And those using IIS. And www.google.com. (apache results don't change) Adding perf - this cuts down the number of packets involved in downloading www.cnn.com from 900 to about 610. Using john taylor's page loader test, we appear to improve in speed between 5% to 30% on www.cnn.com (numbers are very variable, and were generated on a -O2 --enable-debug build) Someone want to try some ibench numbers with and without this patch? gagan/dougt - r/sr?

bbaetz: what if the proxy type is ssl?

you should check if the proxyType is "http" (i think)

Proxy-connectinon: I think they are mentioned in Ari's Web Proxy book, (which I lost my copy of...) tever has a copy on his desk :) More importantly, what proxy servers are following this line?

OK, ibench numbers didn't change over the LAN. benc: No, they're not in that book, but squid accepts them, and does the right thing. In the future, we may consider just not sending these headers, but we're not going to make that change now.

r=gagan on the last patch. Please verify this with a dialup connection to see if this makes a bigger splash on ibench numbers and that will give us an idea if this is a candidate for the branch.

-> dougt for sr and checkin to the trunk early next week jrgm, this is the bug we talked about

retaking/targeting. I'll nag dougt again for an sr this week....

might this causes things like bug 89875?

possibly, although I'm not certain. Its probably related though.

Just under 5% better performance on ibench over a 50K modem connection.

checked in on trunk.

... and marking fixed

embedding wants this for their release. nominating nsBranch

mcelrath@isp.nwu.edu pointed out in bug 84264 that our default timeout should be the same as what we send in the keep-alive header, ie 300 seconds. This seems long, but we already detect when the server closes teh connection before we write to it, and we really want 'infinite' for http/1.1, so this isn't a problem. Patch coming up - see bug 91024. This patch needs to go on the branch in addition to the other one.

r=gagan on the last patch.

a timeout of 300 is way too big.. practically speaking you'll force servers to do the active close.. remember that the side who does the active close needs to go into TIME_WAIT.. and given that there are >=1 clients and exactly 1 server, its much nicer to have clients doing active closes. fwiw, the raw # isn't particularly interesting on the server side.. I've never seen a need to consult it in my applications... there really is no reason to send one, the more interesting thing is to decide when to give up on an idle socket. msie doesn't send a specifc parameter, but its timeouts seem tied to click patterns (i.e. closes a lot of open sessions when a page is fully rendered) and this works well.

I don't really care about the value of the timeout, but I want it to be possible for a proxy to keep a single connection open for an infinite amount of time, as long as Mozilla keeps requesting pages. Mozilla should obey 'Connection: keep-alive' and 'Keep-Alive: <seconds>' when sent with a response from a proxy (or server), and Mozilla should not ever close a connection for any (non-error) reason except timeout (see bug 84264). Of course, the server may close the connection. 10s timeout to server sounds reasonable to me. Really, the default timeout should be set to ~twice the average amount of time it takes Mozilla to parse your average page, figure out which images are in that page, and make requests for those images, on a reasonably slow line.

Please make these comments in bug 91024. After this patch, the timeout will be 300 seconds (ie 5 minutes) idle by default. Its already a pref. We know that the behaviour is not pefect, but its now better than not reusing the connections at all. The server can still close it earlier, and we will respect that. We may need more than just 'keep it open for n seconds', but this is not the bug to talk about that.

dougt says sr=dougt. Oh, and I did mean bug 91024. Oops.

3rd try lucky - bug 91204 is what I meant. No, really :)

pushing out milestone to avoid getting spammed - this is checked in on the trunk already, and so the milestone doesn't really make sense.

checked in on 092 branch.