html/template: (*Template).Parse fails after an associated template has executed

[mstetson]

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]

/cc @adg @bradfitz @robpike

[jessfraz]

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]

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]

ah ok thanks this is helpful

[rsc]

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]

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