Support Dynamic Html Attribute Names

[casid]

For hotwire development it is required to have dynamic attribute names in certain cases. See issue #254

This is prevented by OwaspHtmlPolicy, or more specifically the policy PreventOutputInTagsAndAttributes. This is because attribute names are not a safe slot to put untrusted user input into. There are various ways to escape the attribute name.

This PR fixes a bug in jte, so that correct output is generated, if this policy is disabled by the user and dynamic output is used in an attribute name.

There are currently two ways to achieve this:

• Custom HTML Policy that disables or weakens the PreventOutputInTagsAndAttributes policy
• Using $unsafe for the dynamic output in the attribute name

@wyaeld during implementation I noticed that my initial recommendation to disable the PreventOutputInTagsAndAttributes policy entirely does remove a few more policies that are quite useful and I think that most users will want to have, such as preventing dynamic tag names and preventing control structures and content blocks in attribute names.

That's why I've parameterized the PreventOutputInTagsAndAttributes, to make it possible to only disable the check that you need, like this: https://github.com/casid/jte/blob/66dbec00f9be247e06a200e531b2a3ecdd2a0c08/jte-kotlin/src/test/java/gg/jte/kotlin/TemplateEngine_HtmlOutputEscapingTest.java#L1419

As an alternative, you could also use $unsafe for these parts and stick with the default OwaspHtmlPolicy, like here: https://github.com/casid/jte/blob/66dbec00f9be247e06a200e531b2a3ecdd2a0c08/jte-kotlin/src/test/java/gg/jte/kotlin/TemplateEngine_HtmlOutputEscapingTest.java#L1448

[codecov[bot]]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% :tada:

Comparison is base (3e4c084) 90.97% compared to head (66dbec0) 90.98%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #256 +/- ## ============================================ + Coverage 90.97% 90.98% +0.01% - Complexity 1174 1176 +2 ============================================ Files 74 74 Lines 3091 3096 +5 Branches 478 479 +1 ============================================ + Hits 2812 2817 +5 Misses 172 172 Partials 107 107 ``` | [Files Changed](https://app.codecov.io/gh/casid/jte/pull/256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager) | Coverage Δ | | |---|---|---| | [...va/gg/jte/compiler/kotlin/KotlinCodeGenerator.java](https://app.codecov.io/gh/casid/jte/pull/256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager#diff-anRlLWtvdGxpbi9zcmMvbWFpbi9qYXZhL2dnL2p0ZS9jb21waWxlci9rb3RsaW4vS290bGluQ29kZUdlbmVyYXRvci5qYXZh) | `97.15% <100.00%> (ø)` | | | [.../html/policy/PreventOutputInTagsAndAttributes.java](https://app.codecov.io/gh/casid/jte/pull/256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager#diff-anRlLXJ1bnRpbWUvc3JjL21haW4vamF2YS9nZy9qdGUvaHRtbC9wb2xpY3kvUHJldmVudE91dHB1dEluVGFnc0FuZEF0dHJpYnV0ZXMuamF2YQ==) | `100.00% <100.00%> (ø)` | | | [.../src/main/java/gg/jte/compiler/TemplateParser.java](https://app.codecov.io/gh/casid/jte/pull/256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager#diff-anRlL3NyYy9tYWluL2phdmEvZ2cvanRlL2NvbXBpbGVyL1RlbXBsYXRlUGFyc2VyLmphdmE=) | `91.77% <100.00%> (+0.01%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[wyaeld]

Been a few days away from keyboard. Ran things this morning.

Seems to work pretty well. I'm usually wary about single boolean switched on contructors, but looking at the code I don't see anything obvious that looks better than what you've done there.

I was also exploring the ergonomics of what happens when we don't have params for it, to try and avoid having to include if/else conditional chunks in the templates. My understanding was that attributes without values would get no-oped by JTE, but I think I'm misunderstanding.

What should happen here? I haven't changed the assert from the base test, but wasn't expecting the attributes to render at all.

@Test
        void attributes_dynamicNameForHotwireWithEmptyDefaultsNoParams() {
            codeResolver.givenCode("template.kte", "@param controller:String = \"\"\n@param target:String=\"\"\n<div data-controller=\"${controller}\">" +
             "\n<input data-${controller}-target=\"${target}\"/></div>");

            templateEngine.render("template.kte", TemplateUtils.toMap(), output);

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

This result

org.opentest4j.AssertionFailedError: 
expected: 
  "<div>
  <input/></div>"
 but was: 
  "<div data-controller="">
  <input data--target=""/></div>"

[wyaeld]

Rereading https://github.com/casid/jte/blob/main/DOCUMENTATION.md#smart-attributes

If I made the default values null instead of "" would it work? Will test

Edit: Almost works, but the value needs to be quoted and then renders as ""

@Test
        void attributes_dynamicNameForHotwireWithEmptyDefaultsNoParams() {
            codeResolver.givenCode("template.kte", "@param controller:String?=null\n@param target:String?=null\n<div data-controller=\"${controller}\">" +
             "<input data-${controller}-target=\"${target}\"/></div>");

            templateEngine.render("template.kte", TemplateUtils.toMap(), output);

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

org.opentest4j.AssertionFailedError: 
expected: "<div><input/></div>"
 but was: "<div><input data--target=""/></div>"
Expected :"<div><input/></div>"
Actual   :"<div><input data--target=""/></div>"

I can't do this

<input data-${controller}-target=${target}/>

Because it triggers gg.jte.TemplateException: Failed to compile template.kte, error at line 3: Unquoted HTML attribute values are not allowed: data-${controller}-target

I know we could do if/else wrapping and render 2 different div blocks, but still thinking that gets a little awkward if the templates get busy

[casid]

@wyaeld nope, smart attributes are not working atm when attributes names are dynamic.

This is because smart attributes currently check at compile time if the attribute is a boolean or regular html attribute. In case of boolean it generates different code. That's not possible when the attribute name is not present at compile time.

What's your usecase exactly? Is the controller variable expected to be null, or the target variable?

[wyaeld]

Just exploring different ways to organise our templates, and whether we need to have hotwire and non-hotwire variants of common controls, like inputs, selects etc.

What you've done with this PR gives us good building blocks though, its a big improvement.

[casid]

Okay, I'll then merge this branch so that it is available in the next bugfix release.

We can still try to add more features later, if needed.