html/template: document ability to modify FuncMap after template parse by calling Funcs again

[michaelsafyan]

Background

I have a bunch of templates that have a complex dependency structure like:

 A -> B, D, E
 B ->  E, F
 C -> E, G
 D -> F, H
 E -> H
 F -> nil
 G -> nil
 H -> nil

The contents of the corresponding files ('A.tmpl', 'B.tmpl', etc.) are static; however, the template.FuncMap that they reference contain implicit reference to captured state that varies by caller. That is, the FuncMap is created like so:

func createFuncMap(request *rpc.Request) {
   result := template.FuncMap {
     "Foo": createFooFunc(request),
     "Bar": createBarFunc(request),
      ...
   }
  return request
}

What I Want to Do

I'd like to be able to preparse all of the various files irrespective of the func map (or perhaps with a whitelist of functions that will be defined), and I'd like to parse the individual pieces separately (e.g. parse 'F' only once, even though it is a dependency of 'B' and 'D'), and then somehow combine the parsed templates together like:

   func getTemplate(name string) *template.Template {
     result := template.New(name).Funcs(getStubFunctionMap())
     for _, dep := range getDeps(name) {
        result.AddDep(getTemplate(dep))
      }
      result.Parse(getFileContent(name))
      return result
    }

I'd also like to trivially rebind the function map as in:

   func getBoundTemplate(name string, request *rpc.Request) *template.Template {
     cachedTemplate := getOrCreateCachedTemplate(name)
     boundTemplate := cachedTemplate.Clone().Funcs(funcMapFor(request))
     return boundTemplate
   }

Why I Can't Do It

The documentation of html/template makes it appear that parsing is context dependent and that parsing is linear (dependencies can be added linearly, but cannot be combined from multiple, already-parsed templates). This is a pretty major limitation of the interface.

Impact of This Limitation

This is a major efficiency problem; I want to parse a complex network of templates upfront before receiving requests and do very minimal work on each request. This structure is causing much more work to happen in the context of every request.

For more specifics of this use case, reach out to me directly from your @google.com account, and I can share more details of the specific code in question.

[ianlancetaylor]

CC @robpike

[empijei]

I think I need a little bit more context.

Why do you need to closure some state in the funcs of your templates? Why can't you pass an additional map of data to process?

To use your example: "Foo" func would accept an additional parameter that depends on the request being processed, but its code would stay the same.

What kind of request-scoped data are you currently capturing in your functions that can't be a parameter?

[michaelsafyan]

An example of this is a "FormatTime" function that can format time in a relative way. The input is the time object to format. The time to which it is relative is captured, since we don't want the "now" value to be calculated repeatedly within the template leading to internal inconsistency (and we also want this time to be relative to something meaningful like the underlying event time rather than simply the first time that the function gets invoked in the template). This and other uses certainly could be made to make their input more explicit, but then it harms the readability of the templates and introduces needless cognitive burden to template authors. For syntactic simplicity, it would be ideal if we could make this context implicit while still being able to minimize per-request processing.

On Wed, Oct 16, 2019, 00:25 Roberto Clapis notifications@github.com wrote:

I think I need a little bit more context.

Why do you need to closure some state in the funcs of your templates? Why can't you pass an additional map of data to process?

To use your example: "Foo" func would accept an additional parameter that depends on the request being processed, but its code would stay the same.

What kind of request-scoped data are you currently capturing in your functions that can't be a parameter?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/34680?email_source=notifications&email_token=AABI65N3LLAXUIJMQE6ORF3QO2QS5A5CNFSM4I5HYPN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBLDLQI#issuecomment-542520769, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABI65JSG6XX6WCLLODE4ALQO2QS5ANCNFSM4I5HYPNQ .

[ianlancetaylor]

Something like proposal #31107 would let you pass data into the functions via a context.Context. Would that be another way for you to pass the data you need into the functions?

Can you explain why you want the ability to bind together parsed templates? What would the API in html/template look like?

It sounds like you can do what you need by passing values through the template. You say that making the input more explicit introduces needless cognitive burden to template authors. But Go in general does prefer to make things more explicit.

[ianlancetaylor]

A simple self-contained concrete example might help clarify this. The description is somewhat abstract.

[michaelsafyan]

I've just sent a private email from my @google.com account to Ian's @google.com account with more detail concerning the original motivation.

[rsc]

@michaelsafyan It would be nice if you could say something publicly. We don't make decisions based on unpublished Google-internal motivations. Well, except "no, because it's too Google-specific".

[rsc]

The original comment says:

The documentation of html/template makes it appear that parsing is context dependent and that parsing is linear (dependencies can be added linearly, but cannot be combined from multiple, already-parsed templates). This is a pretty major limitation of the interface.

Yes, parsing is context dependent, so it is not really possible to reuse pieces the way you want. That is fundamental to the security analysis. I don't think what you are trying to do is sound with respect to the security analysis. We won't make unsound changes here.

[michaelsafyan]

Curious, could you elaborate on the security concern?

On Mon, Oct 21, 2019, 17:19 Russ Cox notifications@github.com wrote:

The original comment says:

The documentation of html/template makes it appear that parsing is context dependent and that parsing is linear (dependencies can be added linearly, but cannot be combined from multiple, already-parsed templates). This is a pretty major limitation of the interface.

Yes, parsing is context dependent, so it is not really possible to reuse pieces the way you want. That is fundamental to the security analysis. I don't think what you are trying to do is sound with respect to the security analysis. We won't make unsound changes here.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/34680?email_source=notifications&email_token=AABI65NN3LCF74LIHGUVWGDQPYMEXA5CNFSM4I5HYPN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB32FZA#issuecomment-544711396, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABI65INIIIUACX4K2NHTQTQPYMEXANCNFSM4I5HYPNQ .

