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: AddParseTree does not set root template correctly #48477

Open
bluekeyes opened this issue Sep 20, 2021 · 1 comment
Open

html/template: AddParseTree does not set root template correctly #48477

bluekeyes opened this issue Sep 20, 2021 · 1 comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@bluekeyes
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.17.1 linux/amd64

This was also an issue on go1.16. I haven't tested any older releases.

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/bkeyes/.cache/go-build"
GOENV="/home/bkeyes/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/bkeyes/code/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/bkeyes/code/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build447354891=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.17.1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.17.1
uname -sr: Linux 5.10.16.3-microsoft-standard-WSL2
Distributor ID: Ubuntu
Description:    Ubuntu 20.04.2 LTS
Release:        20.04
Codename:       focal
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Ubuntu GLIBC 2.31-0ubuntu9.2) stable release version 2.31.

Also reproduces on the Go Playground.

What did you do?

Used AddParseTree to set the content of the root/default template then tried to execute it.

https://play.golang.org/p/4UPROPWwFFA

package main

import (
	"fmt"
	html "html/template"
	"os"
	text "text/template"
)

func main() {
	base := text.Must(text.New("base").Parse("template content\n"))
	
	tcopy := text.New("base")
	text.Must(tcopy.AddParseTree("base", base.Tree))
	
	hcopy := html.New("base")
	html.Must(hcopy.AddParseTree("base", base.Tree))

	fmt.Println("== text ==")
	if err := tcopy.ExecuteTemplate(os.Stdout, "base", nil); err != nil {
		panic(err)
	}
	if err := tcopy.Execute(os.Stdout, nil); err != nil {
		panic(err)
	}
	
	fmt.Println("== html ==")
	if err := hcopy.ExecuteTemplate(os.Stdout, "base", nil); err != nil {
		panic(err)
	}
	if err := hcopy.Execute(os.Stdout, nil); err != nil {
		panic(err)
	}
}

What did you expect to see?

Executing the template prints template content\n to standard out, like it does for a text/template.Template created using the same methods.

What did you see instead?

Executing the HTML template returns an error: template: "base" is an incomplete or empty template.

Additional Information

Note that executing the HTML template by name works as expected. This seems to be because the HTML implementation of AddParseTree does not set the Tree field of the Template struct. If I manually set this field, it appears to fix the initial error, but this might miss fixing other internal state.

I encountered this while building a utility for template inheritance. I used AddParseTree because Clone does not support changing the template name and I was trying to avoid re-parsing the base template(s) for each leaf in the tree.

@dr2chase
Copy link
Contributor

@empijei can you give this a look?

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 21, 2021
bluekeyes added a commit to bluekeyes/templatetree that referenced this issue Sep 26, 2021
Using AddParseTree works correctly for text templates, but has a bug
that breaks all HTML templates (golang/go#48477). Attempting to
workaround that bug using the public interface appears to work at first,
but leads to even stranger errors rendering some templates.

While I wanted to avoid parsing templates multiple times, it resolves
all issues I've found so far and simplifies the code, so use it for now.
In most application, parsing templates should be a one-time cost anyway.

Also add some new unit tests for HTML templates that failed when using
the old logic.
bluekeyes added a commit to bluekeyes/templatetree that referenced this issue Sep 26, 2021
Using AddParseTree works correctly for text templates, but has a bug
that breaks all HTML templates (golang/go#48477). Attempting to
workaround that bug using the public interface appears to work at first,
but leads to even stranger errors rendering some templates.

While I wanted to avoid parsing templates multiple times, it resolves
all issues I've found so far and simplifies the code, so use it for now.
In most application, parsing templates should be a one-time cost anyway.

Also add some new unit tests for HTML templates that failed when using
the old logic.
@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants