Navigation Menu

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: (*Template).Parse fails after an associated template has executed #15761

Closed
mstetson opened this issue May 19, 2016 · 6 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mstetson
Copy link

In html/template, if you parse a new template (e.g. t2) after an associated template (e.g. t1) has been executed, Parse returns the mysterious error html/template: cannot redefine "t1" after it has executed. This is mysterious, because t1 is likely neither the template you were trying to define nor the one you were directly associating with. See https://play.golang.org/p/khYvpOgq-6

This doesn't happen with text/template, and the docs don't make it sound like executing a template prevents adding new associated templates. The error message was added to html/template with the new block keyword in commit 12dfc3b.

I believe the behavior is incorrect. But if not, I suggest changing the error message to html/template: cannot (re)define any associated template after "t1" has executed.

@quentinmit quentinmit added this to the Go1.8 milestone Jun 17, 2016
@quentinmit
Copy link
Contributor

/cc @adg @bradfitz @robpike

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 17, 2016
@jessfraz
Copy link
Contributor

changing this behavior breaks this test: https://github.com/golang/go/blob/master/src/html/template/clone_test.go#L90 which makes me wonder if it is intentionally this way and then the docs need to be updated instead.

@mstetson
Copy link
Author

The test you reference checks redefinition of an existing template. That is disallowed by the documentation for (*Template).Parse. But the bug is that no new template may be added to a namespace once any template in that namespace has been executed. I doubt that behavior is intentional.

The bug can be illustrated concisely by adding this test near the one you reference:

    if _, err := t1.Parse(`{{define "unrelated"}} OK {{end}}`); err != nil {
        t.Errorf(`define "unrelated": got err %v want nil`, err)
    }

In Go 1.5, the test passes. In Go 1.6 and 1.7, the test fails with define "unrelated": got err html/template: cannot redefine "a" after it has executed want nil.

For context, here's how the bug bit me: I had a web server installed at client sites that did a fair amount of setup work before serving normal pages, including parsing the majority of its templates from files. During startup, most requests were answered by executing a template named "startup" that told the user that things would be operational soon. If there was an error, a template named "trouble" would be executed. The "startup" and "trouble" templates weren't read from external files; they were included in the Go source code to be available before other templates were parsed, and even if the template files were missing. All the templates were associated so there was only one namespace.

This worked fine in Go 1.4 and Go 1.5, but after updating to Go 1.6, clients started getting mysterious, rare failures with the error message cannot redefine "startup" after it has executed. It turned out that there was now a race: if a web request happened to come in before the external templates were parsed, then parsing them would fail, even though no templates were ever redefined.

@jessfraz
Copy link
Contributor

ah ok thanks this is helpful

@rsc rsc self-assigned this Oct 19, 2016
@rsc
Copy link
Contributor

rsc commented Oct 19, 2016

The rules introduced in Go 1.6 for html/template template redefinition are just fundamentally broken. It allows too much redefinition. A simpler rule - the one I'm going to send out - is that you can't call Parse or AddParseTree or redefine anything after the first execution of any template in the set. The idea is that on first execute, the template set as a whole gets analyzed, after which point it must be considered frozen.

We'll see if there's pushback, but that's at least correct.

@gopherbot
Copy link

CL https://golang.org/cl/31464 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 24, 2017
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants