casid / jte

Secure and speedy templates for Java and Kotlin.
https://jte.gg
Apache License 2.0
830 stars 62 forks source link

Support Dynamic Html Attribute Names #254

Closed wyaeld closed 1 year ago

wyaeld commented 1 year ago

Hi @casid

My Team has recently introduced StimulusJs to our Jte/Ktor application, and are finding a problem with the inability to build Html Attribute Names using expressions.

Stimulus is a JS project, part of the Hotwire project that's extracted from RubyOnRails, but isn't commonly found yet in other stacks. https://stimulus.hotwired.dev/

The core issue, is that Stimulus is a form of declarative controller binding, but uses both the name and value of html attributes to convey instructions, for instance (from their docs)

<div data-controller="hello">
  <input data-hello-target="name" type="text">

  <button data-action="click->hello#greet">
    Greet
  </button>

  <span data-hello-target="output">
  </span>
</div>

The data-hello-target is a specific binding to the target value of the hello-controller

We want to be able to build inputs, selects and other elements as Jte templates that can be reused, but supply things like the controller name as a variable to be passed in.

Is this something that Jte could gain support for?

casid commented 1 year ago

Hi @wyaeld,

that's cool, I wanted to try Hotwire/Stimulus with jte for a while, but haven't got the chance to do this yet. Is your project open source? If so, I'd be curious to have a look at it :-)

As for this issue, you should be able to create a custom HtmlPolicy to allow output in attribute names.

// Same as gg.jte.html.OwaspHtmlPolicy, but allows output in tags and attributes
public class MyHtmlPolicy extends PolicyGroup {
    public MyHtmlPolicy() {
        addPolicy(new PreventUppercaseTagsAndAttributes());
        // addPolicy(new PreventOutputInTagsAndAttributes());
        addPolicy(new PreventUnquotedAttributes());
    }
}

The policy can be set through gg.jte.TemplateEngine#setHtmlPolicy.

For precompiling with maven/gradle plugin the policy class need to be configured there as well, through htmlPolicyClass property.

wyaeld commented 1 year ago

It's an internal company system, so I can't share, but once we iron out these last kinks, I'm happy to write you up a short doc showing how to integrating ktor/jte/hotwire and stimulus, the glue components required are small. I think its a nice combination.

On your recommendations. Doing the above does work for a single attribute expression. We are running into this issue though

    @Test
    void dynamicAttribute2_for_hotwire() {
        class HotwireHtmlPolicy extends PolicyGroup {
            public HotwireHtmlPolicy() {
                addPolicy(new PreventUppercaseTagsAndAttributes());
                // addPolicy(new PreventOutputInTagsAndAttributes());
                addPolicy(new PreventUnquotedAttributes());
            }
        }
        templateEngine.setHtmlPolicy(new HotwireHtmlPolicy());
        codeResolver.givenCode("template.kte", "@param controller:String\n@param target:String\n\n<div data-controller=\"hello\">\n<input data-${controller}-target=\"${target}\"/></div>");

        templateEngine.render("template.kte", TemplateUtils.toMap("controller", "hello", "target", "name"), output);

        assertThat(output.toString()).isEqualTo("<div data-controller=\"hello\"><input data-hello-target=\"name\"/></div>");
    }

This fails with

[ERROR]   TemplateEngine_HtmlOutputEscapingTest.dynamicAttribute2_for_hotwire:1033 
expected: 
  "<div data-controller="hello"><input data-hello-target="name"/></div>"
 but was: 
  "
  <div data-controller="hello">
  <input data-${controller}-target="name"/></div>"

We are finding that we can't use expressions for both controller and expression at the same time. Curiously it just substitutes the 2nd one. Can you see why?

casid commented 1 year ago

Dang! Thanks for the test snippet. I'll have a look at it this weekend.

casid commented 1 year ago

@wyaeld I woke up early today and had a look at it. I've created a PR with a fix and some proposed solutions how to handle this in the most safe way possible. Let me know what you think!

wyaeld commented 1 year ago

Thanks @casid I'll try out the PR branch first thing tomorrow morning

casid commented 1 year ago

@wyaeld did you already get a chance to look into it?