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

Jump to bottom

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 Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@josharian
Copy link
Contributor

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

...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 commented May 17, 2022 •
edited

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

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 Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. 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
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@josharian @mknyszek @empijei