Opened 3 weeks ago

Closed 2 weeks ago

#37318 closed Bug (invalid)

first, last, and random template filters raise KeyError on a dict instead of failing gracefully

Reported by: Akshat Kansal Owned by: Akshat Kansal
Component: Template system Version: dev
Severity: Normal Keywords: defaultfilters first last random
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Akshat Kansal)

The first, last and random template filters raise KeyError when given a
dictionary, instead of failing gracefully the way they do for an empty list.

How to reproduce

import django
from django.conf import settings
settings.configure(TEMPLATES=[{
    "BACKEND": "django.template.backends.django.DjangoTemplates",
    "DIRS": [], "APP_DIRS": False, "OPTIONS": {},
}])
django.setup()

from django.template import Template, Context

Template("{{ d|first }}").render(Context({"d": []}))        # returns ''
Template("{{ d|first }}").render(Context({"d": {"a": 1}}))  # KeyError: 0
Template("{{ d|last }}").render(Context({"d": {}}))         # KeyError: -1

A list degrades to an empty string. A dict raises KeyError, which escapes
template rendering and becomes a 500. Dicts are common in template contexts,
so this is reachable from an ordinary template.

Why this looks like a bug

first() already catches IndexError so that a wrong-shaped value degrades
instead of raising:

def first(value):
    try:
        return value[0]
    except IndexError:
        return ""

A dict is the same class of mistake as an empty list, but one returns "" and
the other 500s. The inconsistency is the complaint, not the fact that dicts
are unsupported.

I am happy to write the patch and tests if this is accepted.

Reproduced on main @ 5180f82 (6.2.dev).

Change History (5)

comment:1 by Akshat Kansal, 3 weeks ago

Description: modified (diff)
Has patch: set

comment:2 by Akshat Kansal, 3 weeks ago

I kansalakshat is working on it

in reply to:  description comment:3 by Jacob Walls, 3 weeks ago

Replying to Akshat Kansal:

Run the reproduction yourself before you submit — that part isn't about policy, it's that you'll be asked follow-up questions and should have seen the failure with your own eyes.

Please proofread next time.

comment:4 by Akshat Kansal, 3 weeks ago

Description: modified (diff)

in reply to:  description comment:5 by Mike Edmunds, 2 weeks ago

Resolution: → invalid
Status: assigned → closed

Thank you for the clear report and example. I'm not convinced that this inconsistency is a bug or that the current behavior is incorrect.

There's a widely-held opinion that silently ignoring errors in templates causes more problems than it solves, by masking genuine coding errors. (See, e.g., ​https://github.com/django/new-features/issues/154 and the large number of past discussions and tickets linked from it.)

I don't think we should add a new special case that ignores what is almost certainly unintended code: passing a mapping object like a dict to a template filter that is meant to return an item from a sequence. If you have a real-world example of code where this is the desired behavior in a template, it would help to share that.

Replying to Akshat Kansal:

first() already catches IndexError so that a wrong-shaped value degrades
instead of raising: […]
A dict is the same class of mistake as an empty list, but one returns "" and
the other 500s. The inconsistency is the complaint, not the fact that dicts
are unsupported.

I would respectfully disagree that these are the same class of error. There's a reasonable argument that, at least in a template context, the answer to the question "what is the first item of an empty list?" is "nothing" rather than an error. (btw, this use case is common enough that JavaScript introduced the Array at() method with exactly those semantics. Also, there's an equally reasonable argument that the answer should be an error, but that would be a different discussion.)

On the other hand, it's hard to see how "what is the first item of a non-empty dict?" should be "nothing." One might argue that it should be the first of the dict's values() or keys() or items(), or that it should be an error, but "nothing" seems unexpected and extremely likely to mask a real problem in the template context. (Also, Python itself considers those to be different classes of error: IndexError vs. KeyError.)

By that logic, I'm closing this as "invalid." (But again, if you have a real-world use case where this is causing problems, please do add that info in a comment and change the ticket back to "unreviewed" for triage.)

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