Code

Opened 2 years ago

Closed 17 months ago

#18260 closed Bug (fixed)

Cache tag doesn't handle filter expresssions

Reported by: FunkyBob Owned by: regebro
Component: Template system Version: master
Severity: Normal Keywords: cache
Cc: charette.s@…, bmispelon@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

I found the cache tag didn't well handle when I passed, for instance:

{% cache ..... request.GET.foo %}

As it would raise an exception if GETfoo? wasn't set... so I tried passing a default:

{% cache ..... request.GET.foo|default:"bar" %}

Only to see it not only didn't fix the issue, but didn't appear to be parsing filter expressions, either.

I've updated the cache tag to use parser.compile_filter for all its arguments.

Attachments (5)

cachetag.diff (1.6 KB) - added by anonymous 2 years ago.
cachetag.2.diff (1.6 KB) - added by FunkyBob 2 years ago.
Changed to using token.split_contents() instead of token.contents.split() [thanks, brad!]
cachetag.3.diff (2.1 KB) - added by FunkyBob 2 years ago.
Fixed error message
cachetag4.diff (3.3 KB) - added by wkeeling 2 years ago.
cachetag5.diff (3.3 KB) - added by charettes 2 years ago.
Removed trailing whitespace and a unicode literal

Download all attachments as: .zip

Change History (13)

Changed 2 years ago by anonymous

Changed 2 years ago by FunkyBob

Changed to using token.split_contents() instead of token.contents.split() [thanks, brad!]

Changed 2 years ago by FunkyBob

Fixed error message

comment:1 Changed 2 years ago by thepointer

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

Looks good to me. Nice job FunkyBob

comment:2 Changed 2 years ago by charettes

  • Cc charette.s@… added
  • Needs tests set
  • Triage Stage changed from Ready for checkin to Accepted

IMHO this should be tested.

Changed 2 years ago by wkeeling

comment:3 Changed 2 years ago by wkeeling

I was looking through the easy pickings and thought I'd write a regression test for this one. Whilst working on the test I discovered that because the cache tag now does parser.compile_filter for all its arguments, it was converting the fragment name into a filter expression. This caused the cache tag to treat the fragment name as a variable and attempt to look it up in its context. The lookup would return nothing, and so the resulting cache key would be missing the fragment name. This would then cause the wrong fragments to be returned from the cache in certain cases.

I've attached a new patch (which doesn't treat the fragment name as a filter expression) plus the regression tests.

comment:4 Changed 2 years ago by charettes

  • Needs documentation set
  • Needs tests unset

You can drop the u':' at line 27 since your using module wide unicode literals: from __future__ import unicode_literals.

Maybe we should add a release note for that? All tests pass on Python 2.7.3rc1 sqlite3.

Last edited 2 years ago by charettes (previous) (diff)

Changed 2 years ago by charettes

Removed trailing whitespace and a unicode literal

comment:5 Changed 17 months ago by regebro

  • Owner changed from nobody to regebro
  • Status changed from new to assigned

comment:6 Changed 17 months ago by bmispelon

I "accidentally" fixed this while working on a different bug.

I have a pull request for this and #6271 at https://github.com/django/django/pull/751

comment:7 Changed 17 months ago by bmispelon

  • Cc bmispelon@… added

comment:8 Changed 17 months ago by Baptiste Mispelon <bmispelon@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 069280a6893d0f496c8f15922807939c68459ec3:

Used token.split_contents() for tokenisation in template tags accepting variables.

Fixed #6271, #18260.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.