Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#10016 closed (fixed)

Cache middleware with memcached can't handle long URLs

Reported by: Jacob Owned by: mcroydon
Component: Core (Cache system) Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Memcached has a hard limit of 250 characters for cache keys (cite). However, cache keys for the cache middleware (and associated view decorators) are of the form

   'views.decorators.cache.cache_page.%s.%s.%s' % (key_prefix, request.path, headers_hash)

(cite)

This means that if the combined length of settings.CACHE_MIDDLEWARE_KEY_PREFIX and request.path is over about 180 characters, you'll get failures. With cmemcache you get obscure server errors; the pure-Python memcached client throws MemcachedKeyLengthError.

Attachments (2)

10016-cache-middleware-handle-long-urls.diff (1.7 KB) - added by Timothée Peignier 8 years ago.
10016-handle-long-urls-with-tests.diff (5.2 KB) - added by mcroydon 8 years ago.
Updated patch (apply with -p1) to include tests for the new behavior.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by Timothée Peignier

Has patch: set
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

I've fix the problem by replacing request.path by an hash of request.path, see attached patch.

comment:2 Changed 8 years ago by Timothée Peignier

Needs tests: set
Owner: changed from nobody to Timothée Peignier
Status: newassigned

comment:3 Changed 8 years ago by Timothée Peignier

Version: 1.0SVN

comment:4 Changed 8 years ago by Jacob

milestone: 1.1
Triage Stage: UnreviewedAccepted

Changed 8 years ago by Timothée Peignier

comment:5 Changed 8 years ago by Timothée Peignier

Updated patch that generate key each form request.path everywhere it's use.

Changed 8 years ago by mcroydon

Updated patch (apply with -p1) to include tests for the new behavior.

comment:6 Changed 8 years ago by mcroydon

Needs tests: unset

I'm not sure if it's worth noting that this is a change in internals (we're generating a key based on a hash of the path not the path itself). There may be some people coding against the existing behavior, but they are probably firmly in "you're mucking with internals so expect things to break" territory.

I also didn't know how to best deal with the really long line lengths in the asserts so I left them there. Feel free to clean that up as you best see fit.

comment:7 Changed 8 years ago by mcroydon

Owner: changed from Timothée Peignier to mcroydon
Status: assignednew

Working on cleaning this up a little more per Jacob.

comment:8 Changed 8 years ago by Jacob

Resolution: fixed
Status: newclosed

(In [10335]) Fixed #10016: the cache middleware no longer vomits when handed long URLs. Thanks, Matt Croydon.

comment:9 Changed 8 years ago by Jacob

(In [10336]) [1.0.X] Fixed #10016: the cache middleware no longer vomits when handed long URLs. Thanks, Matt Croydon. Backport of r10335 from trunk.

comment:10 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

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