Skip to content

Conversation

@prandla
Copy link
Member

@prandla prandla commented Sep 7, 2025

Fixes #1548.

I really hate this feature of jinja (both the fact that we need to thread all variables through macro arguments manually, and the fact that this obvious use-of-undefined-variable isn't caught automatically) but i don't know how to avoid it, short of rewriting CMS in rust or something.

@gollux
Copy link
Contributor

gollux commented Sep 7, 2025

Well, it's not problem of Jinja itself: we can pass undefined=StrictUndefined and it will die on any use of an undefined variable. But it would probably mean fixing half of the templates :(

Also, do we really need to pass everything through a macro? Wouldn't it be easier to pack all fields in an object and pass the object instead? (But no, I am not volunteering to rewrite the whole template now :))

@prandla
Copy link
Member Author

prandla commented Sep 7, 2025

Well, it's not problem of Jinja itself: we can pass undefined=StrictUndefined and it will die on any use of an undefined variable. But it would probably mean fixing half of the templates :(

we already use undefined=StrictUndefined. the problem here is that the buggy code isn't ever being evaluated (the problematic or is short-circuiting and the bad case never came up in any tests). the solution to this is static type-checking, but there is no type checker that can parse jinja templates.

Also, do we really need to pass everything through a macro? Wouldn't it be easier to pack all fields in an object and pass the object instead? (But no, I am not volunteering to rewrite the whole template now :))

idk about that solution, but there's another way to avoid passing all those arguments, and that's to not use macros. cms actually used to do this, but it was changed to the macro (#1115) because it "is supposedly safer (as they don't rely on state of the context, which is error prone)."

I disagree with that justification, as that was literally the commit that introduced this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Server returns 500 error when no tokens available

2 participants