Skip to content

[IMP] runtime: add markup tag function #1670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 25, 2025

Conversation

oomsveta
Copy link
Contributor

Allows markup to be called as a tag function. The interpolated strings are then safely escaped for injection in HTML code.

Example usage:

const maliciousInput = "<script>alert('💥💥')</script>";
const value = markup`<b>${maliciousInput}</b>`;
// no problem, maliciousInput is properly escaped

@oomsveta oomsveta force-pushed the master-tag_function_markup-wil branch 2 times, most recently from 399e5d2 to c06be40 Compare February 28, 2025 15:09
Copy link
Contributor

@kebeclibre kebeclibre left a comment

Choose a reason for hiding this comment

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

let's see what @jum-odoo thinks.

@oomsveta oomsveta force-pushed the master-tag_function_markup-wil branch 2 times, most recently from c6015d6 to aa1db2d Compare March 4, 2025 08:59
@oomsveta oomsveta force-pushed the master-tag_function_markup-wil branch from aa1db2d to e754f3d Compare March 6, 2025 13:05
Allows markup to be called as a tag function. The interpolated strings
are then safely escaped for injection in HTML code.

Example usage:
```js
const maliciousInput = "<script>alert('💥💥')</script>";
const value = markup`<b>${maliciousInput}</b>`;
// no problem, maliciousInput is properly escaped
```
@oomsveta oomsveta force-pushed the master-tag_function_markup-wil branch from e754f3d to 80d882a Compare March 6, 2025 13:42
@seb-odoo
Copy link
Contributor

markup-aware escape should be exported so we can use it in Odoo in place of htmlEscape that serves the exact same purpose.

See odoo/odoo#199300 for a proof of concept in Odoo of this PR.

@ged-odoo ged-odoo merged commit fd3c194 into odoo:master Mar 25, 2025
2 checks passed
@seb-odoo
Copy link
Contributor

Thank you for accepting it, it is a nice improvement!
This will remove the need of overriding the ci/security in many cases by having safer code by default.

I still wish htmlEscape was exported from here rather than having to re-define exactly the same function in Odoo though. I guess I can make a PR for it now that this one is merged.

@ged-odoo
Copy link
Contributor

we need to do something about markup and eventbus at least. Ideally, it should be accessible in some parts of odoo without the need for owl, for example, the website. there is a task for that though https://www.odoo.com/odoo/project/133/tasks/4364029?debug=assets

@seb-odoo
Copy link
Contributor

seb-odoo commented Mar 25, 2025

Should we start re-exporting markup in html.js utils (from odoo) and use this import in business code rather than the one from owl then?

@seb-odoo
Copy link
Contributor

Export of the escape function + tests: #1677

I still think it has value until a proper solution to the above-mentioned task is found.

Feel free to discard the PR, as we already have this function inside Odoo, it's not really "blocking".

@seb-odoo
Copy link
Contributor

And a final request on this matter, would it be possible to make a release with the current PR (and the escape one, if it is approved) one of these days? 🙏

I'd love to start using the new syntax in Odoo whenever possible! We do have quite a few markup-sensitive PR in the way (especially the composer one), and I'd like to rebase and finish odoo/odoo#199300 too.

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.

6 participants