Code

Opened 3 months ago

Closed 2 months 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: numerodix
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

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)

Attachments (0)

Change History (5)

comment:1 Changed 3 months ago by bmispelon

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 3 months 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 months ago by bmispelon

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 2 months ago by numerodix

  • Keywords nlsprint14 added
  • Owner changed from nobody to numerodix
  • Status changed from new to assigned

comment:5 Changed 2 months ago by Baptiste Mispelon <bmispelon@…>

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

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.

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.