[rsc]

The html/template docs give as a simple example <a href="/search?q={{.}}">{{.}}</a>. Those two substitutions need different escaping. In general those could be call-outs to other templates. So fundamentally the analysis of the template is context-dependent.

The original proposal here sounds to me like "don't make template analysis context-dependent". That's not possible (see example).

Is there a more limited proposal you were making, and I've misunderstood?

[michaelsafyan]

The html/template docs give as a simple example <a href="/search?q={{.}}">{{.}}</a>. Those two substitutions need different escaping. In general those could be call-outs to other templates. So fundamentally the analysis of the template is context-dependent.

I agree and am not suggesting that this change.

The original proposal here sounds to me like "don't make template analysis context-dependent".

My apologies for being unclear. No, I'm not asking that parsing cease being context-dependent. I'm asking that parsing not make a distinction regarding whether parameters come from the global scope (via the FuncMap) vs from the local scope. There is currently this inconsistency where FuncMap parameters and parameters that are passed into the template get treated differently insofar as non-FuncMap parameters can be changed while keeping the template the same, while FuncMap parameters cannot be changed after parsing.

What I'd like is for it to be possible to replace the FuncMap (the values, not the keys) on each invocation of the template, without the FuncMap being a shared resource that needs to get locked, so that different requests / instantiations of the template can use different values of the FuncMap parameters. This would make member functions (of whatever is supplied as data interface{}) and global functions (supplied through FuncMap) roughly interchangeable.

[rsc]

I tried to retitle this issue to indicate my current understanding of the suggestion: "add ability to modify FuncMap after template parse".

The original message says:

I'd also like to trivially rebind the function map as in:

  func getBoundTemplate(name string, request *rpc.Request) *template.Template {
    cachedTemplate := getOrCreateCachedTemplate(name)
    boundTemplate := cachedTemplate.Clone().Funcs(funcMapFor(request))
    return boundTemplate
   }

Now I don't understand "why I can't do it". As far as I can see that code should work fine. The "why I can't do it" talks about parsing, but there's no parsing in calling Clone and Funcs.

I'm sorry, but I'm still confused.

[michaelsafyan]

Per the documentation of func (t *Template) Funcs(funcMap FuncMap) *Template: https://golang.org/pkg/html/template/#Template.Funcs

Funcs adds the elements of the argument map to the template's function map. It must be called before the template is parsed. It panics if a value in the map is not a function with appropriate return type. However, it is legal to overwrite elements of the map. The return value is the template, so calls can be chained.

In other words, one cannot call Funcs a second time after calling Clone. One can modify the existing map passed into Funcs before, but one cannot pass in a completely different map (which is important for thread-safety).

I'd like this to be relaxed; one should be able to call Funcs again with a new map, as long as the new map has all of the same keys as the previous one.

(And, yes, I am basing this on more than just documentation, but also trying it; when I tried the code above, the result was an error).

[ianlancetaylor]

Out of curiousity:

Why do you want to call Funcs a second time after calling Clone? If you do, why not just call Clone again?

Or, could you write the functions as redirectors, so that they test whatever condition you would use to call Funcs again to decide themselves which function to call?

[michaelsafyan]

Or, could you write the functions as redirectors, so that they test whatever condition you would use to call Funcs again to decide themselves which function to call?

Doing this would require the introduction of locking, which is what I'm trying to avoid.

To clarify:

Startup / shared thread:

• (Re)load template text files

• (Re)parse text files into templates (using dummy FuncMap)

  (In parallel): request1 thread A:

• Clone (shared) parsed template, creating thread-local copy

• Set copy to use request1's function map

  (In parallel): request2 thread B:

• Clone (shared) parsed template, creating thread-local copy
• Set copy to use request2's function map

[Also thread C, D, etc. up to N request-serving threads]

The same semantics can be achieved with a function that is a "redirector", but then this requires the function to reference an actual thread local variable that it resolves on each invocation. It should be possible to simply allow each clone to reference a separate, independent map.

[kaey]

It should be possible to simply allow each clone to reference a separate, independent map.

This works today and we're using it heavily. What error are you seeing?

[rsc]

Per the documentation of func (t *Template) Funcs(funcMap FuncMap) *Template: https://golang.org/pkg/html/template/#Template.Funcs

Funcs adds the elements of the argument map to the template's function map. It must be called before the template is parsed. It panics if a value in the map is not a function with appropriate return type. However, it is legal to overwrite elements of the map. The return value is the template, so calls can be chained.

In other words, one cannot call Funcs a second time after calling Clone. One can modify the existing map passed into Funcs before, but one cannot pass in a completely different map (which is important for thread-safety).

I think you are misreading the documentation. There are two maps in play.

Funcs copies entries from the map argument into a private map stored in the template. If the caller modifies Funcs's map argument after the call returns, nothing changes in the template. If you Clone the template, the clone gets its own private map initialized to be a copy of the original template (being cloned). If you call Funcs again, more entries are added to the private map.

Have you tried to use Funcs after Clone and had it fail? @kaey says it works for them, for example, and I believe I've done this too. Can you provide a failing test case if it doesn't work for you?

Thanks.

[rsc]

@michaelsafyan, any response to my comment from last week? The code you are trying to write should work already. If not it would help to provide a simple test case. Thanks!

[michaelsafyan]

Sorry, I've not had an opportunity to confirm that it works. But, assuming that it works, this may then simply be a request to clarify/update the documentation than an implementation change.

[rsc]

Will repurpose this issue for updating docs that Funcs can be called multiple times to replace earlier mappings.