diff options
author | Mike Samuel <mikesamuel@gmail.com> | 2011-09-21 21:38:40 -0700 |
---|---|---|
committer | Mike Samuel <mikesamuel@gmail.com> | 2011-09-21 21:38:40 -0700 |
commit | 35819729b83b8521e9ae1bddca5c368672777de5 (patch) | |
tree | be0cbb6afd0bfb9d50f2bef9bd8b71712b175389 | |
parent | 7eab0c2bdc6b3d8694b36254c6ab3e65f8258d26 (diff) | |
download | go-35819729b83b8521e9ae1bddca5c368672777de5.tar.gz go-35819729b83b8521e9ae1bddca5c368672777de5.zip |
exp/template/html: elide comments in template source.
When templates are stored in external files, developers often embed
comments to explain&|disable code.
<!-- Oblique reference to project code name here -->
{{if .C}}...{{else}}<!-- commented out default -->{{end}}
This unnecessarily increases the size of shipped HTML and can leak
information.
This change elides all comments of the following types:
1. <!-- ... --> comments found in source.
2. /*...*/ and // comments found in <script> elements.
3. /*...*/ and // comments found in <style> elements.
It does not elide /*...*/ or // comments found in HTML attributes:
4. <button onclick="/*...*/">
5. <div style="/*...*/">
I can find no examples of comments in attributes in Closure Templates
code and doing so would require keeping track of character positions
post decode in
<button onclick="/*...*/">
To prevent token joining, /*comments*/ are JS and CSS comments are
replaced with a whitespace char.
HTML comments are not, but to prevent token joining we could try to
detect cases like
<<!---->b>
</<!---->b>
which has a well defined meaning in HTML but will cause a validator
to barf. This is difficult, and this is a very minor case.
I have punted for now, but if we need to address this case, the best
way would be to normalize '<' in stateText to '<' consistently.
The whitespace to replace a JS /*comment*/ with depends on whether
there is an embedded line terminator since
break/*
*/foo
...
is equivalent to
break;
foo
...
while
break/**/foo
...
is equivalent to
break foo;
...
Comment eliding can interfere with IE conditional comments.
http://en.wikipedia.org/wiki/Conditional_comment
<!--[if IE 6]>
<p>You are using Internet Explorer 6.</p>
<![endif]-->
/*@cc_on
document.write("You are using IE4 or higher");
@*/
I have not encountered these in production template code, and
the typed content change in CL 4962067 provides an escape-hatch
if conditional comments are needed.
R=nigeltao
CC=golang-dev
https://golang.org/cl/4999042
-rw-r--r-- | src/pkg/exp/template/html/escape.go | 33 | ||||
-rw-r--r-- | src/pkg/exp/template/html/escape_test.go | 29 |
2 files changed, 46 insertions, 16 deletions
diff --git a/src/pkg/exp/template/html/escape.go b/src/pkg/exp/template/html/escape.go index 28019f2525..650a6acd28 100644 --- a/src/pkg/exp/template/html/escape.go +++ b/src/pkg/exp/template/html/escape.go @@ -568,12 +568,43 @@ func (e *escaper) escapeText(c context, n *parse.TextNode) context { written = j + 1 } } + } else if isComment(c.state) && c.delim == delimNone { + switch c.state { + case stateJSBlockCmt: + // http://es5.github.com/#x7.4: + // "Comments behave like white space and are + // discarded except that, if a MultiLineComment + // contains a line terminator character, then + // the entire comment is considered to be a + // LineTerminator for purposes of parsing by + // the syntactic grammar." + if bytes.IndexAny(s[written:i1], "\n\r\u2028\u2029") != -1 { + b.WriteByte('\n') + } else { + b.WriteByte(' ') + } + case stateCSSBlockCmt: + b.WriteByte(' ') + } + written = i1 + } + if c.state != c1.state && isComment(c1.state) && c1.delim == delimNone { + // Preserve the portion between written and the comment start. + cs := i1 - 2 + if c1.state == stateHTMLCmt { + // "<!--" instead of "/*" or "//" + cs -= 2 + } + b.Write(s[written:cs]) + written = i1 } c, i = c1, i1 } if written != 0 && c.state != stateError { - b.Write(n.Text[written:]) + if !isComment(c.state) || c.delim != delimNone { + b.Write(n.Text[written:]) + } e.editTextNode(n, b.Bytes()) } return c diff --git a/src/pkg/exp/template/html/escape_test.go b/src/pkg/exp/template/html/escape_test.go index 84bf6b7a4a..8a64515dec 100644 --- a/src/pkg/exp/template/html/escape_test.go +++ b/src/pkg/exp/template/html/escape_test.go @@ -363,13 +363,12 @@ func TestEscape(t *testing.T) { { "HTML comment", "<b>Hello, <!-- name of world -->{{.C}}</b>", - // TODO: Elide comment. - "<b>Hello, <!-- name of world --><Cincinatti></b>", + "<b>Hello, <Cincinatti></b>", }, { "HTML comment not first < in text node.", "<<!-- -->!--", - "<<!-- -->!--", + "<!--", }, { "HTML normalization 1", @@ -384,18 +383,18 @@ func TestEscape(t *testing.T) { { "HTML normalization 3", "a<<!-- --><!-- -->b", - "a<<!-- --><!-- -->b", + "a<b", }, { "Split HTML comment", "<b>Hello, <!-- name of {{if .T}}city -->{{.C}}{{else}}world -->{{.W}}{{end}}</b>", - "<b>Hello, <!-- name of city --><Cincinatti></b>", + "<b>Hello, <Cincinatti></b>", }, { "JS line comment", "<script>for (;;) { if (c()) break// foo not a label\n" + "foo({{.T}});}</script>", - "<script>for (;;) { if (c()) break// foo not a label\n" + + "<script>for (;;) { if (c()) break\n" + "foo( true );}</script>", }, { @@ -405,8 +404,8 @@ func TestEscape(t *testing.T) { // Newline separates break from call. If newline // removed, then break will consume label leaving // code invalid. - "<script>for (;;) { if (c()) break/* foo not a label\n" + - " */foo( true );}</script>", + "<script>for (;;) { if (c()) break\n" + + "foo( true );}</script>", }, { "JS single-line block comment", @@ -417,25 +416,25 @@ func TestEscape(t *testing.T) { // removed, then break will consume label leaving // code invalid. "<script>for (;;) {\n" + - "if (c()) break/* foo a label */foo;" + + "if (c()) break foo;" + "x( true );}</script>", }, { "JS block comment flush with mathematical division", "<script>var a/*b*//c\nd</script>", - "<script>var a/*b*//c\nd</script>", + "<script>var a /c\nd</script>", }, { "JS mixed comments", "<script>var a/*b*///c\nd</script>", - "<script>var a/*b*///c\nd</script>", + "<script>var a \nd</script>", }, { "CSS comments", "<style>p// paragraph\n" + `{border: 1px/* color */{{"#00f"}}}</style>`, - "<style>p// paragraph\n" + - "{border: 1px/* color */#00f}</style>", + "<style>p\n" + + "{border: 1px #00f}</style>", }, { "JS attr block comment", @@ -462,12 +461,12 @@ func TestEscape(t *testing.T) { { "HTML substitution commented out", "<p><!-- {{.H}} --></p>", - "<p><!-- --></p>", + "<p></p>", }, { "Comment ends flush with start", "<!--{{.}}--><script>/*{{.}}*///{{.}}\n</script><style>/*{{.}}*///{{.}}\n</style><a onclick='/*{{.}}*///{{.}}' style='/*{{.}}*///{{.}}'>", - "<!----><script>/**///\n</script><style>/**///\n</style><a onclick='/**///' style='/**///'>", + "<script> \n</script><style> \n</style><a onclick='/**///' style='/**///'>", }, { "typed HTML in text", |