#22242 closed Bug (fixed)
Setting cookie that is too large fails silently
Reported by: | 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 , 11 years ago
Component: | Uncategorized → HTTP handling |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 4 comment:3 by , 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"
comment:4 by , 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 , 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 , 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 , 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 , 11 years ago
Easy pickings: | unset |
---|---|
Owner: | removed |
Status: | assigned → new |
comment:9 by , 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 , 11 years ago
Component: | HTTP handling → Documentation |
---|---|
Easy pickings: | set |
Version: | 1.6 → master |
Yes, please make a documentation patch.
comment:11 by , 11 years ago
Has patch: | set |
---|
I have made a documentation patch. Please let me know if anything else should be added.
comment:12 by , 11 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:13 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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.