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

Support for eval-less template execution #1934

Open
legrego opened this issue Apr 18, 2023 · 9 comments
Open

Support for eval-less template execution #1934

legrego opened this issue Apr 18, 2023 · 9 comments

Comments

@legrego
Copy link

legrego commented Apr 18, 2023

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
Copy link

salmin89 commented May 3, 2023

  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
Copy link
Author

legrego commented May 3, 2023

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

@salmin89
Copy link

salmin89 commented May 5, 2023

@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
Copy link
Author

legrego commented May 5, 2023

@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: elastic/kibana#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
Copy link

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

@spandl
Copy link

spandl commented Feb 16, 2024

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
Copy link

Georift commented May 21, 2024

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
Copy link
Collaborator

nknapp commented Jun 10, 2024

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 nknapp closed this as completed Jun 10, 2024
@nknapp nknapp reopened this Jun 10, 2024
@nknapp
Copy link
Collaborator

nknapp commented Jun 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants