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

Jump to bottom

html/template: template.HTML being escaped (rather than included verbatim) sometimes #20842

Closed
dmitshur opened this issue Jun 29, 2017 · 4 comments
Closed

html/template: template.HTML being escaped (rather than included verbatim) sometimes #20842

dmitshur opened this issue Jun 29, 2017 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.9

Comments

@dmitshur
Copy link
Contributor

I've spotted a regression in html/template behavior in Go 1.9 Beta 2.

I investigated and was able to reduce it the following relatively minimal test case.

https://play.golang.org/p/BrsSxT5CQK

Output with go1.8.3:

Hello, <strong>gopher</strong>.

---

<html>
	<body>
		Hello, <strong>gopher</strong>.
	</body>
</html>

Output with go1.9beta2:

Hello, <strong>gopher</strong>.

---

<html>
	<body>
		Hello, &lt;strong&gt;gopher&lt;/strong&gt;.
	</body>
</html>

I suspected this is caused by CL 37880, and I've confirmed that hunch. 9ffd933 is the first bad commit; its parent does not have the regression. /cc @stjj89 @rsc @mikesamuel @cespare

My understanding is that this is an unintended bug, because the commit message says:

html/template: panic if predefined escapers are found in pipelines during rewriting

Report an error if ...

But no panics/errors are reported. Only the output is different.

(Adding milestone Go1.9, please let me know if that's not correct.)
@dmitshur dmitshur added this to the Go1.9 milestone Jun 29, 2017
@bradfitz bradfitz added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Jun 29, 2017
@mikesamuel
Copy link
Contributor

@stjj89

@stjj89
Copy link
Contributor

stjj89 commented Jun 29, 2017

Thanks for reporting this. The problem is that {{renderHTML}} gets rewritten to {{renderHTML | _html_template_htmlescaper}} after template "hello" is autoescaped, but then gets further written rewritten to {{renderHTML | _html_template_htmlescaper | _html_template_htmlescaper}} after the main template is autoescaped, which leads to the overescaping.

9ffd933 removed some logic that would prevent this duplicate escaper from being inserted. I'm working on a fix right now.

@gopherbot
Copy link

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

@stjj89
Copy link
Contributor

stjj89 commented Jun 30, 2017

Note that the reported issue only occurs because template "hello" is executed (and thus escaped) on its own, before being executed as a nested template in the main template. All is well if the main template is directly executed. I've added logic in https://golang.org/cl/47256 to account for these edge cases.

@moorereason moorereason mentioned this issue Sep 12, 2017
Closed
@golang golang locked and limited conversation to collaborators Jul 14, 2018
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
5 participants
@bradfitz @mikesamuel @dmitshur @stjj89 @gopherbot