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

Html atributes get escaped on formatting #293

Closed
ross96D opened this issue Nov 9, 2023 · 5 comments
Closed

Html atributes get escaped on formatting #293

ross96D opened this issue Nov 9, 2023 · 5 comments

Comments

@ross96D
Copy link
Contributor

ross96D commented Nov 9, 2023

Given this code i would write using alpinejs

image

When saving the file, so the lsp formating is call and it writes back this
image

I find this behavior uncomfortable. I would prefer the escape happens at generation time, and do no affect my templ file so i dont have to interpret what these characters mean.

I was looking at the source code and was able to craft a solution. I would be glad to submit a pull request if this behavior was not intended

@OrlandoCo
Copy link

Agree, most of the time attributes are not client inputs, and maybe if required is possible to create a helper like templ.Escape

@joerdav
Copy link
Collaborator

joerdav commented Nov 10, 2023

I believe this is only an issue with constant attributes attr="somestring", dynamic attrs (attr={s}) are already ran through templ.EscapeString.

This format feature was introduced recently in the html format package that templ depends on: a-h/htmlformat@5bd994f

One way to avoid this in your case @ross96D would be to define your string as a go string x-data={"{ var1='some-text' }"}.
Although I can see why this wouldn't be ideal.

@a-h
Copy link
Owner

a-h commented Nov 10, 2023

@joerdav - the HTML formatting change isn't actually related, the HTML format lib is only used in tests to compare minified templ output with unminified expected outputs.

Formatting in templ is done by parsing a templ file into an object model, and then writing it back out.

So, when parsing a HTML element opening tag, the attribute parser is used:

https://github.com/a-h/templ/blob/fb535826f48e00c23bc8c3d0adcd78c6fc4c1bb8/parser/v2/elementparser.go#L262C15-L282

The parser attempts to grab various types of attributes from the input,.

In the case of a constant attribute it returns a ConstantAttribute struct, which implements the Attribute interface, so it can be added to list of Attributes on the element struct.

Constant attribute parser:

var (
attributeConstantValueParser = parse.StringUntil(parse.Rune('"'))
attributeConstantValueSingleQuoteParser = parse.StringUntil(parse.Rune('\''))
constantAttributeParser = parse.Func(func(pi *parse.Input) (attr ConstantAttribute, ok bool, err error) {
start := pi.Index()
// Optional whitespace leader.
if _, ok, err = parse.OptionalWhitespace.Parse(pi); err != nil || !ok {
return
}
// Attribute name.
if attr.Name, ok, err = attributeNameParser.Parse(pi); err != nil || !ok {
pi.Seek(start)
return
}
// ="
result, ok, err := parse.Or(parse.String(`="`), parse.String(`='`)).Parse(pi)
if err != nil || !ok {
pi.Seek(start)
return
}
valueParser := attributeConstantValueParser
closeParser := parse.String(`"`)
if result.B.OK {
valueParser = attributeConstantValueSingleQuoteParser
closeParser = parse.String(`'`)
}
// Attribute value.
if attr.Value, ok, err = valueParser.Parse(pi); err != nil || !ok {
pi.Seek(start)
return
}
attr.Value = html.UnescapeString(attr.Value)
// " - closing quote.
if _, ok, err = Must(closeParser, fmt.Sprintf("missing closing quote on attribute %q", attr.Name)).Parse(pi); err != nil || !ok {
pi.Seek(start)
return
}
return attr, true, nil
})
)

Element struct:

templ/parser/v2/types.go

Lines 392 to 399 in fb53582

type Element struct {
Name string
Attributes []Attribute
IndentAttrs bool
Children []Node
IndentChildren bool
TrailingSpace TrailingSpace
}

To write out the formatted templ code, the attributes all have a Write(w io.Writer, indent int) error method on them.

In the case of the constant attribute, the contents are escaped...

templ/parser/v2/types.go

Lines 661 to 673 in fb53582

// href=""
type ConstantAttribute struct {
Name string
Value string
}
func (ca ConstantAttribute) String() string {
return ca.Name + `="` + html.EscapeString(ca.Value) + `"`
}
func (ca ConstantAttribute) Write(w io.Writer, indent int) error {
return writeIndent(w, indent, ca.String())
}

However, I don't believe they need to be, since the code is directly under the developer's control - i.e. was typed out by the developer.

The general concept of templ's security model is... don't mess with constants that the developer typed, escape any content that comes from a Go string, since the Go string could contain data that comes from an untrusted source.

In this case the constant attribute value is directly under the developer's control - i.e. was typed out by the developer, so I think it's OK to simply remove the escaping here (would need to check the tests).

@a-h
Copy link
Owner

a-h commented Nov 10, 2023

@ross96D - I forgot to say that I'd happily accept a PR for this change.

If you're too busy, please just say, so that someone else can feel free to pick it up. 😃

Thanks!

ross96D added a commit to ross96D/templ that referenced this issue Nov 11, 2023
a-h added a commit that referenced this issue Nov 14, 2023
a-h added a commit that referenced this issue Nov 14, 2023
@a-h
Copy link
Owner

a-h commented Nov 15, 2023

This was closed in the previous commits! Thanks for the PR.

@a-h a-h closed this as completed Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants