Should NativeEnvironment try to evaluate Markup?

[tailg8nj]

If I understand correctly, when NativeEnvironment was added, the idea was to leverage most of the common CodeGenerator / NodeVisitor with slight changes to rendering child AST nodes. The aim was to then subsequently pass the return of the generated python code into ast.literal_eval. If it succeeded it was representative of a native Python object.

An issue my team has come across several times is with how NativeEnvironment handles the result of the tojson filter. For example:
{{some_global|tojson}}. This would return a str successfully iff the parameter to the filter has either a JSON boolean (lowercase true|false) or null. Without these values in the parameter, literal_eval would successfully parse the str to a Python list or dict.

Upon further inspection, the result of this single Node result is of type Markup from the MarkupSafe library.

Recently in parallel, with Python 3.10, a fix was added to have some conditional on the result here:

jinja/src/jinja2/nativetypes.py

Line 32 in 0d17780

if not isinstance(raw, str):

My proposal is to add an additional isinstance check to the same line of code to return raw:

        if isinstance(raw, Markup) or not isinstance(raw, str):
            return raw

My main point here is why should we be evaluating anything that is already escaped into Markup? And the added benefit is a more sane approach to the result of tojson.

There are some limitations here, in that this still would not work with constants, e.g. {{{'a': 1}|tojson}} as Jinja will try to compile this static expression to a str directly (because only expressions that evaluate at runtime throw nodes.Impossible()). I'm sure we could rework the _output_to_child_const code path to solve this as well. If an expression results in Markup which is already a sublcass of str there is no need to convert it to a constant (it already is one).

[gaborbernat]

I agree that if the user generates a JSON we should keep that a string rather than cast it back to pure Python. Note this would already happen if the JSON blob contains null or other values that are not valid in Python. Feels a bit off for the behaviour to differ based on the value content of the payload.

[davidism]

This is a weird corner case, but I'm not sure what the solution should be yet. This is also very similar to this issue: #917. Copying my comment from the linked Ansible issue:

You need to write a Python string literal to end up with a string containing JSON data when using Jinja's NativeEnvironment.

[gaborbernat]

As above explains currently we use the Markup (which is a child of str) to mark JSON dumps. Perhaps if we detect this at the root level we should skip trying to call literal_eval as in that case the user opted explicitly for a JSON that's a string. Not?

[davidism]

Markup isn't used to mark JSON though, it's used to mark a Python string as escaped. The fact that tojson returns Markup is for the benefit of HTML templates, not for the native environment.

[gaborbernat]

In that case, perhaps we can just parse the template and see if at the root level there's a tojson? 🤔 I feel that a template of {{?? |tojson}} should preserve the string and not unload it.

[tailg8nj]

Unfortunately the parsing into Python code from the AST and calling literal_eval happen at very different times in the execution. You do not have the AST nodes available in native_concat

[davidism]

This discussion split from here to Discord. I'm copying back my answer from Discord.

Basically, the problem is that the built-in tojson filter is geared towards HTML/JavaScript, where you want to treat the resulting JSON as data. It's not applicable in the native environment, where you truly want to end up with the string it produces, not the data it represents.

It's perfectly fine to override the filter in that case to suit the needs of that environment. All that needs to be done is to surround the produced string with single quotes. You can even reuse the existing filter. Calling str on the Markup produced by the built-in do_tojson stops it from applying further escaping, so it can be used in the f-string.

from jinja2.filters import do_tojson

@pass_eval_context
def do_tojson_str(eval_ctx, value):
    return f"'{str(do_tojson(eval_ctx, value))}'"

env.filters["tojson"] = do_tojson_str

[mkrizek]

FWIW as a note we solved this issue in Ansible by keeping a user configured list of filters that must return strings (Ansible's to_json being one of them) and we wrap return values of these filters in a special type that is not passed into literal_eval in our custom native_concat function.