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: improve ErrBadHTML error report #19408

Open
perillo opened this issue Mar 5, 2017 · 8 comments

Comments

@perillo
Copy link

commented Mar 5, 2017

Please answer these questions before submitting your issue. Thanks!

What did you do?

Recently I got the ErrBadHTML error 2-3 times, and and each time it took longer than expected to fix the typo, since there was no line reported in the error message.

Here is an example:
https://play.golang.org/p/syrtLCXWeE

What did you expect to see?

The error message should report the line were the error was found.

What did you see instead?

The error message only reports

"<" in attribute name: " selected<"

but this does not help when searching for the error in a big template.

Looking at the source code, it is possible that this issue does not only affect ErrBadHTML but also other errors.

System details

go version go1.8 linux/amd64
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/code/src/go"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build656653276=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
GOROOT/bin/go version: go version go1.8 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.8 X:framepointer
uname -sr: Linux 4.9.11-1-ARCH
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.24, by Roland McGrath et al.
gdb --version: GNU gdb (GDB) 7.12.1
@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 21, 2017

@robpike, thoughts on line numbers in error messages?

@robpike

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2017

This is html/template, which I no longer maintain. The Nodes have the information, so line numbers could be added. They are present in messages from text/template/parse, and so delivered in parse errors. The html/package needs to use them in its own messages. Should be easy.

@robpike robpike removed their assignment Mar 21, 2017

@bradfitz bradfitz added this to the Go1.9Maybe milestone Mar 21, 2017

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2017

Anyone is welcome to do this (for Go 1.10; Go 1.9 is closed).

@rsc rsc modified the milestones: Go1.10, Go1.9Maybe Jun 5, 2017

@rsc rsc added NeedsFix and removed NeedsDecision labels Jun 5, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Jul 19, 2017

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

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 13, 2017

For Go 1.11, I think we could consider the following, which threads the current parse.Node into the code where that error is generated. Then the location information is for the start of the surrounding text node, which maybe is better or maybe is not. But it is probably the best we can do. CL 49990 is too invasive.

