Skip to content
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

Qute @CheckedTemplate ignores SELECTED_VARIANT for HTML escaping #21730

Closed
famod opened this issue Nov 26, 2021 · 7 comments
Closed

Qute @CheckedTemplate ignores SELECTED_VARIANT for HTML escaping #21730

famod opened this issue Nov 26, 2021 · 7 comments
Labels
area/qute The template engine kind/bug Something isn't working

Comments

@famod
Copy link
Member

famod commented Nov 26, 2021

Describe the bug

I have xhtml templates for generation of emails and pdf conversion.

To avoid a warning message I have set the proper content-type:

  qute:
    template-path-exclude: '.*(\.jpg|\.properties)'
    suffixes:
      - xhtml
    content-types:
      xhtml: application/xhtml+xml

The render call tooks like this:

templateInstace.setAttribute(TemplateInstance.SELECTED_VARIANT, VARIANT).render();

Variant:

new Variant(Locale.GERMANY, Variant.TEXT_HTML, StandardCharsets.UTF_8.name())

This variant seems to be ignored because I can only see the implicitly created variant with application/xhtml+xml being passed to HtmlEscaper, not my selected variant.
Since escaping is hard-coded to be only done for text/html and text/xml, there will be no escaping here.

Workaround:

    content-types:
      xhtml: text/xml

(or html)

But what about locale and charset? I'm under the impression that the selected variant is irgnored for everything, not just html escaping?

Expected behavior

Selected variant should be used or if this is not supported by design, an actionable exception should be thrown and documentation should be extended.

Actual behavior

Selected variant is ignored silently.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.5.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/Qute.20and.20XML.20templates.20.28char.20escaping.20issues.29/near/262782613

Somewhat related: #21405

@famod famod added the kind/bug Something isn't working label Nov 26, 2021
@quarkus-bot quarkus-bot bot added the area/qute The template engine label Nov 26, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 26, 2021

/cc @mkouba

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 26, 2021

You added a link to a Zulip discussion, please make sure the description of the issue is comprehensive and doesn't require accessing Zulip.

This message is automatically generated by a bot.

@mkouba
Copy link
Contributor

mkouba commented Nov 26, 2021

Well, the selected variant has no effect because the HtmlEscaper only considers the variant of the originating template; in your case it would be application/xhtml+xml.

The selected variant is only used to choose the correct template for an injected Template or @CheckedTemplate. In other words, if you do @Inject Template foo the selected variant can e.g. help to choose the foo.html or the foo.txt template but it does not affect whether the template should be escaped or not. And it would not make any sense if it does IMO.

But what about locale and charset? I'm under the impression that the selected variant is irgnored for everything, not just html escaping?

Locale and charset negotiation is not implemented yet and it's a known issue.

@famod
Copy link
Member Author

famod commented Nov 26, 2021

In other words, if you do @Inject Template foo the selected variant can e.g. help to choose the foo.html or the foo.txt template but it does not affect whether the template should be escaped or not. And it would not make any sense if it does IMO.

But isn't "html or txt" a perfect example that shows escaping should respect the selected variant? I mean, txt doesn't need escaping, but html does.

@mkouba
Copy link
Contributor

mkouba commented Nov 26, 2021

In other words, if you do @Inject Template foo the selected variant can e.g. help to choose the foo.html or the foo.txt template but it does not affect whether the template should be escaped or not. And it would not make any sense if it does IMO.

But isn't "html or txt" a perfect example that shows escaping should respect the selected variant? I mean, txt doesn't need escaping, but html does.

Indeed, but for foo.html we detect text/html -> escaped and for foo.txt we detect text/plain -> not escaped. It's the content of the template that is important and not the selected variant which is only part of the negotiation which template/content to choose... At least that's my understanding ;-).

@famod
Copy link
Member Author

famod commented Nov 26, 2021

In other words, if you do @Inject Template foo the selected variant can e.g. help to choose the foo.html or the foo.txt template but it does not affect whether the template should be escaped or not. And it would not make any sense if it does IMO.

But isn't "html or txt" a perfect example that shows escaping should respect the selected variant? I mean, txt doesn't need escaping, but html does.

Indeed, but for foo.html we detect text/html -> escaped and for foo.txt we detect text/plain -> not escaped.

Only for known content-types, though (which cover most cases but not all).
But I guess one main takeaway is that escaping will be configurable per content-type via #21731 and the (default) locale via #21405, so there shouldn't be the need anymore to set either one via SELECTED_VARIANT.

@famod famod changed the title Qute @CheckedTemplates ignore SELECTED_VARIANT for HTML escaping Qute @CheckedTemplate ignores SELECTED_VARIANT for HTML escaping Nov 27, 2021
@mkouba
Copy link
Contributor

mkouba commented Dec 1, 2021

@famod I'm going to close this one. Feel free to reopen if needed.

@mkouba mkouba closed this as completed Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/qute The template engine kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants