Opened 10 years ago

Closed 10 years ago

#21741 closed Cleanup/optimization (fixed)

render_to_string without providing a dictionary still puts a dictionary into the context

Reported by: Keryn Knight <django@…> Owned by: Martin Matusiak
Component: Template system Version: dev
Severity: Normal Keywords: nlsprint14
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

given the following scenario:

>>> from django.template.loader import render_to_string
>>> from django.template import Context
>>> render_to_string('file.extension', context_instance=Context())
>>> # or ...
>>> render_to_string('file.extension')

where a dictionary is explicitly not being provided, the final context given to the template when render() is called will have {} as the last of the .dicts

The reason is because if none is provided, a new dict is created, and then it is provided to the context instance (1, 2) without checking the dict's len()

Marking as cleanup/optimization because it doesn't cause any problems (the only one being: it appears in debug_toolbar), but providing the ticket at least means the option is there to change it, and a history of why not to change it exists if wontfix'd (which may be the best course of action, on balance, but that's not for me to say)

Change History (5)

comment:1 by Baptiste Mispelon, 10 years ago

Triage Stage: UnreviewedAccepted

Hi,

I'm +0 on this but I think it'd be easier to discuss over a concrete proposal so I'll accept the ticket.

Any proposal should not add too much complexity and should aim at remaining as backwards-compatible as possible.

Thanks

comment:2 by Keryn Knight <django@…>, 10 years ago

The patch, such as it is,

All tests pass, but there's no test to evidence the proposed fix works as intended, because I'm not sure how to write such a unit test - the function is a black box - I can't verify the context given to the template is of the correct length, because the return value is obviously a string. Any ideas on how one might test the context for correctness in this scenario would be gratefully received.

comment:3 by Baptiste Mispelon, 10 years ago

I guess it'd be possible to write a custom template tag that outputs info about the context and use it, but that might be a bit complicated.

Another way would be to create a custom Context and pass it as context_instance. That way you could make sure that no extra layer is added to it.
The downside of this is that you're only testing half of the change but other than that, it's quite simple to implement.

comment:4 by Martin Matusiak, 10 years ago

Keywords: nlsprint14 added
Owner: changed from nobody to Martin Matusiak
Status: newassigned

comment:5 by Baptiste Mispelon <bmispelon@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 7e1376c2b0ea9e02e61c1c764f02cd40c6f7e849:

Fixed #21741 -- Fixed render_to_string to stop pushing empty dictionaries to its Context

Thanks to kezabelle for the report and original patch
and to numerodix for his improved patch.

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