handlebars-lang / handlebars.js

Minimal templating on steroids.
http://handlebarsjs.com
MIT License
17.82k stars 2.04k forks source link

Support for eval-less template execution #1934

Open legrego opened 1 year ago

legrego commented 1 year ago

One of the ways Kibana leverages Handlebars is via user-supplied templates, which are then executed in the browser.

We recently, finally, removed script-src 'unsafe-eval' from our Content Security Policy. The most challenging part of this exercise was finding a way to get Handlebars to execute templates without the need for dynamic code generation (in other words, without eval).

Inspired by @nknapp's comment in #1443, we took a stab at executing templates by walking the Handlebars-generated AST. We've had our implementation running in production for a little while now, and it's working well for us.

I won't go into too many details about our approach here, but I'll instead refer you to our implementation, which includes a descriptive README: https://github.com/elastic/kibana/tree/main/packages/kbn-handlebars

If there is a community interest for this, we would be happy to contribute our work to the Handlebars project. Our approach would live alongside the existing approach, rather than replace it. The performance tradeoff would not be acceptable for all users.

So, with all that said: 1) Is there interest in having Handlebars natively support eval-less execution? 2) If so, what would be the best way to incorporate our work into the project? What sort of API would you like to see?


aside: I am just the person asking the questions, credit for this work goes to @watson and @thomheymann

salmin89 commented 1 year ago
  1. Is there interest in having Handlebars natively support eval-less execution?
  2. If so, what would be the best way to incorporate our work into the project? What sort of API would you like to see?

aside: I am just the person asking the questions, credit for this work goes to @watson and @thomheymann

  1. Yes, we use handlebars and have hit a roadblock due to the origin of this issue. Your fix might be just what we need. Will test it out this week.
  2. I’m not sure if you want to maintain a fork or what the best approach is. I’ll let other people decide on this but I did want to raise appreciation for the solution provided.
legrego commented 1 year ago

Thanks for expressing your interest, @salmin89, and do let me know how you make out with your test!

salmin89 commented 1 year ago

@legrego your solution worked perfectly. I will copy the package for now, but I'd love to see it natively supported by handlebars.

If you don't get any response, are there any plans on publishing your package as a standalone solution?

legrego commented 1 year ago

@salmin89, happy to hear that.

If you don't get any response, are there any plans on publishing your package as a standalone solution?

We are discussing this as an alternative here: https://github.com/elastic/kibana/issues/150522. My personal preference is to have this become a part of the official distribution, so that we don't have to deal with compatibility changes across the various versions of Handlebars

davidfant commented 4 months ago

For those looking, I created a kibana fork to publish kbn-handlebars: https://www.npmjs.com/package/kbn-handlebars https://github.com/davidfant/kibana-handlebars/commit/e3e925c9ee5b4d7f3a8cc60e3b7c202841490490

spandl commented 4 months ago

This is great and working for many cases.

I came across a use case that isn't supported: nested expressions.

Example, the first block is working, but the second block isn't omitting the nested CUSTOM_HELPER

{{#if (CUSTOM_HELPER "parameter")}}
        Text
{{else}}
         Alternative text
{{/if}}
{{#if (CUSTOM_HELPER "parameter")}}
        Text with {{CUSTOM_HELPER_2 "parameter"}}
{{else}}
         Alternative text
{{/if}}

Chances this could make it into the library? Thanks!

Georift commented 1 month ago

Cheers for the library, and sharing the work with the open source community, so great to have a path forward on this while handlebars upstream is blocked.

Just out of interest I wonder if another solution to this probably could be shipping the unsafe-eval work into a worker. And posting messages to and from to do the rendering work. Broadly following something like this gist describes

@legrego @thomheymann @watson am I missing something that you might have found in the trenches? Would this not actually solve the issue for some reason or another?

nknapp commented 3 weeks ago

I forgot to mention in my comment, that I had started a project https://handlebars-ng.knappi.org a while ago. It should also create a language spec.

I stopped because there seemed to be no interest and I didn't need it personally.

Its a bit stale now, but I would invite everybody who wants to contribute. Although this might be too late now...

nknapp commented 3 weeks ago

Cheers for the library, and sharing the work with the open source community, so great to have a path forward on this while handlebars upstream is blocked.

Just out of interest I wonder if another solution to this probably could be shipping the unsafe-eval work into a worker. And posting messages to and from to do the rendering work. Broadly following something like this gist describes

@legrego @thomheymann @watson am I missing something that you might have found in the trenches? Would this not actually solve the issue for some reason or another?

The documentation at handlebars.js uses workers to run templates in the playground. I did this to prevent the main thread from being stuck in endless loops.