html/template: Go 1.8 has a different encoding logic. Intentionally?

[leonklingele]

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

Go 1.8 beta (130ad87)

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

Ubuntu, OS X

What did you do?

• Test this script with Go 1.8

What did you expect to see?

• Works fine with this version of Go

What did you see instead?

• Strings do not match: got '<script type="application/json">{&#34;name&#34;:&#34;Peter&#34;}</script>', want '<script type="application/json">{"name":"Peter"}</script>'

If that's how it should look like with Go 1.8, how can I get back the old behaviour?

[bradfitz]

I don't see any obvious commit during Go 1.8 which would've caused this, unless the change wasn't in the html/template package.

@rsc, do you know off hand?

commit 1e917312511f6ce54bbdcea8cd0c25e66973d49e
Author: Caleb Spare <cespare@gmail.com>
Date:   Sun Nov 13 18:06:16 2016 -0800

    html/template: fix multiple Clones of redefined template
    
    This change redoes the fix for #16101 (CL 31092) in a different way by
    making t.Clone return the template associated with the t.Name() while
    allowing for the case that a template of the same name is define-d.
    
    Fixes #17735.
    
    Change-Id: I1e69672390a4c81aa611046a209008ae4a3bb723
    Reviewed-on: https://go-review.googlesource.com/33210
    Run-TryBot: Caleb Spare <cespare@gmail.com>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: Rob Pike <r@golang.org>

commit 2442b49c47aa818bbc55e4c064e9ea0ca058735f
Author: Marcel Edmund Franke <marcel.edmund.franke@gmail.com>
Date:   Mon Nov 14 21:46:25 2016 +0100

    html/template: typo fix
    
    comment on unexported function starts with wrong functionname
    
    Change-Id: Ib16c2fe42b5a8d4606ed719f620923c17839d091
    Reviewed-on: https://go-review.googlesource.com/33203
    Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

commit d8264de8683dac99ffbbbc1f46415e627b73c9ed
Author: Dmitri Shuralyov <shurcooL@gmail.com>
Date:   Wed Nov 9 14:49:12 2016 -0800

    all: spell "marshal" and "unmarshal" consistently
    
    The tree is inconsistent about single l vs double l in those
    words in documentation, test messages, and one error value text.
    
            $ git grep -E '[Mm]arshall(|s|er|ers|ed|ing)' | wc -l
                  42
            $ git grep -E '[Mm]arshal(|s|er|ers|ed|ing)' | wc -l
                1694
    
    Make it consistently a single l, per earlier decisions. This means
    contributors won't be confused by misleading precedence, and it helps
    consistency.
    
    Change the spelling in one error value text in newRawAttributes of
    crypto/x509 package to be consistent.
    
    This change was generated with:
    
            perl -i -npe 's,([Mm]arshal)l(|s|er|ers|ed|ing),$1$2,' $(git grep -l -E '[Mm]arshall' | grep -v AUTHORS | grep -v CONTRIBUTORS)
    
    Updates #12431.
    Follows https://golang.org/cl/14150.
    
    Change-Id: I85d28a2d7692862ccb02d6a09f5d18538b6049a2
    Reviewed-on: https://go-review.googlesource.com/33017
    Run-TryBot: Minux Ma <minux@golang.org>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

commit ef3a9f2dd410ec1672988a72c72919eab2d58e6c
Author: Russ Cox <rsc@golang.org>
Date:   Wed Oct 26 13:12:17 2016 -0400

    html/template, text/template: drop defined template list from errors
    
    The report in #17414 points out that if you have many many templates,
    then this is an overwhelming list and just hurts the signal-to-noise ratio of the error.
    
    Even the test of the old behavior also supports the idea that this is noise:
    
            template: empty: "empty" is an incomplete or empty template; defined templates are: "secondary"
    
    The chance that someone mistyped "secondary" as "empty" is slim at best.
    
    Similarly, the compiler does not augment an error like 'unknown variable x'
    by dumping the full list of all the known variables.
    
    For all these reasons, drop the list.
    
    Fixes #17414.
    
    Change-Id: I78f92d2c591df7218385fe723a4abc497913acf8
    Reviewed-on: https://go-review.googlesource.com/32116
    Run-TryBot: Russ Cox <rsc@golang.org>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: Rob Pike <r@golang.org>

commit 2693fa15ee12acd67e45d8fa57626675903ab605
Author: Russ Cox <rsc@golang.org>
Date:   Wed Oct 19 13:34:15 2016 -0400

    html/template: add test case for unbounded template expansion
    
    Fixed by CL 31092 already, but that change is a few steps away
    from the problem observed here, so add an explicit test.
    
    Fixes #17019.
    
    Change-Id: If4ece1418e6596b1976961347889ce12c5969637
    Reviewed-on: https://go-review.googlesource.com/31466
    Run-TryBot: Russ Cox <rsc@golang.org>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: Quentin Smith <quentin@golang.org>

commit 604146ce8961d32f410949015fc8ee31f9052209
Author: Russ Cox <rsc@golang.org>
Date:   Wed Oct 19 10:27:05 2016 -0400

    html/template, text/template: docs and fixes for template redefinition
    
    All prior versions of Go have allowed redefining empty templates
    to become non-empty. Unfortunately, that has never consistently
    taken effect in html/template after the first execution:
    
            // define and execute
            t := template.New("root")
            t.Parse(`{{define "T"}}{{end}}<a href="{{template "T"}}">`)
            t.Execute(w, nil) // <a href="">
    
            // redefine
            t.Parse(`{{define "T"}}my.url{{end}}`) // succeeds, but ignored
            t.Execute(w, nil) // <a href="">
    
    When Go 1.6 added {{block...}} to text/template, that loosened the
    redefinition rules to allow redefinition at any time. The loosening was
    undone a bit in html/template, although inconsistently:
    
            // define and execute
            t := template.New("root")
            t.Parse(`{{define "T"}}body{{end}}`)
            t.Lookup("T").Execute(ioutil.Discard, nil)
    
            // attempt to redefine
            t.Parse(`{{define "T"}}body{{end}}`) // rejected in all Go versions
            t.Lookup("T").Parse("body") // OK as of Go 1.6, likely unintentionally
    
    Like in the empty->non-empty case, whether future execution takes
    notice of a redefinition basically can't be explained without going into
    the details of the template escape analysis.
    
    Address both the original inconsistencies in whether a redefinition
    would have any effect and the new inconsistencies about whether a
    redefinition is allowed by adopting a new rule: no parsing or modifying
    any templates after the first execution of any template in the same set.
    Template analysis begins at first execution, and once template analysis
    has begun, we simply don't have the right logic to update the analysis
    for incremental modifications (and never have).
    
    If this new rule breaks existing uses of templates that we decide need
    to be supported, we can try to invalidate all escape analysis for the
    entire set after any modifications. But let's wait on that until we know
    we need to and why.
    
    Also fix documentation of text/template redefinition policy
    (redefinition is always OK).
    
    Fixes #15761.
    
    Change-Id: I7d58d7c08a7d9df2440ee0d651a5b2ecaff3006c
    Reviewed-on: https://go-review.googlesource.com/31464
    Run-TryBot: Russ Cox <rsc@golang.org>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: Andrew Gerrand <adg@golang.org>

commit d2315fdc11ebdf5c0ae94f33cb01ffaab82c00b6
Author: Russ Cox <rsc@golang.org>
Date:   Wed Oct 19 13:14:16 2016 -0400

    html/template: adjust ambiguous URL context text
    
    Before: ... appears in an ambiguous URL context.
    After:  ... appears in an ambiguous context within a URL.
    
    It's a minor point, but it's confused multiple people.
    Try to make clearer that the ambiguity is "where exactly inside the URL?"
    
    Fixes #17319.
    
    Change-Id: Id834868d1275578036c1b00c2bdfcd733d9d2b7b
    Reviewed-on: https://go-review.googlesource.com/31465
    Run-TryBot: Russ Cox <rsc@golang.org>
    Reviewed-by: Rob Pike <r@golang.org>
    TryBot-Result: Gobot Gobot <gobot@golang.org>

commit 2f7f679c79009137bd34fcc33a6d3a6762f45e75
Author: Russ Cox <rsc@golang.org>
Date:   Tue Oct 18 23:22:38 2016 -0400

    html/template, text/template: clarify template redefinition behavior
    
    Make two important points clearer:
    
     - Giving a template definition containing
       nothing but spaces has no effect.
     - Giving a template definition containing
       non-spaces can only be done once per template.
    
    Fixes #16912.
    Fixes #16913.
    Fixes #17360.
    
    Change-Id: Ie3971b83ab148b7c8bb800fe4a21579566378e3e
    Reviewed-on: https://go-review.googlesource.com/31459
    Run-TryBot: Russ Cox <rsc@golang.org>
    Reviewed-by: Rob Pike <r@golang.org>
    Reviewed-by: Andrew Gerrand <adg@golang.org>

commit cd2c9df7612795cad5b56cabe5ec29c7771db5fe
Author: Caleb Spare <cespare@gmail.com>
Date:   Fri Oct 14 00:59:19 2016 -0700

    html/template: fix Clone so that t.Lookup(t.Name()) yields t
    
    Template.escape makes the assumption that t.Lookup(t.Name()) is t
    (escapeTemplate looks up the associated template by name and sets
    escapeErr appropriately).
    
    This assumption did not hold for a Cloned template, because the template
    associated with t.Name() was a second copy of the original.
    
    Add a test for the assumption that t.Lookup(t.Name()) == t.
    
    One effect of this broken assumption was #16101: parallel Executes
    racily accessed the template namespace because each Execute call saw
    t.escapeErr == nil and re-escaped the template concurrently with read
    accesses occurring outside the namespace mutex.
    
    Add a test for this race.
    
    Related to #12996 and CL 16104.
    
    Fixes #16101
    
    Change-Id: I59831d0847abbabb4ef9135f2912c6ce982f9837
    Reviewed-on: https://go-review.googlesource.com/31092
    Run-TryBot: Rob Pike <r@golang.org>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: Rob Pike <r@golang.org>

commit ffd1c781b77aab542713b66ef387fa9307e4060b
Author: Nodir Turakulov <nodir@google.com>
Date:   Sat Sep 5 06:38:13 2015 -0700

    html/template: check "type" attribute in <script>
    
    Currently any script tag is treated as a javascript container, although
    <script type="text/template"> must not be. Check "type" attribute of
    "script" tag. If it is present and it is not a JS MIME type, do not
    transition to elementScript state.
    
    Fixes #12149, where // inside text template was treated as regexp.
    Fixes #6701
    
    Change-Id: I8fc9e504f7280bdd800f40383c061853665ac8a2
    Reviewed-on: https://go-review.googlesource.com/14336
    Run-TryBot: Russ Cox <rsc@golang.org>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: Russ Cox <rsc@golang.org>

[odeke-em]

The change that caused it was made commit ffd1c78, CL https://go-review.googlesource.com/c/14336/ which was merged on 29th September 2016,
but was written in September 2015 so probably that's why @bradfitz hadn't seen it as an obvious
commit to html/template.

The problem stems from this helper function isJSType used in escape.go which checks each script's type tag for mimeTypes that
should be included as JS scripts.
In the failing example, the script type is "application/json"
but unfortunately that mimeType is not included in the cases for valid JS content

go/src/html/template/js.go

Lines 381 to 398 in ffd1c78

	case
	"application/ecmascript",
	"application/javascript",
	"application/x-ecmascript",
	"application/x-javascript",
	"text/ecmascript",
	"text/javascript",
	"text/javascript1.0",
	"text/javascript1.1",
	"text/javascript1.2",
	"text/javascript1.3",
	"text/javascript1.4",
	"text/javascript1.5",
	"text/jscript",
	"text/livescript",
	"text/x-ecmascript",
	"text/x-javascript":
	return true

/cc @nodirt

[leonklingele]

@odeke-em thanks for investigating. Adding application/json to js.go fixes the issue.
Now I'm wondering why JSON is not considered to be a JS type.

[gopherbot]

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

[gopherbot]

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

[anthonybishopric]

I'm fairly certain this change is related to a breakage I'm seeing with the Javascript templating framework EJS.

In 1.7 I'm able to embed the following directly into my document and have it come out unadulterated:

  <script id="new-key" type="text/template">
      <tr class="api-key">
        <td class="api-key-cell"><%= key %></td>
      </tr>
   </script>

As of 1.8 the EJS template gets mangled due to escaping:

  <script id="new-key" type="text/template">
      <tr class="api-key">
        <td class="api-key-cell">&lt;%= key %></td>
      </tr>
   </script>

If this is desired behavior, is there a workaround?

[bradfitz]

@anthonybishopric, you're commenting on a closed bug. If you want somebody to see it and consider it for Go 1.8, please file a new bug.

[anthonybishopric]

@bradfitz Thanks, sorry I didn't see the closed indicator. Will open a new issue