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/template: Parsing invalid templates with no actions should fail-fast #52906

Open
josharian opened this issue May 13, 2022 · 3 comments
Open

html/template: Parsing invalid templates with no actions should fail-fast #52906

josharian opened this issue May 13, 2022 · 3 comments
Labels
NeedsInvestigation
Milestone

Comments

@josharian
Copy link
Contributor

@josharian josharian commented May 13, 2022

Run:

package main

import (
	"html/template"
	"io"
	"log"
)

func main() {
	t := template.Must(template.New("errmsg").Parse(`<textarea/>`))
	err := t.Execute(io.Discard, nil)
	log.Print(err)
}

https://go.dev/play/p/oiAIres95vU

Result:

2009/11/10 23:00:00 html/template:errmsg: ends in a non-text context: {stateRCDATA delimNone urlPartNone jsCtxRegexp attrNone elementTextarea <nil>}

That error message reads more like an internal debugging message than something intended for users. It took me a while to narrow down the source of the problem.

cc @empijei @kele

@josharian
Copy link
Contributor Author

@josharian josharian commented May 14, 2022

...and on further reflection, since this template has no pipelines at all, it really ought to either fail at Parse time or execute successfully.

@empijei
Copy link
Contributor

@empijei empijei commented May 17, 2022

Hi,

Thanks for reporting this.

The problem here is twofold:

  1. The error message is not human-friendly
  2. The template is lazily evaluated, so the error only comes up when the template is executed.

About 1: #30635 is tracking this, and we should definitely address it (yes, I was the original reporter and then forgot about it by the time I started maintaining the package 🤦 ). Since this part of the bug would be a duplicate, I'm changing the title of the bug to only refer to the second point.

About 2: I think this is working as intended. Templates are not necessarily valid when parsed the first time, as they might only become so once sub-templates are added. We could special-case templates that contain no actions as you point out, but I'm not sure this is the best way forward. It would definitely improve UX, but it adds a bit complexity and any unit-test that covers the template at least once would provide the same value.

That said, I am not against such a change, but I'd be curious about @kele's opinion on it before we decide to address it.

@empijei empijei changed the title html/template: confusing error message when template contains <textarea/> html/template: Parsing invalid templates with no actions should fail-fast May 17, 2022
@josharian
Copy link
Contributor Author

@josharian josharian commented May 17, 2022

Thanks. Makes sense. I’m fine closing this as a dup of 30635, as long as we add this test case there.

@mknyszek mknyszek added the NeedsInvestigation label May 17, 2022
@mknyszek mknyszek added this to the Backlog milestone May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

3 participants