Skip to content

Added support for typst in default text escapers#365

Closed
xabi00 wants to merge 1 commit intoaskama-rs:masterfrom
xabi00:master
Closed

Added support for typst in default text escapers#365
xabi00 wants to merge 1 commit intoaskama-rs:masterfrom
xabi00:master

Conversation

@xabi00
Copy link
Copy Markdown

@xabi00 xabi00 commented Mar 12, 2025

I did this exact Pull Request in what today is named 'askama-old' repository. These changes were successfully merged; however, they occurred after Rinja's fork, which means they are missing from the current codebase.

@Kijewski
Copy link
Copy Markdown
Member

Sorry, but I don't think this is a good addition. Typst has multiple characters that need escaping depending on the context, at least newlines, but possibly also $, - and = as far as I can tell.

"Html",
),
(&["md", "none", "txt", "yml", ""], "Text"),
(&["md", "none", "txt", "typ", "yml", ""], "Text"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the contrary, I would say that we should remove md and yml from the list.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

I see, didn't expect that characters needed to be escaped.

@Kijewski
Copy link
Copy Markdown
Member

I don't know a lot of typst, but a user could do "malicious" things like injecting images #image(…) to exfiltrate data. The other examples like - (<li>), $ (enter math mode) are not that bad, but they could break formatting.

@xabi00
Copy link
Copy Markdown
Author

xabi00 commented Mar 12, 2025

Those characters don't need to be handled by askama because they belong to the typst (or markdown) compiler. Also, the special characters used in typst don't conflict with the special characters in askama.

@Kijewski
Copy link
Copy Markdown
Member

Those characters don't need to be handled by askama because they belong to the typst (or markdown) compiler. Also, the special characters used in typst don't conflict with the special characters in askama.

Then you could also say that <> don't need escaping in HTML, because they belong to the HTML parser. :-/

@xabi00
Copy link
Copy Markdown
Author

xabi00 commented Mar 12, 2025

But html is handled different to text

@Kijewski
Copy link
Copy Markdown
Member

Yeah, but typst is not text. @GuillaumeGomez, do you see that differently?

Maybe I'm too pedantic, but IMHO that default is not safe.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

I consider it "fine". There is no conflict with jinja syntax so as such so in my opinion, the rest needs to be handled by users, not us.

@xabi00
Copy link
Copy Markdown
Author

xabi00 commented Mar 13, 2025

I'm working on fixing the typos check and wanted to get your input. Do you prefer to ignore typos globally by adding 'typ' to the extend-words section in typos.toml, or would you rather use the extend-ignore-re to ignore just the specific lines related to extensions?

@Kijewski
Copy link
Copy Markdown
Member

Kijewski commented Mar 13, 2025

I consider it "fine". There is no conflict with jinja syntax so as such so in my opinion, the rest needs to be handled by users, not us.

Sorry, I veto this PR. If there was a conflict with jinja syntax was never even a question.

From a security perspective, this change is not fine, as it allows code injection. If we claim to handle typst files, then it is up to us to prevent that. That is exactly the same, without any difference at all, if we auto-escape chevrons in an HTML input "<script>", or the hash in a typst input "#image()".

For any format we claim to support, we need to provide a proper auto-escaper for the output of expressions. If we do not, then we cannot claim support for the format.

The user can always change their configuration to not do any escaping in .typ files, if they validated the input.

@Kijewski Kijewski requested a review from GuillaumeGomez March 13, 2025 11:14
@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

That is a valid reason indeed.

@xabi00
Copy link
Copy Markdown
Author

xabi00 commented Mar 13, 2025

I tried what @Kijewski proposed and it worked, so this solution is valid for me

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

So I guess we can close this PR? We should also have an example on how to implement the Escaper trait.

@Kijewski
Copy link
Copy Markdown
Member

I'll open an issue to add an Escaper example.

@Kijewski Kijewski closed this Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants