Re: Is using version 2 of the pickle protocol in {DB,FileBased}Cache
Thanks for filing the bug. It's probably best to discuss higher-level stuff on the mailing list, and details of the ticket on the ticket. That said, there's a lot of overlap. While setting pickle to use a lower protocol would "fix" the problem, it's really only a bandaid. Lower versions of the pickle protocol are badly inefficient with new-style classes, and should be avoided at all costs. We really just need to stop trying to pickle a class that is unpicklable. The appropriate fix for this probably lies in serializing the SimpleCookie object back into a string (that's what cookies are anyway, right?) before we pass it to pickle. I haven't looked closely at the code there, but that's where I would start. As for the test, you probably want to test it more holistically - maybe a test that fetches a page in the test client several times in a row and makes sure that it is the same each time. Further discussion should probably happen on the ticket. -Paul -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Is using version 2 of the pickle protocol in {DB,FileBased}Cache
Paul McMillan writes: > Ah, this does sound like a pretty nasty issue with our code then. > Unfortunately, the test suite doesn't get run as often as it should > with the various cache backends enabled, and I imagine that may be how > this cropped up. There have been a number of similar bugs in the past > - caching is hard! I'd appreciate it if you could open a ticket for > this and reference the mailing list discussion. If you could set the > owner to me (PaulM), I'll work on it when I get a chance. Thanks for > taking the time to track down how this is happening. Hey there, Paul, Thanks for the interest, I've filed bug #15863. In case you have some ideas on the directions to take, I could try working on a patch to tackle the issue -- from what I see, using pickle.HIGHEST_PROTOCOL is causing the bug, however using a value < pickle.HIGHEST_PROTOCOL is also unwanted due to the serialized string being much larger? In case I work on a patch to improve the unit tests for this part, should I file a new ticket? Should this mail have actually been sent as a comment to the ticket? :) -- Raphael Kubo da Costa ProFUSION embedded systems http://profusion.mobi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Is using version 2 of the pickle protocol in {DB,FileBased}Cache
Ah, this does sound like a pretty nasty issue with our code then. Unfortunately, the test suite doesn't get run as often as it should with the various cache backends enabled, and I imagine that may be how this cropped up. There have been a number of similar bugs in the past - caching is hard! I'd appreciate it if you could open a ticket for this and reference the mailing list discussion. If you could set the owner to me (PaulM), I'll work on it when I get a chance. Thanks for taking the time to track down how this is happening. -Paul -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Is using version 2 of the pickle protocol in {DB,FileBased}Cache
Paul McMillan writes: > Yes, SimpleCookie is known to be an unpickleable class. We shouldn't > be directly pickling it anywhere in Django. In your code, you should > probably turn the cookie into a string before caching it. I'm not > clear if the bug you're experiencing is happening in Django's code or > something your application is doing directly with SimpleCookie. [snip] > I think that your provided test case is trying to do something that is > explicitly not supported, but I'm unclear on whether or not there is > an issue in Django-provided code. Could you provide a little more > information? Hi Paul, I am not trying to pickle SimpleCookie directly -- in fact, I enabled the cache middlewares in settings.py and then set CACHE_BACKEND to 'file:///some/directory'. I then had a view with no specific cache decorators, but since the session backend is also on it added the `Vary: Cookie' header. After that, I started noticing that a login page including the `csrf_token' tag started repeating the token when I used curl to access it without providing any cookies or login credentials. And after the first time I accessed it, the Set-Cookie header started misbehaving like it did in the test case I attached -- instead of looking like Set-Cookie: foo=bar; other-parameters; it was looking like Set-Cookie: foo="Set-Cookie: foo=bar; other-parameters;" and the value in the csrf tag was being expanded to something along the lines of "Set-Cookie: foo=bar; other-parameters;", so validation failed later. Some investigation led me to find the problem in the cache backend I was using, as the cookies inside the cached HttpResponse were being serialized incorrectly and later picked by FetchFromCacheMiddleware. -- Raphael Kubo da Costa ProFUSION embedded systems http://profusion.mobi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Is using version 2 of the pickle protocol in {DB,FileBased}Cache
Hi Raphael, Yes, SimpleCookie is known to be an unpickleable class. We shouldn't be directly pickling it anywhere in Django. In your code, you should probably turn the cookie into a string before caching it. I'm not clear if the bug you're experiencing is happening in Django's code or something your application is doing directly with SimpleCookie. The last memcached backend I looked at pickled things using the highest protocol. I don't remember which one it was. It's very odd that you're not seeing the same error using memcached. Locmem should use the highest version of pickle, to more closely mirror the behavior of memcached. I'll open a ticket about that. Django shouldn't switch to the earlier versions of pickle for performance reasons. I think that your provided test case is trying to do something that is explicitly not supported, but I'm unclear on whether or not there is an issue in Django-provided code. Could you provide a little more information? Thanks, -Paul On Fri, Apr 15, 2011 at 1:23 PM, Raphael Kubo da Costa wrote: > Hello there, > > I was experiencing some problems with Django's caching system on 1.2.X > (1.2.5, to be more specific) when using either the database or the > file-based backends. > > Both use pickle and call pickle.dumps(..., pickle.HIGHEST_PROTOCOL) when > serializing the data after being called by UpdateCacheMiddleware. > > However, it looks like pickle does not play nice with SimpleCookie-based > classes [1]. In the first request (still not cached), the csrf token is > set correctly as follows: > > Set-Cookie: csrftoken=XX; Max-Age=31449600; Path=/ > > When the same view is requested again, though, the cookie is retrieved > incorrectly from the cache by FetchFromCacheMiddleware: > > Set-Cookie: csrftoken="Set-Cookie: csrftoken=XX Max-Age=31449600\073 Path=/" > > The locmem, dummy and memcached backends do not present this problem: > locmem does not specify the protocol version when calling pickle.dumps, > which means protocol version 0 will be used; dummy does not do anything > and memcached does not use pickle. Pickle protocol versions 0 and 1 work > fine. > > The following patch to the unit tests should break both > FileBasedCacheTests and DBCacheTests. I only tested it against the 1.2.X > git branch, but 1.3.X should present the same behaviour, and both Python > 2.7.1 and Python 3.2 fail. > > diff --git a/tests/regressiontests/cache/tests.py > b/tests/regressiontests/cache/tests.py > index 0581e4e..5611eef 100644 > --- a/tests/regressiontests/cache/tests.py > +++ b/tests/regressiontests/cache/tests.py > @@ -285,6 +285,22 @@ class BaseCacheTests(object): > self.assertEqual(self.cache.get("expire2"), "newvalue") > self.assertEqual(self.cache.has_key("expire3"), False) > > + def test_cookie_caching(self): > + try: > + from Cookie import SimpleCookie > + except ImportError: > + from http.cookies import SimpleCookie > + > + test_cookie = SimpleCookie() > + test_cookie['key'] = 'some value' > + > + self.cache.set('some_cookie', test_cookie) > + > + cached_cookie = self.cache.get('some_cookie') > + > + self.assertEqual(cached_cookie['key'].value, > + test_cookie['key'].value) > + > def test_unicode(self): > # Unicode values can be cached > stuff = { > > [1] http://bugs.python.org/issue826897 > > -- > Raphael Kubo da Costa > ProFUSION embedded systems > http://profusion.mobi > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-developers@googlegroups.com. > To unsubscribe from this group, send email to > django-developers+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/django-developers?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Is using version 2 of the pickle protocol in {DB,FileBased}Cache
Hello there, I was experiencing some problems with Django's caching system on 1.2.X (1.2.5, to be more specific) when using either the database or the file-based backends. Both use pickle and call pickle.dumps(..., pickle.HIGHEST_PROTOCOL) when serializing the data after being called by UpdateCacheMiddleware. However, it looks like pickle does not play nice with SimpleCookie-based classes [1]. In the first request (still not cached), the csrf token is set correctly as follows: Set-Cookie: csrftoken=XX; Max-Age=31449600; Path=/ When the same view is requested again, though, the cookie is retrieved incorrectly from the cache by FetchFromCacheMiddleware: Set-Cookie: csrftoken="Set-Cookie: csrftoken=XX Max-Age=31449600\073 Path=/" The locmem, dummy and memcached backends do not present this problem: locmem does not specify the protocol version when calling pickle.dumps, which means protocol version 0 will be used; dummy does not do anything and memcached does not use pickle. Pickle protocol versions 0 and 1 work fine. The following patch to the unit tests should break both FileBasedCacheTests and DBCacheTests. I only tested it against the 1.2.X git branch, but 1.3.X should present the same behaviour, and both Python 2.7.1 and Python 3.2 fail. diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py index 0581e4e..5611eef 100644 --- a/tests/regressiontests/cache/tests.py +++ b/tests/regressiontests/cache/tests.py @@ -285,6 +285,22 @@ class BaseCacheTests(object): self.assertEqual(self.cache.get("expire2"), "newvalue") self.assertEqual(self.cache.has_key("expire3"), False) +def test_cookie_caching(self): +try: +from Cookie import SimpleCookie +except ImportError: +from http.cookies import SimpleCookie + +test_cookie = SimpleCookie() +test_cookie['key'] = 'some value' + +self.cache.set('some_cookie', test_cookie) + +cached_cookie = self.cache.get('some_cookie') + +self.assertEqual(cached_cookie['key'].value, + test_cookie['key'].value) + def test_unicode(self): # Unicode values can be cached stuff = { [1] http://bugs.python.org/issue826897 -- Raphael Kubo da Costa ProFUSION embedded systems http://profusion.mobi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.