Can we add an option to disabled SafeClass ?

[gungun974]

The idea of not trusting user input by default is a good one.
Especially on suspicious character.

But for me I use a lot of TailwindCSS and the need to wrap nearly every class with SafeClass is overwhelm.

I don't want to change the default behavior of Templ.
No I prefer to propose to add an option to disable this behaviour so code using a lot of utility function with special character can be easy and quick to write.

And also with this option we can Imagine add an EscapeClass function to enable back the escape on specific part of our code.

Oh and when I say "an option", I mean in the templ generate command.

If necessary I will obviously see how to implement this but first I would like to know if this is something that is of interest or not.

[gungun974]

Just here is an example of a situation where by deactivating the basic behavior I could make the code more readable.

<div
  class={ 
    "absolute inset-x-0 bottom-0 flex items-center justify-between gap-2 p-6 py-4",
    templ.KV("flex-row-reverse", variant != "A"),
    templ.SafeClass("app-hero-slide-move translate-y-full opacity-0 transition duration-[750ms] ease-out"),
  }
>

[aranw]

I think I'd really like a option like this. What am building right now I can't ever imagine why I'd pass user input into the CSS classes. I'm also struggling to imagine a scenario where this could be possibly really dangerous for my application. I think there's valid concerns with scripts and urls. But I'm not sure about CSS classes?

[joerdav]

There are no additional restrictions on the tokens authors can use in the class attribute, but authors are encouraged to use values that describe the nature of the content, rather than values that describe the desired presentation of the content.

https://html.spec.whatwg.org/#classes

There's no rules in HTML on what a class can or can't be. Should we deprecate SafeClass and allow all characters that wouldn't break the validity of an element? Maybe just spaces and quotes shouldn't be allowed to avoid the possibility of xss:

class: " href="get-hacked

[joerdav]

Even google/safehtml doesn't sanitize classs names: https://github.com/google/safehtml/blob/be23134998433fcf0135dda53593fc8f8bf4df7c/template/sanitizers.go#L257

[gungun974]

There are no additional restrictions on the tokens authors can use in the class attribute, but authors are encouraged to use values that describe the nature of the content, rather than values that describe the desired presentation of the content.

https://html.spec.whatwg.org/#classes

There's no rules in HTML on what a class can or can't be. Should we deprecate SafeClass and allow all characters that wouldn't break the validity of an element? Maybe just spaces and quotes shouldn't be allowed to avoid the possibility of xss:

class: " href="get-hacked

Personally I agree to keep banning quote cause of course If you can inject in a Class and break the attribute, this could lead to XSS.

But I don't see why permit space would also lead to XSS ?
Can you provide an example where you broke an HTML class with a space ?

[joerdav]

Sorry, I worded that badly! We definitely should allow spaces in the class attribute as that's how you split class names. I don't believe there's a security risk to spaces.

[oliverpool]

Even google/safehtml doesn't sanitize classs names

But I believe class names are still somehow escaped to prevent <a class="{{ .MyVar}}"> with MyVar being outputted verbatim (like " href="get-hacked).

See ErrPredefinedEscaper in https://pkg.go.dev/github.com/google/[email protected]/template

Package html/template already contextually escapes all pipelines to produce HTML output safe against code injection. Manually escaping pipeline output using the predefined escapers "html" or "urlquery" is unnecessary, and may affect the correctness or safety of the escaped pipeline output in Go 1.8 and earlier.

---

Apparently the class name is already escaped in templ:

https://github.com/a-h/templ/blob/d917f3d2f872e907228b7996ac53fd598cf8c0b1/examples/counter/components/components_templ.go#L99C67-L99C79

I would also be in favor of dropping the SafeClass.

[gungun974]

Ok I just found using py-2.5 in Templ was consider as unsafe CSS class.
Even if Templ stick with the "safe" CSS class feature, maybe make this feature less strict cause it become ridiculous...

[a-h]

I checked against the behaviour of other sytems, such as React, and found that CSS class names were not sanitized, even if the CSS class names come from strings which are not under the developer's control.

As per @oliverpool's comment in #211 (comment) templ already HTML attribute encodes all class name values, so we think that this is enough.

To prevent breaking existing code, the SafeCSS and other functions will need to remain. It could be that we can deprecate those functions.

What do people think about the PR. As per expectations?

[joerdav]

LGTM, maybe we add some godoc deprecations for the unneeded bits of the runtime?

That way people's static check tools can let them know they can remove some code.

[gungun974]

@a-h Just thank you!
​
With the release of v0.2.476 I can now read my Tailwind classes with clarity and no longer have the fear of having to SafeClass everywhere!

I know it's unusual to thank someone in a closed ticket several days after but with the removal of 66 SafeClass from my project, this makes me so happy !!!

[a-h]

It's really nice of you to spend your time to do that. Thanks!