a-h / templ

A language for writing HTML user interfaces in Go.
https://templ.guide/
MIT License
8.28k stars 273 forks source link

parser: Allow exception of unsafe style attribute expressions. #454

Closed chrisbward closed 7 months ago

chrisbward commented 9 months ago

Unsure how to proceed here - as there's no mention in the docs regarding the style attribute, but an old ticket mentioning templ.RawCSS, and I wasn't able to get this to work - thanks!

iamajoe commented 9 months ago

You can go around with templ.Attributes.

https://github.com/a-h/templ/blob/main/parser/v2/types.go#L483 This is done for security purposes from my understanding, right templ team?

I've also got this issue and I am wondering though, how much of an issue it is to actually supoprt it?! What could be done? I don't mind working on this but I don't want to go against the direction of the templ team.

joerdav commented 9 months ago

Correct, this is a security control. But it does bring into question whether it should be escaped, similar to how we escape URL sanitisation.

iamajoe commented 9 months ago

In my opinion, I think there should be an way to escape. I can give an example where using style makes sense.

Lets say you have a loading bar of some sort. For onboarding for example. In general, the best way would be to setup a width: 74% but the "only" way for you to do that would be through style. You can't use the css as well because this value needs to stay dynamic.

This situation has happened to me several times while using templ. My solution was to use templ.Attributes but that was just a way for me to circumvent the safeties.

Style security becomes a little bit shaded when you think that you have full control over your code and what goes into style. To "ban" the usage on simple cases because of a very specific case seems a bad move?! That said, I like the idea of escape with sanitisation like it is done with URL.

joerdav commented 9 months ago

Would the proposal on the following help you, it's a slightly different solution: https://github.com/a-h/templ/issues/88

iamajoe commented 9 months ago

Ah yes, where you pass the argument to css? Definitely. I actually thought was possible until I figured that couldn't be done with current templ.

I still think that denying style is a bit of an opinionated option that removes flexibility out of the html usage. Even though I like your proposal, I still think that it is a bandaid (a cool and good one) around the denial of an attribute of the html standard.

I think your proposal and the the sanitize of style like it is done with URL would be invaluable. Just my 2 cents.

chrisbward commented 9 months ago

+1 for supporting inline safely

chrisbward commented 9 months ago

In my opinion, I think there should be an way to escape. I can give an example where using style makes sense.

Lets say you have a loading bar of some sort. For onboarding for example. In general, the best way would be to setup a width: 74% but the "only" way for you to do that would be through style. You can't use the css as well because this value needs to stay dynamic.

This situation has happened to me several times while using templ. My solution was to use templ.Attributes but that was just a way for me to circumvent the safeties.

Style security becomes a little bit shaded when you think that you have full control over your code and what goes into style. To "ban" the usage on simple cases because of a very specific case seems a bad move?! That said, I like the idea of escape with sanitisation like it is done with URL.

The use-case I'm having is exactly this, a progress bar, but within Bootstrap

MarhyCZ commented 9 months ago

So how would we safely handle a usecase like this:

I built a web component (Using Lit Element) and it takes array of strings as an attribute.

export class InlineAutocomplete extends LitElement {
  @property({ type: Array<String> }) options?: string[];

In HTML it should look like this:

<inline-autocomplete options='["Alpha", "Bravo"]'>
    <textarea type="text" class="w-full p-4..." name="transcript"></textarea>
</inline-autocomplete>

Templ escaped the string so I would need to do parsing in the Web Component, but it seems unnecessary for me as this way, it automatically parses attribute into the array property of strings.

So thanks to @iamajoe I tried it with templ.Attributes and it works.

<inline-autocomplete { templ.Attributes{"options": recservice.SuggestionsJSON()}... }>
    <textarea type="text" class="w-full p-4..." name="transcript"></textarea>
</inline-autocomplete>

The SuggestionsJSON would return this (simplified)

func SuggestionsJSON() string {
    bytes, _ := json.Marshal(suggestions)
    return string(bytes)
}

Is there a better Templ way? For me its safe enough as I am passing marshaled string.

a-h commented 9 months ago

@MarhyCZ - are you sure you need to do "parsing" in the web component?

Although the HTML is attribute encoded, that's just standard attribute encoding for quotes - there's nothing special there. The property isn't unreadable. The browser knows what to do, and JavaScript reads it as plaintext.

Screenshot 2024-02-04 at 15 36 25
package main

import "encoding/json"

func jsonOf(v any) string {
    b, _ := json.Marshal(v)
    return string(b)
}

templ Page(options []string) {
    <!DOCTYPE html>
    <html>
        <head>
            <title>Options</title>
        </head>
        <body>
            <div id="component" options={ jsonOf(options) }></div>
            <button onClick='alert(document.getElementById("component").getAttribute("options"))'>Show JSON</button>
        </body>
    </html>
}
package main

import (
    "context"
    "net/http"
    "os"

    "github.com/a-h/templ"
)

func main() {
    h := Page([]string{"a", "b"})
    h.Render(context.Background(), os.Stdout)
    http.ListenAndServe("localhost:8080", templ.Handler(h))
}

The HTML output is:

<!doctype html><html><head><title>Options</title></head><body><div id="component" options="[&#34;a&#34;,&#34;b&#34;]"></div><button onClick="alert(document.getElementById(&#34;component&#34;).getAttribute(&#34;options&#34;))">Show JSON</button></body></html>

This issue is specifically about syle attributes though.

Dynamic CSS properties were merged into the templ CLI thanks to @joerdav's PR. There's a few things to fix with the LSP support, then it's time for a new release.

MarhyCZ commented 9 months ago

I am so embarassed. I would swear, that LSP was returning an error that I need to wrap it inside templ.EscapeString. But now its totally fine. Sorry for spamming.

TemplStringArray

Thanks for such an awesome library <3 Its a real joy doing frontend for my diploma thesis using Go+Templ instead of black magic like NextJS.

a-h commented 9 months ago

No need to be embarrassed. You're not the first to think it wouldn't work because of how it looks. The output of the HTML attributes looks odd because it's not how a human would write it. 😀

MarhyCZ commented 9 months ago

Yeah, the &quot; which it generates now is totally ok. I thought I needed to use the templ.EscapeString function which encodes it into character entities &#34; - those are ofc not interpretable by default.

I can push a PR to include this example in docs if something similar is not already there. https://templ.guide/syntax-and-usage/attributes. e.g. "Passing data to attributes"

a-h commented 9 months ago

I'd love a PR on that if you've got time, thanks!

a-h commented 8 months ago

On this issue, I've sort-of lost track of what we want to do.

We now have CSS template args (https://templ.guide/syntax-and-usage/css-style-management#css-component-arguments), as of the latest release, and this morning I added support for marking CSS properties as safe in CSS template in https://github.com/a-h/templ/commit/8d4898441887f4155389b48ed93d09ab94b255e9 as well as fixing and issue with unquoted URLs in https://github.com/a-h/templ/commit/f3417e00176ff37a48af9f989737819e9e96aede

I want to build out an update to CSS, along the lines of https://github.com/a-h/templ/issues/262#issuecomment-1950287939 - basically, to be able to use variables directly in <style> tags without the need for the CSS component redirection, and make sure that it supports important use cases like psuedo-attributes and media queries from the get-go.

Critically important for me is to have a migration path that doesn't break people's code. That migration path might be eternal or temporary (hopefully not - everyone hates it when code breaks) backwards compatibility, a one-off migration tool, or all of the above.