Skip to content

let vue-template-compiler work in strict mode #3832

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

Closed
micheletriaca opened this issue Oct 2, 2016 · 10 comments
Closed

let vue-template-compiler work in strict mode #3832

micheletriaca opened this issue Oct 2, 2016 · 10 comments

Comments

@micheletriaca
Copy link

Hi,
I really love vue and I would like to use it to customize the user interface of Salesforce.
The problem is that Salesforce has recently introduced a security feature called LockerService, that enforces strict mode on any 3rd party libraries that are included in the project and in the near future will enforce also CSP.
I've done some tests and it seems that the only thing that prevent a Salesforce developer from using vue is that the compiler renders the templates using the "with" functionality.
Using inline templates is not an option because they use 'new Function', violating CSP.
It seems to me that it should be possible to modify the compiler to be strict mode compliant.. Is this improvement in the roadmap? Thank you for your awesome work

@fnlctrl
Copy link
Member

fnlctrl commented Oct 2, 2016

Hi, thanks for filling this issue.

Is this improvement in the roadmap?

No.. Using with this was the design decision in 2.0 to avoid manually parsing the expressions (which bloats the code and takes more time), so it's unlikely to be changed.

However it's relatively easy to remove with this (and you'd have to add this. to every expression you write in the template afterwards), so maybe it can be an option for the compiler.

For now, you can try using jsx (if you don't have negative opinions about it..), there's a official babel plugin for compiling jsx to vue's render function, which really works great and is much more flexible than templates (and doesn't require with this).

@yyx990803
Copy link
Member

yyx990803 commented Oct 2, 2016

Hmm, pre-compiled templates, although using with (this), is CPS-compliant, but does not work in strict mode. It seems a bit over the top for a platform to enforce strict mode on 3rd party code though.

As of now, removing with (this) from generated render functions involves some fundamental changes to the compiler implementation for very marginal gain, so unfortunately I don't think that will happen any time soon.

The alternative solution, as @fnlctrl suggested, is using render function / JSX.

@micheletriaca
Copy link
Author

Hi @yyx990803 , the problem here is that Salesforce exposes a SPA in which you can develop components.. these components must not directly manipulate the DOM of others components, but should use the "official" way based on events to communicate with other components. Strict mode and other security features are required to keep a component in a sort of sandbox. That said, I've tried with JSX/render functions. It works, but pre-compiled templates are a lot better. You can still be productive even with JSX except for the lack of a valid v-model alternative (you can use a value + on-click binding but is clearly not optimal if you are doing a lot of data entry)... I understand that my situation is probably an edge case, but I'm still very sad to hear that this feature is not in the roadmap

@coolaj86
Copy link

coolaj86 commented Nov 6, 2016

Also concerned about having to turn off security features to use vue, which could be a deal breaker.

I'm just casually cruising through the code and I haven't come across anything that necessitates with (this) { ... }.

I'd like to know if there's a technical reason for it, but there doesn't seem to be.

with (this) {
  return /* built string */
}

can be replaced with

var v = this;
return /* built string */

And then also replacing lines like this

addProp(el, 'value', isNative ? ("_s(" + value + ")") : ("(" + value + ")"))

with lines like this:

addProp(el, 'value', isNative ? ("v._s(v." + value + ")") : ("(v." + value + ")"))

Furthermore, I don't understand the purpose of building with strings rather than using actual functions and replacing dynamic closure variables templates like this:

// create a closure with an easy to replace name for some value
var __FOO__ = el.value;

// create a function in regular javascript, not as a string
(function () {
  var x = __FOO__;
}).toString().replace(/__FOO__/g, el.value)
// this is a usable function in standalone mode and easy to template for pre-built mode

I believe that such a pattern would make it possible to not violate any CSP and still have the build tools get the strings that they need for export.

If I could get some help from someone a little more familiar with the code I'm pretty confident that I could get this converted from insecure non-standard es3 with proper and secure es5.1

Is there something that I'm missing as to why this isn't possible to do in this way?

@micheletriaca
Copy link
Author

micheletriaca commented Nov 6, 2016

I've resolved my issue modifying the compiler in this way:

