Last Comment Bug 561042 - cannot load page send too large cookie
: cannot load page send too large cookie
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla8
Assigned To: arno renevier
:
Mentors:
http://renevier.net/bugs/bigcookie.php
: 685808 692016 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-22 02:58 PDT by arno renevier
Modified: 2011-10-05 00:24 PDT (History)
6 users (show)
dao+bmo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (155 bytes, text/plain)
2010-04-22 10:33 PDT, arno renevier
no flags Details
grey background when opening in a new tab (40.94 KB, image/png)
2010-04-23 00:47 PDT, arno renevier
no flags Details
patch proposal (3.75 KB, patch)
2011-07-16 04:08 PDT, arno renevier
bzbarsky: review+
Details | Diff | Splinter Review

Description arno renevier 2010-04-22 02:58:41 PDT
Hi,
when a page tries to set a big cookie, page does not load. Gecko does not even display about:blank, it just stays on current page. If i copy this url in firefox urlbar and open a new tab with Alt-Enter, all I get is a blank tab with grey background.

http://localhost/tmp/cookie.php
source:
http://localhost/tmp/cookie.txt
Comment 1 dwitte@gmail.com 2010-04-22 10:27:17 PDT
Can you attach the files here?
Comment 2 arno renevier 2010-04-22 10:33:21 PDT
Created attachment 440805 [details]
testcase
Comment 3 dwitte@gmail.com 2010-04-22 11:03:04 PDT
Sending an HTTP header that large will bust the limit on either the webserver, or the client (Firefox), and result in the connection being dropped. Hence the pageload fail.

This isn't a bug on our side; if anything, you should file a bug with the PHP folks to throw an error if you try to set a cookie larger than 4KB, and maybe if the total HTTP header size exceeds the limit. (I'm not sure exactly what that limit is; it varies between browsers and probably servers too.)
Comment 4 arno renevier 2010-04-23 00:42:40 PDT
(In reply to comment #3)
> Sending an HTTP header that large will bust the limit on either the webserver,
> or the client (Firefox), 

It's probably Firefox: other browsers handle this page well.

> and result in the connection being dropped. Hence the
> pageload fail.

Ok. But may be it's better to load aubout:neterror when firefox drop connection instead of letting a grey background.

> This isn't a bug on our side; if anything, you should file a bug with the PHP
> folks to throw an error if you try to set a cookie larger than 4KB, and maybe
> if the total HTTP header size exceeds the limit. (I'm not sure exactly what
> that limit is; it varies between browsers and probably servers too.)

Even if php modifies his behaviour. I can still send those headers with python, C, whatever. That won't prevent users from having this
Comment 6 arno renevier 2010-04-23 00:45:43 PDT
(In reply to comment #3)
> Sending an HTTP header that large will bust the limit on either the webserver,
> or the client (Firefox), 

It's probably Firefox: other browsers handle this page well.

> and result in the connection being dropped. Hence the
> pageload fail.

Ok. But may be it's better to load about:neterror when firefox drop connection instead of doing nothing.

> This isn't a bug on our side; if anything, you should file a bug with the PHP
> folks to throw an error if you try to set a cookie larger than 4KB, and maybe
> if the total HTTP header size exceeds the limit. (I'm not sure exactly what
> that limit is; it varies between browsers and probably servers too.)

Even if php modifies his behaviour. I can still send those headers with python, C, whatever.
Comment 7 arno renevier 2010-04-23 00:47:48 PDT
Created attachment 440977 [details]
grey background when opening in a new tab
Comment 8 dwitte@gmail.com 2010-04-23 10:23:44 PDT
Other browsers don't die, you say?

Worth another look, then.
Comment 9 Boris Zbarsky [:bz] 2010-04-23 12:22:07 PDT
nsHttpTransaction::ParseLineSegment has:

721     if (mLineBuf.Length() + len > MAX_LINEBUF_LENGTH) {
722         LOG(("excessively long header received, canceling transaction [trans=%x]", this));
723         return NS_ERROR_ABORT;
724     }

MAX_LINEBUF_LENGTH is (1024 * 10).

That code was added in bug 152697.

I suppose we could do an about:neterror here by using a status code nsDocShell::EndLoad recognizes or adding NS_ERROR_ABORT there, or using a new code just for this case.  We could also increase (or remove?) the limit here.
Comment 10 dwitte@gmail.com 2010-04-23 12:59:37 PDT
Sounds like a sensible thing to do, especially since a single cookie can be 4K. :)
Comment 11 Boris Zbarsky [:bz] 2010-04-23 14:05:14 PDT
Which one does?

I just checked and chrome has no obvious header length limit, btw.
Comment 12 dwitte@gmail.com 2010-04-23 14:12:38 PDT
Either making it much bigger or removing it. Having such a small limit to prevent DOS isn't such a strong argument nowadays, I think.

Ideally we'd also have a neterror page, but that can be a separate bug...
Comment 13 Boris Zbarsky [:bz] 2010-04-26 13:34:20 PDT
My gut feel is to nix the check.  Jason, thoughts?
Comment 14 Jason Duell [:jduell] (needinfo me) 2010-04-26 17:07:26 PDT
Sure, my gut feel is fine with nixing the check.  So long as we also make sure nothing breaks as a result :)
Comment 15 arno renevier 2011-07-16 04:08:49 PDT
Created attachment 546303 [details] [diff] [review]
patch proposal

patch proposal: remove http header length check.
Comment 16 Boris Zbarsky [:bz] 2011-07-22 08:42:24 PDT
Comment on attachment 546303 [details] [diff] [review]
patch proposal

r=me
Comment 17 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-07-25 06:16:39 PDT
http://hg.mozilla.org/mozilla-central/rev/9c8b0c5a5591
Comment 18 Alice0775 White 2011-09-09 09:28:02 PDT
*** Bug 685808 has been marked as a duplicate of this bug. ***
Comment 19 Alice0775 White 2011-10-05 00:24:09 PDT
*** Bug 692016 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.