batiste / pug-vdom

PUG template to HyperScript Virtual DOM
MIT License
18 stars 6 forks source link

Load parsed scripts #38

Closed gryphonmyers closed 5 years ago

gryphonmyers commented 5 years ago

Adds support for script execution in nodes that have parsed HTML strings.

Adds some tests covering this functionality.

Also fixes previously broken tests - my bad!

batiste commented 5 years ago

Is this something that makes any sense in the context of the server? I find this feature disturbing and dangerous...

gryphonmyers commented 5 years ago

Sure well, it does respect pug's built in syntax for escaped vs non-escaped HTML (you have to opt in to this behavior by using flags that pug warns against). The nature of runtime HTML parsing has an additional quirk you wouldn't get in a pre-compiled context though: your HTML you add to the DOM won't run scripts. This means if you have an HTML snippet loaded in at runtime with, say, a Twitter embed in it (my exact use case) it won't run. If you had used the same pug template in a compiled context, however, it would.

Sure running scripts in parsed HTML is risky, but I think that should be up to the template's author to use or not use the sanitization flags. Otherwise this just seems broken.

If it would make you more comfortable, I could hide this functionality behind an additional compile-time flag?

On Tue, Jun 18, 2019, 5:53 AM Batiste Bieler notifications@github.com wrote:

Is this something that makes any sense in the context of the server? I find this feature disturbing and dangerous...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/batiste/pug-vdom/pull/38?email_source=notifications&email_token=ABMENWH6E76OESNRRPWROODP3DLDFA5CNFSM4HYZGTPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX6OEAI#issuecomment-503112193, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMENWFKTHGMKSW2IY3HSUTP3DLDFANCNFSM4HYZGTPA .

batiste commented 5 years ago

@gryphonmyers this is a feature that works in the original PUG? Could you point me to it?

gryphonmyers commented 5 years ago

https://pugjs.org/language/code.html#unescaped-buffered-code

It doesn't specifically have to support this feature, it just allows for rendering arbitrary unescaped HTML. In a pre-compiled context this allows you to render a script tag, which will execute once the browser parses it. The only reason I had to make this PR is that there is a particular quirk with appending HTML to the DOM at runtime - it won't execute script tags. There are a few workarounds to this issue - the one I used was manually recreating the script tags and re-appending them to the dom.

On Wed, Jun 19, 2019, 12:20 AM Batiste Bieler notifications@github.com wrote:

@gryphonmyers https://github.com/gryphonmyers this is a feature that works in the original PUG? Could you point me to it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/batiste/pug-vdom/pull/38?email_source=notifications&email_token=ABMENWGHKX7CTA2NTXQZBLLP3HM2HA5CNFSM4HYZGTPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYA6BHQ#issuecomment-503439518, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMENWAHS4A5BRUZVV6MIALP3HM2HANCNFSM4HYZGTPA .

albb0920 commented 5 years ago

I think the feature is quite nice and makes sense.

In pug-vdom, this works:

if condition
  script
    | alert("ok")

But this doesn't:

if condition
  | <script>alert("ok")</script>

Both works in original pug. This is a side effect of how runtime.makeHtmlNode is implemented.

In the first example, the script tag is created by virtual-dom via createElement, which will run the script. In the second example, the script tag is created in runtime.makeHtmlNode, through assigning innerHTML, which doesn't run the script.

What this patch does, is replace all script created in innerHTML, and re-create it with createElement, to allow the script to run.

In both examples, there's nothing dangerous. Things only gets scary when people use user input, which is also allowed in original pug with a big warning in the document.