html/template: merging escapers can lead to over-escaping

[stjj89]

version: go version devel +4cce27a3fa Sat Jan 21 03:20:55 2017 +0000 linux/amd64
env: linux amd64

The template

<a href={{ . }}>

is rewritten into

<a href={{ . | _html_template_urlfilter | _html_template_urlnormalizer | _html_template_nospaceescaper }}>

after contextual auto-escaping. However,

<a href={{ . | html | print 3 | urlquery }}>

is rewritten into

<a href={{ . | _html_template_urlfilter | _html_template_urlnormalizer | html | print 3 | urlquery }}>

Since _html_template_urlnormalizer and urlquery are considered equivalent, this output of this pipeline will be over-escaped.

This happens because of the html/template escaper's attempt to merge inserted escapers with user-supplied escapers, described in #19336. This merge algorithm assumes that the user-supplied escapers in the pipeline will appear in the same order that they do in the sequence of escapers-to-be-inserted (see the merge logic for the gory details).

[stjj89]

@mikesamuel

[mikesamuel]

Ok, let me try to explain this to myself.
Assuming no user error, the intent of the author who wrote pipeline . | html | print 3 | urlquery was probably to send a GET to something that expects to receive URL-encoded HTML and the content the author wants the recipient to unpack is an HTML text node whose content is "3 " followed by the stringy form of ..

So it looks like you're probably right about the URL over-escaping.

If the user were to have written <a href={{ . | html | print 3 }}> would they have gotten the right result according to the interpretation above?

I ask to see whether your proposal to reposition inserted filters at the end addresses the problem if the user also removes explicit escaping for context at the end.

[stjj89]

If the user were to have written <a href={{ . | html | print 3 }}> would they have gotten the right result according to the interpretation above?

<a href={{ . | html | print 3 }}>

would be rewritten into

<a href={{ . | _html_template_urlfilter | _html_template_urlnormalizer | html | print 3 }}>

since _html_template_nospaceescaper is considered equivalent to html, and the escaper preserves the order of inserted escapers (i.e. URL filter, URL normalizer, then HTML escaper). This is not the correct result according to your interpretation: the user will get a HTML-encoded URL, whose content is prefixed by a "3".

This seems to be the contextual auto-escaper working as intended: it sees the "a href" context, determines that the value in this context must be a HTML-encoded, escaped URL, and adds the correct sanitizers to guarantee that. If I understand correctly, if the user wants to add a URL-encoded HTML here, the user should wrap the HTML in the HTML typed string, so that the auto-escaper will only perform the URL filtering/escaping steps on it.

[stjj89]

I ask to see whether your proposal to reposition inserted filters at the end addresses the problem if the user also removes explicit escaping for context at the end.

Just to make sure we're on the same page, my latest recommendation is to ignore user-supplied escapers completely. I think it's best if we first remove them (i.e. user-supplied calls to html and urlquery) from the existing pipeline, then insert internal escaping directives to the end of the pipeline (see the latest comment on #19336). By doing this, we get rid of this complexity and unintuitive edge cases, and ensure that the contextual autoescaper always gets its way.

I should probably write a formal proposal for this, just so I won't have to repost this across all the html/template bug threads.

[gopherbot]

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

[stjj89]

This is no longer an issue now that golang.org/cl/37880 has landed.