Template panel can trigger a SQL query

[nijel]

I've noticed this while working on #1833

When the template panel gets an object implementing __repr__, it might lead to triggering a SQL query:

django-debug-toolbar/debug_toolbar/panels/templates/panel.py

Lines 120 to 132 in 199c2b3

	token = allow_sql.set(False) # noqa: FBT003
	try:
	saferepr(value) # this MAY trigger a db query
	except SQLQueryTriggered:
	temp_layer[key] = "<<triggers database query>>"
	except UnicodeEncodeError:
	temp_layer[key] = "<<Unicode encode error>>"
	except Exception:
	temp_layer[key] = "<<unhandled exception>>"
	else:
	temp_layer[key] = value
	finally:
	allow_sql.reset(token)

The problem is that saferepr(value) is used for testing, while value is used for the actual assignment.

For example, with SimpleLazyObject, the __repr__ returns <SimpleLazyObject: <function <lambda> at 0x7fe8ccb9dca0>>, but later temp_layer[key] = value causes the query to be executed. This particular case will be addressed by #1833.

Using saferepr was introduced in 06bafad via #1076 to address other side effects of evaluating.

To make this code consistent, it should evaluate the value just once and use that later in the assignment.

[denisSurkov]

As far as I understand the code should be something like this?

 try: 
     value_safe_repr = saferepr(value)  # this MAY trigger a db query 

 ... 

 else: 
     temp_layer[key] = value_safe_repr

But I'm afraid of missing out details.

[nijel]

In that case, you can directly assign it without the else block:

try: 
    temp_layer[key] = saferepr(value)
... 

I didn't submit a PR with such a change because it does change the output (for example additional quoting on a string) as the result is then processed by pformat.

Current output is:

{'key': 'value'}

With the change, it would end up:

{'key': "'value'"}

But this made me actually look at what saferepr and pformat do, and they are doing almost the same thing, if one of them doesn't trigger evaluation, the other should neither.

In the end, what was triggering the SQL query was isinstance because that forces evaluation of the lazy object. This special case is covered by #1833, so I think this issue can be closes as there is nothing actually wrong with the referenced code.