diff --git a/src/compiler/codegen/index.js b/src/compiler/codegen/index.js
index 1de4605..44cc965 100644
--- a/src/compiler/codegen/index.js
+++ b/src/compiler/codegen/index.js
@@ -30,7 +30,7 @@ export function generate (
   const code = ast ? genElement(ast) : '_h("div")'
   staticRenderFns = prevStaticRenderFns
   return {
-    render: `with(this){return ${code}}`,
+    render: `return (function(){var _self=this;var _h=this._h.bind(this);var _s=this._s.bind(this);var _n=this._n.bind(this);var _e=this._e.bind(this);var _q=this._q.bind(this);var _i=this._i.bind(this);var _m=this._m.bind(this);var _f=this._f.bind(this);var _l=this._l.bind(this);var _t=this._t.bind(this);var _b=this._b.bind(this);var _k=this._k.bind(this);return ${code}}).call(this,this)`,
     staticRenderFns: currentStaticRenderFns
   }
 }
@@ -39,7 +39,7 @@ function genElement (el: ASTElement): string {
   if (el.staticRoot && !el.staticProcessed) {
     // hoist static sub-trees out
     el.staticProcessed = true
-    staticRenderFns.push(`with(this){return ${genElement(el)}}`)
+    staticRenderFns.push(`return (function(){var _self=this;var _h=this._h.bind(this);var _s=this._s.bind(this);var _n=this._n.bind(this);var _e=this._e.bind(this);var _q=this._q.bind(this);var _i=this._i.bind(this);var _m=this._m.bind(this);var _f=this._f.bind(this);var _l=this._l.bind(this);var _t=this._t.bind(this);var _b=this._b.bind(this);var _k=this._k.bind(this);return ${genElement(el)}}).call(this,this)`)
     return `_m(${staticRenderFns.length - 1}${el.staticInFor ? ',true' : ''})`
   } else if (el.for && !el.forProcessed) {
     return genFor(el)

This simple modification let me compile templates in strict mode. The only difference with the with(this) approach is that every expression that references view model variables must be prefixed with _self.. Example:

<div v-show="isVisible">xx</div>

becomes

<div v-show="_self.isVisible">xx</div>

I think that this could be avoided introducing an expression parser and some logic to determine if an expression variable is a view model variable, but this could slow down compilation and it is not so easy to do as the patch I did.

It would be great to have an option to instruct vue-loader or vueify to compile templates in strict-mode. I think that users that need this functionality could accept to prefix the vm variables with _self. in their code. It's better that having to switch to another framework..

@coolaj86
Copy link

coolaj86 commented Nov 6, 2016

You really don't even have to get that fancy, check this out:

#4135

All I do is add a prefix in a few key functions. I just tested with a simple demo, but perhaps you can test too and help me find where I'm missing the fix.

Beyond strict mode, I don't see any reason (yet) that this needs to be created from insecure strings and run through eval. It should be easy to create as functions and stringify them later.

@yyx990803
Copy link
Member

See the reasons explained in #4115

@micheletriaca
Copy link
Author

I have modified the compiler to be strict mode compliant changing just 2 lines of code. Why are you saying that this solution is fancy?

I agree with you that your PR is trying to address the issue in a more 'clean' way, but be careful because there could be a lot of edge cases to address. Maybe I would change v to something different because you could have a v value in your vm, or you could use v as a loop variable in a v-for. And I think you should prefix your vm variables with v. (_self. in my case) anyway...

It seems that we are trying to do exactly the same thing, using two different approaches. Maybe my approach could be easier if you want to modify the compiler to have an additional useStrictMode option

@coolaj86
Copy link

coolaj86 commented Nov 6, 2016

@micheletriaca sorry, when I was looking at it on my phone and it looked like a lot lot more because of the extensive line wrapping. Now that I see it in the browser I see it's just the two.

I agree. I think the controller-as syntax in Angular is basically the same idea of the in-template prefix as you're saying (and honestly that's what I would prefer).

Do you think there's mix between our two approaches that could keep api compat and not break existing apps?

@micheletriaca
Copy link
Author

Using, for example, my approach, the users could compile templates in strict mode but, in that case, they would need to write the expressions referencing also the scope (as you do in JSX, for example). That's what I'm doing now, but it seems to me that this solution is unlikely to be accepted (this is the reason why I haven't created a PR). If you want not to break existing apps, I think the correct approach to address the issue is the one suggested by @yyx990803 : a babel plugin to postprocess templates. You have to use some full javascript parser to assign the correct scope to the variables in the expressions :-(

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

No branches or pull requests

4 participants