Conversation
5ff3b12 to
49392bf
Compare
<form action={ safeUrl() }>With Error</form>The generated code looks like this: templ/generator/test-form-action/template_templ.go Lines 63 to 67 in 49392bf Can I propose to remove the var templ_7745c5c3_Var4 string
templ_7745c5c3_Var4, templ_7745c5c3_Err = templ.JoinStringErrs(safeUrl())
if templ_7745c5c3_Err != nil {
return templ.Error{Err: templ_7745c5c3_Err, FileName: `generator/test-form-action/template.templ`, Line: 7, Col: 25}
} This will allow both templ/generator/test-form-action/template.templ Lines 11 to 17 in 49392bf |
49392bf to
1b45c44
Compare
aa4a1a4 to
b2113fb
Compare
There was a problem hiding this comment.
Pull Request Overview
Adds error-aware URL sanitation by introducing JoinURLErrs and updating generated templates to propagate URL errors.
- Define
JoinURLErrsin url.go to sanitize or bypass based on input type and combine errors. - Update
EscapeStringto a generic signature accepting any string-like type. - Regenerate template and generator code across tests, examples, and benchmarks to call
JoinURLErrs, insert error handlers, and adjust expected outputs.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| url.go | Introduce JoinURLErrs for sanitizing URLs and bundling errors. |
| url_test.go | Add unit tests for JoinURLErrs covering sanitization and error joins. |
| runtime.go | Change EscapeString to generic form (T ~string). |
| generator/generator.go | Update code generator to emit JoinURLErrs calls and error checks. |
| generator/test-html/template_templ.go | Regenerated HTML template to use JoinURLErrs and handle errors. |
| generator/test-form-action/template_templ.go | Regenerated form-action template with error-handling around URLs. |
| generator/test-form-action/template.templ | Add safeUrl/stringUrl helpers in template to test error variants. |
| generator/test-form-action/expected.html | Adjust expected output for form-action tests with error cases. |
| generator/test-attribute-escaping/template_templ.go | Regenerated attribute-escaping test template for JoinURLErrs. |
| generator/test-a-href/template_templ.go | Regenerated anchor-href test template to use JoinURLErrs. |
| examples/static-generator/blog_templ.go | Update example generator to propagate URL errors in blog links. |
| cmd/templ/lspcmd/httpdebug/list_templ.go | Update HTTP-debug template to join URL errors in link generation. |
| benchmarks/templ/template_templ.go | Update benchmark template to use JoinURLErrs with error checks. |
Comments suppressed due to low confidence (2)
url.go:25
- [nitpick] Consider enhancing this doc comment to mention that SafeURL inputs bypass sanitization and that incoming errors are combined using errors.Join.
// JoinURLErrs joins an optional list of errors and returns a sanitized SafeURL.
generator/test-form-action/template.templ:11
- [nitpick] Function name 'safeUrl' should use the 'URL' acronym in uppercase (e.g., 'safeURL') to align with Go naming conventions.
func safeUrl(s string) (templ.SafeURL, error) {
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Looks good to me, makes the behaviour consistent with other attributes, I like it. Just has a failing test is all. |
| ```templ | ||
| templ component(contact model.Contact) { | ||
| <div hx-get={ string(templ.URL(fmt.Sprintf("/contacts/%s/email", contact.ID)))}> | ||
| <div hx-get={ templ.URL(fmt.Sprintf("/contacts/%s/email", contact.ID)) }> |
There was a problem hiding this comment.
According to the description above, this should just be fmt.Sprintf("/contacts/%s/email", contact.ID), URL sanitising is automatic
There was a problem hiding this comment.
It's not automatic for hx-get because it's a non-standard attribute.
templ could have a list of "well known" attribute names that contain URLs, but since data-* URLs can contain them, it wouldn't be much use.
So, I think this documentation is accurate.
There was a problem hiding this comment.
oh yeah, you're right!
templ could have a list of "well known"
I'm ok with current behaviour. Having the "well known" creates possibilities of conflict. And will always not be enough as different people will want to have their favourite framework supported.
fixes #1167
Thi PR changes form action and anchor href to use SafeURL with error