Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#22242 closed Bug (fixed)

Setting cookie that is too large fails silently

Reported by: Euphorbium <compositum@…> Owned by: pirosb3
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Recently I've ran into a situation, that was extremely difficult to debug. Sometimes the cookie was set correctly, but sometimes it was not set at all. Turns out, that sometimes I was trying to set a cookie that was larger than 4096 bytes, and that is the cookie size limit in chrome and many other browsers. Trying to set a cookie that is too large should throw an exception. I could fix this myself, but I would like to hear other's opinions on this, because cookie size limit varies between the browsers, and I don't think that there is a way to find this out from django without a trial and error. This affects all versions of django.

Change History (14)

comment:1 by Sasha Romijn, 11 years ago

Component: UncategorizedHTTP handling
Triage Stage: UnreviewedAccepted

Both RFC 2109 and the more recent, but possibly not adopted everywhere yet, RFC 6265 state a minimum size allowance of 4096 bytes for cookies, as a SHOULD. Note that this includes the name, value and attributes as well.

If modern browsers follow the RFC, (it seems that Internet Explorer does, for example), and all allow at least 4096 bytes, I think we should throw an exception for larger cookies. This does not catch every potential issue: there are also limits on the total amount or size of cookies. However, testing for the maximum size of a single cookie seems to be so trivial compared to the benefit, I think we should add this check.

comment:2 by Euphorbium <compositum@…>, 11 years ago

Owner: changed from nobody to anonymous
Status: newassigned

comment:3 by Euphorbium <compositum@…>, 11 years ago

Ok, I've noticed that I am unable to set any cookie bigger than 4087 bytes on Chrome from django.

#views.py
from django.http import HttpResponse

def cookie(request):
    response.set_cookie('a', value="a"*4086)
    return response

Chrome probably displays the size of the cookie incorrectly, because if I try to set any cookie parameter, the cookie is not set at all, but if I try to set a parameter for a smaller cookie the cookie sets correctly, but the size displayed is not increased. It is the same is for firefox with firebug, but firefox tolerates larger cookies, so that is not a problem.

So django seems to set some cookie parameters by default, and it is hard form me to find them, as the size of the parameters is not calculated in the size of the cookie displayed by chrome.
I was not able to set the cookie that was 4096 bytes from javascript either, max that it accepted was 4095 bytes

document.cookie="a=any_4094_character_string"

in reply to:  3 comment:4 by Sasha Romijn, 11 years ago

Replying to Euphorbium <compositum@…>:

So django seems to set some cookie parameters by default, and it is hard form me to find them, as the size of the parameters is not calculated in the size of the cookie displayed by chrome.

Well, one of the defaults is to set the Path to /, for example. But we shouldn't make assumptions about the cookie parameters in any case. It looks like Django cookie handling code can't determine the final length: in the end the cookie is generated by the Python Cookie library. Encoding of special characters also influences the size. Therefore, I'm not sure whether it's actually possible to determine the final length of the cookie when set_cookie is being called.

comment:5 by Daniel Pyrathon, 11 years ago

hey guys,

In core/handlers/wsgi.py

        for c in response.cookies.values():
            response_headers.append((str('Set-Cookie'), str(c.output(header=''))))

Couldn't we override django.http.cookie.SimpleCookie.output, and make it throw an exception for larger cookies?

Regards,
Daniel Pyrathon

comment:6 by Daniel Pyrathon, 11 years ago

I had a bigger look at this.

in Cookie.py, the OutputString is what builds our output. http://hg.python.org/cpython/file/2.7/Lib/Cookie.py
I'll paste it for convenience:

  def OutputString(self, attrs=None):
        # Build up our result
        #
        result = []
        RA = result.append

        # First, the key=value pair
        RA("%s=%s" % (self.key, self.coded_value))

        # Now add any defined attributes
        if attrs is None:
            attrs = self._reserved
        items = self.items()
        items.sort()
        for K,V in items:
            if V == "": continue
            if K not in attrs: continue
            if K == "expires" and type(V) == type(1):
                RA("%s=%s" % (self._reserved[K], _getdate(V)))
            elif K == "max-age" and type(V) == type(1):
                RA("%s=%d" % (self._reserved[K], V))
            elif K == "secure":
                RA(str(self._reserved[K]))
            elif K == "httponly":
                RA(str(self._reserved[K]))
            else:
                RA("%s=%s" % (self._reserved[K], V))

        # Return the result
        return _semispacejoin(result)

RCF 6265 says that Cookies are measured by the sum of the length of the cookie's name, value, and attributes. So we are interested in the result array, which would make it very easy to calculcate the size of each cookie. Unfortunately the function returns a string. Does anyone see a good way of interacting with this function without having to repeat this code in another function?

comment:7 by Sasha Romijn, 11 years ago

So, a number of things have come up:

  • In set_cookie(), there is insufficient information to decide whether or not the cookie will be too large.
  • One way to resolve that is to generate the full cookie in set_cookie(), or something like that, just to check the length.
  • Alternatively, this could be caught when generating the Set-Cookie headers in the WSGIHandler.
  • Regardless of this change, there are other ways to send broken cookies, like having a path or domain that does not match the current one, or having many cookies which are individually small enough, but together too large.

Raising an error from WSGIHandler does not seem like the appropriate place, and regardless would not even work as expected: as far as I can see, the 500 handling, which will display the debug page or render the configured 500 page, etc., is not called if an exception is raised at this stage - that exception handling happens earlier.

Calculating the full length of the cookie on every set_cookie() call would work. However, this adds overhead to every cookie ever set, while setting cookies that are too large seems very exceptional to me. I don't think the rarity of this issue warrants adding such a performance impact on every cookie generated by every Django instance. And even in that case, we would only catch some of the errors made.

All things considered, it seems this is much harder than initially thought. Considering that, in combination with the rareness of this issue, and the fact that we can only catch some of these problems, I think it is not sensible to enforce cookie size in set_cookie(). Amending the set_cookie() documentation to mention the typical maximum length would definitely be useful and reasonable.

comment:8 by Euphorbium <compositum@…>, 11 years ago

Easy pickings: unset
Owner: anonymous removed
Status: assignednew

comment:9 by pirosb3, 11 years ago

Hi erikr,

I understand your reasoning perfectly. The compromise of having to check the cookie size on every set is too big.
Should I update the documentation?

Regards,
Dan

comment:10 by Sasha Romijn, 11 years ago

Component: HTTP handlingDocumentation
Easy pickings: set
Version: 1.6master

Yes, please make a documentation patch.

comment:11 by pirosb3, 11 years ago

Has patch: set

I have made a documentation patch. Please let me know if anything else should be added.

https://github.com/django/django/pull/2434

comment:12 by pirosb3, 11 years ago

Owner: set to pirosb3
Status: newassigned

comment:13 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 885e7adf568037b59b5642ab061133eaa00e5d7d:

Fixed #22242 -- Documented common cookie size limit.

comment:14 by Tim Graham <timograham@…>, 11 years ago

In 9f7bd831846a921e233c1f95d725235db9550438:

[1.6.x] Fixed #22242 -- Documented common cookie size limit.

Backport of 885e7adf56 from master

Note: See TracTickets for help on using tickets.
Back to Top