Opened 3 years ago

Closed 3 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: master
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


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 Changed 3 years ago by Baptiste Mispelon

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted


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.


comment:2 Changed 3 years ago by Keryn Knight <django@…>

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 Changed 3 years ago by Baptiste Mispelon

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 Changed 3 years ago by Martin Matusiak

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

comment:5 Changed 3 years ago by Baptiste Mispelon <bmispelon@…>

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