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

Should all hrefs require templ.SafeURL to be consistent? #1091

Open
1 task done
drognisep opened this issue Mar 8, 2025 · 6 comments
Open
1 task done

Should all hrefs require templ.SafeURL to be consistent? #1091

drognisep opened this issue Mar 8, 2025 · 6 comments

Comments

@drognisep
Copy link

drognisep commented Mar 8, 2025

Before you begin

  • Please make sure you're using the latest version of the templ CLI (go install github.com/a-h/templ/cmd/templ@latest), and have upgraded your project to use the latest version of the templ runtime (go get -u github.com/a-h/templ@latest)
    • I tried using the latest CLI and runtime version, and I'm seeing the same behavior.

Describe the bug

First off, I'm not sure if this is a bug. I would expect that a link's href attribute would require a templ.SafeURL to be consistent with an anchor tag, but it seems to require a string instead.

I'm using a couple functions to add a URL path prefix to templ templates, and I noticed that static assets in the head element referenced by link allow (actually require due to how it's generated) a string.

One function returns a string (intended for HTMX attributes), and another returns a templ.SafeURL. I'm using the first variant without issue in link hrefs.

To Reproduce

Add a link element to a template (rel icon or stylesheet is what I'm seeing), and pass a plain Go string in curly braces ({}). There doesn't seem to be any requirement for a templ.SafeURL.

This is how I've produced this.

Expected behavior

I'd expect consistent behavior between hrefs for different elements, but there could be a good reason for the current behavior that I don't know about.

Edit: Trimming down text for clarity and adding specific links.

@joerdav
Copy link
Collaborator

joerdav commented Mar 10, 2025

This makes sense for consistency. I gree that the behaviour would make sense to be the same.

My concern here is backwards compatability, I can imagine that using dynamic strings in a link href is quite niche, I could still see us breaking some code if we introduced a type change. This would potentially have to go in an experiment flag with a warning of some kind if we went ahead.

@drognisep
Copy link
Author

drognisep commented Mar 13, 2025

My concern here is backwards compatability, I can imagine that using dynamic strings in a link href is quite niche, I could still see us breaking some code if we introduced a type change.

Sorry for the late reply, @joerdav.

I guess I was assuming that there's some level of switching/interpreting between behavior if I use a plain href in the template vs. a dynamic string. That seems to be the case for things like an anchor tag's href attribute, where it will accept a static string in the template if that's what I pass to it, but I must pass a templ.SafeURL if it's dynamic. If that same switching can be applied for a link's href attribute, then it seems unlikely there would be many broken by that (other than people like me that would prefer that behavior).

I also think there's an argument beyond consistency, where expecting a templ.SafeURL for a link href is more correct than accepting a string. An anchor tag is a more interactive component than a link, but it seems like it could be similarly exploited if not confirmed to be a safe URL.

@drognisep
Copy link
Author

It doesn't seem like passing an unsafe URL as a templ.SafeURL makes any semantic difference, but it makes me think about it. The fact that it makes me pause and consider is the real value to me.

@joerdav
Copy link
Collaborator

joerdav commented Mar 14, 2025

Yes there is differing behaviour between dynamic and constant attributes.

My backwards compatability concern is if someone currently is writing dynamic link hrefs they are able to do string interpolation: <link href={host+"/path"} />.

But if we introduced a need for it to be a templ.SafeURL then this would cause the above programs to not compile.

As you say, maybe this is okay as security in this case could be more important than backwards compatability.

@drognisep
Copy link
Author

As you say, maybe this is okay as security in this case could be more important than backwards compatability.

I would tend to agree, @joerdav, but I'm also not as familiar with the goals/priorities of the project. I'm just a guy that really enjoys the results. :)

I'm not sure what next steps would be for this, whether it finds itself on a backlog, or if there are other people that should weigh in. Based on your response, it seems like this is more in the realm of an enhancement. Is there anything else I can do to help justify/prioritize?

@joerdav
Copy link
Collaborator

joerdav commented Mar 17, 2025

Since this would be potentially a breaking change of some degree, I'd be interested to get the opinion of @a-h before making the call that we are ready for an implementation.

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

2 participants