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: rewriting html/template files #17933

Closed
jmhodges opened this issue Nov 16, 2016 · 2 comments
Closed

html/template: rewriting html/template files #17933

jmhodges opened this issue Nov 16, 2016 · 2 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@jmhodges
Copy link
Contributor

jmhodges commented Nov 16, 2016

This is with Go 1.7.

It would be nice if it was easier to rewrite html/template files but the current tools (html/template and x/net/html) seem to have a gap between them. Maybe there's a way to do this that I've missed, so let me explain the problems I wanted to solve in detail.

I'm trying to make it easy to support CSP inline hashing in my templates without nagging users and requiring them to remember to use some custom funcs instead of the usual HTML. It'd also be nice to have form-based CSRF protection which would be implemented in a similar way. I'm sure there are other similar rewrites, but these security-focused ones were the first I've run into.

Both CSP inline hashing and form-based CSRF protection involve a developer adding additional attributes or elements to a template. In the CSP inline hashing case, it's an attribute or nonce added script tags. In form-based CSRF protection, it can be a hidden input that sends up a per-request token inside a form tag.

It would be nice to be able to, at parse time, not execution time, pull apart a html/template template, inspect it for the relevant tags, and then change the template for the user instead of returning an error if they forgot the important elements. At parse time, the code has all the data it needs to adjust the template. Similar to why gofmt is capable of rewriting a file, adjusting a template automatically instead of telling them they made an error would a much nicer experience for the developer.

In the form-based CSRF case, the template rewrite would be adding a input tag that calls a template func (or struct value) that has the token data. In the CSP inline hashing case, the rewrite would be detecting if the script tag contained only static information and adding the hash automatically, or returning an error if the developer needs to get involved (usually, to handle the case of requesting javascript files from elsewhere on the web).

However, html/template only has text/template/parse as its AST which doesn't suffice for working at the level of HTML semantics. Folks have offered up x/net/html's ParseFragment as a solution but that doesn't work well.

One problem is that x/net/html's ParseFragment makes it difficult to render exactly back what was in a template. Without providing a context *html.Node, you'll get back a tree with additional bogus nodes added around the outside. This doesn't work well because rewriting code may need to behave differently depending on what context *html.Node is in play instead of guessing that all templates are in body or whatever. But calculating that context *html.Node is difficult since a template can be loaded from other templates. That would involve writing similar template loading code to what is already inside text/template.

A second problem since some of those templates will be called in different escaping contexts, folks using ParseFragment have to re-do the escape context code in html/template within x/net/html's AST.

I haven't found a way to make my templates better without requiring the developer to handle more of what would otherwise be easily automated problems.

I'm not sure, yet, what kind of work it would take to make html/template easier for this purpose. Maybe if html.Node gained line and column information someone could compare it to text/template/parse.Pos and do some grungy work to get the right context Nodes?

Maybe other folks have clearer ideas.

@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 16, 2016
@quentinmit quentinmit added this to the Go1.9Maybe milestone Nov 16, 2016
@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.10 Jul 20, 2017
@jmhodges
Copy link
Contributor Author

Just reupping this now that 1.10 is open. Anyone got an idea on how we can get the escaping info out of html/template and use it when modifying the template as an net/html AST?

@rsc
Copy link
Contributor

rsc commented Dec 4, 2017

We can't support expanding html/template's scope in this way. The escaping API is completely unclear, as are the security implications. The text/template/parse package says:

Package parse builds parse trees for templates as defined by text/template
and html/template. Clients should use those packages to construct templates
rather than this one, which provides shared internal data structures not
intended for general use.

Our suggestion would be to fork html/template and revise the fork as needed.

@rsc rsc closed this as completed Dec 4, 2017
@golang golang locked and limited conversation to collaborators Dec 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

5 participants