r$ git diff
diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index b51a37039b..e0fa105150 100644
--- a/src/html/template/escape.go
+++ b/src/html/template/escape.go
@@ -637,7 +637,7 @@ var doctypeBytes = []byte("<!DOCTYPE")
 func (e *escaper) escapeText(c context, n *parse.TextNode) context {
 	s, written, i, b := n.Text, 0, 0, new(bytes.Buffer)
 	for i != len(s) {
-		c1, nread := contextAfterText(c, s[i:])
+		c1, nread := contextAfterText(n, c, s[i:])
 		i1 := i + nread
 		if c.state == stateText || c.state == stateRCDATA {
 			end := i1
@@ -703,16 +703,16 @@ func (e *escaper) escapeText(c context, n *parse.TextNode) context {
 
 // contextAfterText starts in context c, consumes some tokens from the front of
 // s, then returns the context after those tokens and the unprocessed suffix.
-func contextAfterText(c context, s []byte) (context, int) {
+func contextAfterText(n parse.Node, c context, s []byte) (context, int) {
 	if c.delim == delimNone {
-		c1, i := tSpecialTagEnd(c, s)
+		c1, i := tSpecialTagEnd(n, c, s)
 		if i == 0 {
 			// A special end tag (`</script>`) has been seen and
 			// all content preceding it has been consumed.
 			return c1, 0
 		}
 		// Consider all content up to any end tag.
-		return transitionFunc[c.state](c, s[:i])
+		return transitionFunc[c.state](n, c, s[:i])
 	}
 
 	// We are at the beginning of an attribute value.
@@ -742,7 +742,7 @@ func contextAfterText(c context, s []byte) (context, int) {
 		//     <button onclick="alert(&quot;Hi!&quot;)">
 		// without having to entity decode token boundaries.
 		for u := []byte(html.UnescapeString(string(s))); len(u) != 0; {
-			c1, i1 := transitionFunc[c.state](c, u)
+			c1, i1 := transitionFunc[c.state](n, c, u)
 			c, u = c1, u[i1:]
 		}
 		return c, len(s)
diff --git a/src/html/template/html.go b/src/html/template/html.go
index de4aa4abb2..b1eae7c24b 100644
--- a/src/html/template/html.go
+++ b/src/html/template/html.go
@@ -179,7 +179,7 @@ func stripTags(html string) string {
 			if c.element != elementNone && !isInTag(st) {
 				st = stateRCDATA
 			}
-			d, nread := transitionFunc[st](c, s[i:])
+			d, nread := transitionFunc[st](nil, c, s[i:])
 			i1 := i + nread
 			if c.state == stateText || c.state == stateRCDATA {
 				// Emit text up to the start of the tag or comment.
diff --git a/src/html/template/transition.go b/src/html/template/transition.go
index df7ac2289b..f875e7cf53 100644
--- a/src/html/template/transition.go
+++ b/src/html/template/transition.go
@@ -7,13 +7,14 @@ package template
 import (
 	"bytes"
 	"strings"
+	"text/template/parse"
 )
 
 // transitionFunc is the array of context transition functions for text nodes.
 // A transition function takes a context and template text input, and returns
 // the updated context and the number of bytes consumed from the front of the
 // input.
-var transitionFunc = [...]func(context, []byte) (context, int){
+var transitionFunc = [...]func(parse.Node, context, []byte) (context, int){
 	stateText:        tText,
 	stateTag:         tTag,
 	stateAttrName:    tAttrName,
@@ -44,7 +45,7 @@ var commentStart = []byte("<!--")
 var commentEnd = []byte("-->")
 
 // tText is the context transition function for the text state.
-func tText(c context, s []byte) (context, int) {
+func tText(n parse.Node, c context, s []byte) (context, int) {
 	k := 0
 	for {
 		i := k + bytes.IndexByte(s[k:], '<')
@@ -82,7 +83,7 @@ var elementContentType = [...]state{
 }
 
 // tTag is the context transition function for the tag state.
-func tTag(c context, s []byte) (context, int) {
+func tTag(n parse.Node, c context, s []byte) (context, int) {
 	// Find the attribute name.
 	i := eatWhiteSpace(s, 0)
 	if i == len(s) {
@@ -94,7 +95,7 @@ func tTag(c context, s []byte) (context, int) {
 			element: c.element,
 		}, i + 1
 	}
-	j, err := eatAttrName(s, i)
+	j, err := eatAttrName(n, s, i)
 	if err != nil {
 		return context{state: stateError, err: err}, len(s)
 	}
@@ -129,8 +130,8 @@ func tTag(c context, s []byte) (context, int) {
 }
 
 // tAttrName is the context transition function for stateAttrName.
-func tAttrName(c context, s []byte) (context, int) {
-	i, err := eatAttrName(s, 0)
+func tAttrName(n parse.Node, c context, s []byte) (context, int) {
+	i, err := eatAttrName(n, s, 0)
 	if err != nil {
 		return context{state: stateError, err: err}, len(s)
 	} else if i != len(s) {
@@ -140,7 +141,7 @@ func tAttrName(c context, s []byte) (context, int) {
 }
 
 // tAfterName is the context transition function for stateAfterName.
-func tAfterName(c context, s []byte) (context, int) {
+func tAfterName(n parse.Node, c context, s []byte) (context, int) {
 	// Look for the start of the value.
 	i := eatWhiteSpace(s, 0)
 	if i == len(s) {
@@ -164,7 +165,7 @@ var attrStartStates = [...]state{
 }
 
 // tBeforeValue is the context transition function for stateBeforeValue.
-func tBeforeValue(c context, s []byte) (context, int) {
+func tBeforeValue(n parse.Node, c context, s []byte) (context, int) {
 	i := eatWhiteSpace(s, 0)
 	if i == len(s) {
 		return c, len(s)
@@ -182,7 +183,7 @@ func tBeforeValue(c context, s []byte) (context, int) {
 }
 
 // tHTMLCmt is the context transition function for stateHTMLCmt.
-func tHTMLCmt(c context, s []byte) (context, int) {
+func tHTMLCmt(n parse.Node, c context, s []byte) (context, int) {
 	if i := bytes.Index(s, commentEnd); i != -1 {
 		return context{}, i + 3
 	}
@@ -205,7 +206,7 @@ var (
 
 // tSpecialTagEnd is the context transition function for raw text and RCDATA
 // element states.
-func tSpecialTagEnd(c context, s []byte) (context, int) {
+func tSpecialTagEnd(n parse.Node, c context, s []byte) (context, int) {
 	if c.element != elementNone {
 		if i := indexTagEnd(s, specialTagEndMarkers[c.element]); i != -1 {
 			return context{}, i
@@ -240,12 +241,12 @@ func indexTagEnd(s []byte, tag []byte) int {
 }
 
 // tAttr is the context transition function for the attribute state.
-func tAttr(c context, s []byte) (context, int) {
+func tAttr(n parse.Node, c context, s []byte) (context, int) {
 	return c, len(s)
 }
 
 // tURL is the context transition function for the URL state.
-func tURL(c context, s []byte) (context, int) {
+func tURL(n parse.Node, c context, s []byte) (context, int) {
 	if bytes.ContainsAny(s, "#?") {
 		c.urlPart = urlPartQueryOrFrag
 	} else if len(s) != eatWhiteSpace(s, 0) && c.urlPart == urlPartNone {
@@ -257,7 +258,7 @@ func tURL(c context, s []byte) (context, int) {
 }
 
 // tJS is the context transition function for the JS state.
-func tJS(c context, s []byte) (context, int) {
+func tJS(n parse.Node, c context, s []byte) (context, int) {
 	i := bytes.IndexAny(s, `"'/`)
 	if i == -1 {
 		// Entire input is non string, comment, regexp tokens.
@@ -294,7 +295,7 @@ func tJS(c context, s []byte) (context, int) {
 
 // tJSDelimited is the context transition function for the JS string and regexp
 // states.
-func tJSDelimited(c context, s []byte) (context, int) {
+func tJSDelimited(n parse.Node, c context, s []byte) (context, int) {
 	specials := `\"`
 	switch c.state {
 	case stateJSSqStr:
@@ -347,7 +348,7 @@ func tJSDelimited(c context, s []byte) (context, int) {
 var blockCommentEnd = []byte("*/")
 
 // tBlockCmt is the context transition function for /*comment*/ states.
-func tBlockCmt(c context, s []byte) (context, int) {
+func tBlockCmt(n parse.Node, c context, s []byte) (context, int) {
 	i := bytes.Index(s, blockCommentEnd)
 	if i == -1 {
 		return c, len(s)
@@ -364,7 +365,7 @@ func tBlockCmt(c context, s []byte) (context, int) {
 }
 
 // tLineCmt is the context transition function for //comment states.
-func tLineCmt(c context, s []byte) (context, int) {
+func tLineCmt(n parse.Node, c context, s []byte) (context, int) {
 	var lineTerminators string
 	var endState state
 	switch c.state {
@@ -397,7 +398,7 @@ func tLineCmt(c context, s []byte) (context, int) {
 }
 
 // tCSS is the context transition function for the CSS state.
-func tCSS(c context, s []byte) (context, int) {
+func tCSS(n parse.Node, c context, s []byte) (context, int) {
 	// CSS quoted strings are almost never used except for:
 	// (1) URLs as in background: "/foo.png"
 	// (2) Multiword font-names as in font-family: "Times New Roman"
@@ -470,7 +471,7 @@ func tCSS(c context, s []byte) (context, int) {
 }
 
 // tCSSStr is the context transition function for the CSS string and URL states.
-func tCSSStr(c context, s []byte) (context, int) {
+func tCSSStr(n parse.Node, c context, s []byte) (context, int) {
 	var endAndEsc string
 	switch c.state {
 	case stateCSSDqStr, stateCSSDqURL:
@@ -489,7 +490,7 @@ func tCSSStr(c context, s []byte) (context, int) {
 	for {
 		i := k + bytes.IndexAny(s[k:], endAndEsc)
 		if i < k {
-			c, nread := tURL(c, decodeCSS(s[k:]))
+			c, nread := tURL(n, c, decodeCSS(s[k:]))
 			return c, k + nread
 		}
 		if s[i] == '\\' {
@@ -504,13 +505,13 @@ func tCSSStr(c context, s []byte) (context, int) {
 			c.state = stateCSS
 			return c, i + 1
 		}
-		c, _ = tURL(c, decodeCSS(s[:i+1]))
+		c, _ = tURL(n, c, decodeCSS(s[:i+1]))
 		k = i + 1
 	}
 }
 
 // tError is the context transition function for the error state.
-func tError(c context, s []byte) (context, int) {
+func tError(n parse.Node, c context, s []byte) (context, int) {
 	return c, len(s)
 }
 
@@ -518,7 +519,7 @@ func tError(c context, s []byte) (context, int) {
 // It returns an error if s[i:] does not look like it begins with an
 // attribute name, such as encountering a quote mark without a preceding
 // equals sign.
-func eatAttrName(s []byte, i int) (int, *Error) {
+func eatAttrName(n parse.Node, s []byte, i int) (int, *Error) {
 	for j := i; j < len(s); j++ {
 		switch s[j] {
 		case ' ', '\t', '\n', '\f', '\r', '=', '>':
@@ -527,7 +528,7 @@ func eatAttrName(s []byte, i int) (int, *Error) {
 			// These result in a parse warning in HTML5 and are
 			// indicative of serious problems if seen in an attr
 			// name in a template.
-			return -1, errorf(ErrBadHTML, nil, 0, "%q in attribute name: %.32q", s[j:j+1], s)
+			return -1, errorf(ErrBadHTML, n, 0, "%q in attribute name: %.32q", s[j:j+1], s)
 		default:
 			// No-op.
 		}

@rsc rsc modified the milestones: Go1.10, Go1.11 Dec 13, 2017

@namusyaka

This comment has been minimized.

Copy link
Member

commented Jan 16, 2018

I've encountered a similar issue while using html/template.
@rsc's patch looks very helpful, and it breaks only one test but is not regression.

Current patch is here:

diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index 5963194be6..feb2705279 100644
--- a/src/html/template/escape.go
+++ b/src/html/template/escape.go
@@ -656,7 +656,7 @@ var doctypeBytes = []byte("<!DOCTYPE")
 func (e *escaper) escapeText(c context, n *parse.TextNode) context {
 	s, written, i, b := n.Text, 0, 0, new(bytes.Buffer)
 	for i != len(s) {
-		c1, nread := contextAfterText(c, s[i:])
+		c1, nread := contextAfterText(n, c, s[i:])
 		i1 := i + nread
 		if c.state == stateText || c.state == stateRCDATA {
 			end := i1
@@ -722,16 +722,16 @@ func (e *escaper) escapeText(c context, n *parse.TextNode) context {
 
 // contextAfterText starts in context c, consumes some tokens from the front of
 // s, then returns the context after those tokens and the unprocessed suffix.
-func contextAfterText(c context, s []byte) (context, int) {
+func contextAfterText(n parse.Node, c context, s []byte) (context, int) {
 	if c.delim == delimNone {
-		c1, i := tSpecialTagEnd(c, s)
+		c1, i := tSpecialTagEnd(n, c, s)
 		if i == 0 {
 			// A special end tag (`</script>`) has been seen and
 			// all content preceding it has been consumed.
 			return c1, 0
 		}
 		// Consider all content up to any end tag.
-		return transitionFunc[c.state](c, s[:i])
+		return transitionFunc[c.state](n, c, s[:i])
 	}
 
 	// We are at the beginning of an attribute value.
@@ -761,7 +761,7 @@ func contextAfterText(c context, s []byte) (context, int) {
 		//     <button onclick="alert(&quot;Hi!&quot;)">
 		// without having to entity decode token boundaries.
 		for u := []byte(html.UnescapeString(string(s))); len(u) != 0; {
-			c1, i1 := transitionFunc[c.state](c, u)
+			c1, i1 := transitionFunc[c.state](n, c, u)
 			c, u = c1, u[i1:]
 		}
 		return c, len(s)
diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index 55f808ccba..431f8d8cee 100644
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -927,7 +927,7 @@ func TestErrors(t *testing.T) {
 		},
 		{
 			"{{range .Items}}<a{{end}}",
-			`z:1: on range loop re-entry: "<" in attribute name: "<a"`,
+			`z:1:16: on range loop re-entry: "<" in attribute name: "<a"`,
 		},
 		{
 			"\n{{range .Items}} x='<a{{end}}",
diff --git a/src/html/template/html.go b/src/html/template/html.go
index de4aa4abb2..b1eae7c24b 100644
--- a/src/html/template/html.go
+++ b/src/html/template/html.go
@@ -179,7 +179,7 @@ func stripTags(html string) string {
 			if c.element != elementNone && !isInTag(st) {
 				st = stateRCDATA
 			}
-			d, nread := transitionFunc[st](c, s[i:])
+			d, nread := transitionFunc[st](nil, c, s[i:])
 			i1 := i + nread
 			if c.state == stateText || c.state == stateRCDATA {
 				// Emit text up to the start of the tag or comment.
diff --git a/src/html/template/transition.go b/src/html/template/transition.go
index c72cf1ea60..d050936a83 100644
--- a/src/html/template/transition.go
+++ b/src/html/template/transition.go
@@ -7,13 +7,14 @@ package template
 import (
 	"bytes"
 	"strings"
+	"text/template/parse"
 )
 
 // transitionFunc is the array of context transition functions for text nodes.
 // A transition function takes a context and template text input, and returns
 // the updated context and the number of bytes consumed from the front of the
 // input.
-var transitionFunc = [...]func(context, []byte) (context, int){
+var transitionFunc = [...]func(parse.Node, context, []byte) (context, int){
 	stateText:        tText,
 	stateTag:         tTag,
 	stateAttrName:    tAttrName,
@@ -45,7 +46,7 @@ var commentStart = []byte("<!--")
 var commentEnd = []byte("-->")
 
 // tText is the context transition function for the text state.
-func tText(c context, s []byte) (context, int) {
+func tText(n parse.Node, c context, s []byte) (context, int) {
 	k := 0
 	for {
 		i := k + bytes.IndexByte(s[k:], '<')
@@ -83,7 +84,7 @@ var elementContentType = [...]state{
 }
 
 // tTag is the context transition function for the tag state.
-func tTag(c context, s []byte) (context, int) {
+func tTag(n parse.Node, c context, s []byte) (context, int) {
 	// Find the attribute name.
 	i := eatWhiteSpace(s, 0)
 	if i == len(s) {
@@ -95,7 +96,7 @@ func tTag(c context, s []byte) (context, int) {
 			element: c.element,
 		}, i + 1
 	}
-	j, err := eatAttrName(s, i)
+	j, err := eatAttrName(n, s, i)
 	if err != nil {
 		return context{state: stateError, err: err}, len(s)
 	}
@@ -132,8 +133,8 @@ func tTag(c context, s []byte) (context, int) {
 }
 
 // tAttrName is the context transition function for stateAttrName.
-func tAttrName(c context, s []byte) (context, int) {
-	i, err := eatAttrName(s, 0)
+func tAttrName(n parse.Node, c context, s []byte) (context, int) {
+	i, err := eatAttrName(n, s, 0)
 	if err != nil {
 		return context{state: stateError, err: err}, len(s)
 	} else if i != len(s) {
@@ -143,7 +144,7 @@ func tAttrName(c context, s []byte) (context, int) {
 }
 
 // tAfterName is the context transition function for stateAfterName.
-func tAfterName(c context, s []byte) (context, int) {
+func tAfterName(n parse.Node, c context, s []byte) (context, int) {
 	// Look for the start of the value.
 	i := eatWhiteSpace(s, 0)
 	if i == len(s) {
@@ -168,7 +169,7 @@ var attrStartStates = [...]state{
 }
 
 // tBeforeValue is the context transition function for stateBeforeValue.
-func tBeforeValue(c context, s []byte) (context, int) {
+func tBeforeValue(n parse.Node, c context, s []byte) (context, int) {
 	i := eatWhiteSpace(s, 0)
 	if i == len(s) {
 		return c, len(s)
@@ -186,7 +187,7 @@ func tBeforeValue(c context, s []byte) (context, int) {
 }
 
 // tHTMLCmt is the context transition function for stateHTMLCmt.
-func tHTMLCmt(c context, s []byte) (context, int) {
+func tHTMLCmt(n parse.Node, c context, s []byte) (context, int) {
 	if i := bytes.Index(s, commentEnd); i != -1 {
 		return context{}, i + 3
 	}
@@ -209,7 +210,7 @@ var (
 
 // tSpecialTagEnd is the context transition function for raw text and RCDATA
 // element states.
-func tSpecialTagEnd(c context, s []byte) (context, int) {
+func tSpecialTagEnd(n parse.Node, c context, s []byte) (context, int) {
 	if c.element != elementNone {
 		if i := indexTagEnd(s, specialTagEndMarkers[c.element]); i != -1 {
 			return context{}, i
@@ -244,12 +245,12 @@ func indexTagEnd(s []byte, tag []byte) int {
 }
 
 // tAttr is the context transition function for the attribute state.
-func tAttr(c context, s []byte) (context, int) {
+func tAttr(n parse.Node, c context, s []byte) (context, int) {
 	return c, len(s)
 }
 
 // tURL is the context transition function for the URL state.
-func tURL(c context, s []byte) (context, int) {
+func tURL(n parse.Node, c context, s []byte) (context, int) {
 	if bytes.ContainsAny(s, "#?") {
 		c.urlPart = urlPartQueryOrFrag
 	} else if len(s) != eatWhiteSpace(s, 0) && c.urlPart == urlPartNone {
@@ -261,7 +262,7 @@ func tURL(c context, s []byte) (context, int) {
 }
 
 // tJS is the context transition function for the JS state.
-func tJS(c context, s []byte) (context, int) {
+func tJS(n parse.Node, c context, s []byte) (context, int) {
 	i := bytes.IndexAny(s, `"'/`)
 	if i == -1 {
 		// Entire input is non string, comment, regexp tokens.
@@ -298,7 +299,7 @@ func tJS(c context, s []byte) (context, int) {
 
 // tJSDelimited is the context transition function for the JS string and regexp
 // states.
-func tJSDelimited(c context, s []byte) (context, int) {
+func tJSDelimited(n parse.Node, c context, s []byte) (context, int) {
 	specials := `\"`
 	switch c.state {
 	case stateJSSqStr:
@@ -351,7 +352,7 @@ func tJSDelimited(c context, s []byte) (context, int) {
 var blockCommentEnd = []byte("*/")
 
 // tBlockCmt is the context transition function for /*comment*/ states.
-func tBlockCmt(c context, s []byte) (context, int) {
+func tBlockCmt(n parse.Node, c context, s []byte) (context, int) {
 	i := bytes.Index(s, blockCommentEnd)
 	if i == -1 {
 		return c, len(s)
@@ -368,7 +369,7 @@ func tBlockCmt(c context, s []byte) (context, int) {
 }
 
 // tLineCmt is the context transition function for //comment states.
-func tLineCmt(c context, s []byte) (context, int) {
+func tLineCmt(n parse.Node, c context, s []byte) (context, int) {
 	var lineTerminators string
 	var endState state
 	switch c.state {
@@ -401,7 +402,7 @@ func tLineCmt(c context, s []byte) (context, int) {
 }
 
 // tCSS is the context transition function for the CSS state.
-func tCSS(c context, s []byte) (context, int) {
+func tCSS(n parse.Node, c context, s []byte) (context, int) {
 	// CSS quoted strings are almost never used except for:
 	// (1) URLs as in background: "/foo.png"
 	// (2) Multiword font-names as in font-family: "Times New Roman"
@@ -474,7 +475,7 @@ func tCSS(c context, s []byte) (context, int) {
 }
 
 // tCSSStr is the context transition function for the CSS string and URL states.
-func tCSSStr(c context, s []byte) (context, int) {
+func tCSSStr(n parse.Node, c context, s []byte) (context, int) {
 	var endAndEsc string
 	switch c.state {
 	case stateCSSDqStr, stateCSSDqURL:
@@ -493,7 +494,7 @@ func tCSSStr(c context, s []byte) (context, int) {
 	for {
 		i := k + bytes.IndexAny(s[k:], endAndEsc)
 		if i < k {
-			c, nread := tURL(c, decodeCSS(s[k:]))
+			c, nread := tURL(n, c, decodeCSS(s[k:]))
 			return c, k + nread
 		}
 		if s[i] == '\\' {
@@ -508,13 +509,13 @@ func tCSSStr(c context, s []byte) (context, int) {
 			c.state = stateCSS
 			return c, i + 1
 		}
-		c, _ = tURL(c, decodeCSS(s[:i+1]))
+		c, _ = tURL(n, c, decodeCSS(s[:i+1]))
 		k = i + 1
 	}
 }
 
 // tError is the context transition function for the error state.
-func tError(c context, s []byte) (context, int) {
+func tError(n parse.Node, c context, s []byte) (context, int) {
 	return c, len(s)
 }
 
@@ -522,7 +523,7 @@ func tError(c context, s []byte) (context, int) {
 // It returns an error if s[i:] does not look like it begins with an
 // attribute name, such as encountering a quote mark without a preceding
 // equals sign.
-func eatAttrName(s []byte, i int) (int, *Error) {
+func eatAttrName(n parse.Node, s []byte, i int) (int, *Error) {
 	for j := i; j < len(s); j++ {
 		switch s[j] {
 		case ' ', '\t', '\n', '\f', '\r', '=', '>':
@@ -531,7 +532,7 @@ func eatAttrName(s []byte, i int) (int, *Error) {
 			// These result in a parse warning in HTML5 and are
 			// indicative of serious problems if seen in an attr
 			// name in a template.
-			return -1, errorf(ErrBadHTML, nil, 0, "%q in attribute name: %.32q", s[j:j+1], s)
+			return -1, errorf(ErrBadHTML, n, 0, "%q in attribute name: %.32q", s[j:j+1], s)
 		default:
 			// No-op.
 		}
@gopherbot

This comment has been minimized.

Copy link

commented Feb 24, 2018

Change https://golang.org/cl/96955 mentions this issue: html/template: improve ErrBadHTML error report

@gopherbot gopherbot modified the milestones: Go1.11, Unplanned May 23, 2018

@dchapes

This comment has been minimized.

Copy link

commented May 6, 2019

Only semi-related but I noticed the above patch includes:

[]byte(html.UnescapeString(string(s)))

This is a lot of extra []byte <-> string copying since html.UnescapeString does it's own b := []byte(s) conversion (if it has potential work to do) and finishes with a string(b). The real work is all done in html.unescapeEntity which works with byte slices.

Is there any chance for a new html.UnescapeBytes([]byte) []byte that avoids the copying?
(Or probably better, something like func html.UnescapeAppend(dst, src []byte) []byte for the caller to provide the destination buffer).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.