krakenjs / adaro

A Dust.js view renderer for express
Other
127 stars 41 forks source link

when render data has properties like "views,settings,ext", it will throw errors #21

Open winnieBear opened 10 years ago

winnieBear commented 10 years ago

when call like this in controller,

var data = {
    "key is not views,settings,ext":"run ok "
   };
    res.render('index', data);

if data has no properties like "views,settings,ext", it can run ok,
BUT,when data has these properties,like this

var data = {
    name: 'test',
    views: 'this is test string!',
    settings: 'this is the data from models',
    ext:'but it will not ok!'
  };
  res.render('index', data);

the programe will throw errors of 503.

I find these errors is caused by adaro\lib\engine.js,adaro\lib\utils.js, in which the above three properties is used as important parameters.

But these properties name can also be used as data for render. When your data has "views,settings,ext" properties name, the programe will core.

You can git checkout my test case to verify it.

pvenkatakrishnan commented 10 years ago

sorry for the delay. looking into this.

pvenkatakrishnan commented 9 years ago

The reason for this being a problem is data and res.locals get merged before getting passed to view engine in express. Unfortunately, views, settings and ext are internally defined values when dust processes which collides with your definitions and hence the problem.

pvenkatakrishnan commented 9 years ago

Agreed it is invonvenient(dust could have used something better namespaced), but is there a way you can exclude these property names ?

aredridel commented 9 years ago

I think I can solve this in the adaro 1.0 release, since there's already a rescoping of some values in there.

aks- commented 9 years ago

It will be fixed. I made these changes which will be reflected in Express 5 https://github.com/strongloop/express/pull/2648

aredridel commented 9 years ago

Indeed. Doesn't look like it'll make the 1.0 release, since Express itself is what mixes these concerns together.

aks- commented 9 years ago

Aha, right