Skip to content

Commit 51f0151

Browse files
committed
Fix explicit null value handling in compat mode
Follow-up to GH-2150. `container.lookup` used `result != null` which caused explicit null values to fall through to parent contexts. This was inconsistent with expected behavior and with Mustache, which stops and returns a key when present in the object, regardless of its value. The whole point of `compat` mode is to match Mustache's lookup semantics, so I don't think this behavior was intended. There were no tests for `compat` with an explicit null value, apart from a couple `compat` + `strict` mode tests I just added myself in GH-2150. One of these tests ("should still perform recursive lookup with a multi-part path not in context") was inconsistent with the test immediately below it ("should directly return an explicitly null property"), and that is fixed here. This change makes it possible for unify the lookup logic in `container.lookup` and `container.strictLookup`, additionally fixing an issue where `strictLookup` didn't respect the `allowedProtoMethods` runtime option the same as `lookup` (see the added test in `spec/security.js` compared to the test above it).
1 parent 3105ca7 commit 51f0151

4 files changed

Lines changed: 33 additions & 18 deletions

File tree

lib/handlebars/runtime.js

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -117,20 +117,10 @@ export function template(templateSpec, env) {
117117
return container.lookupProperty(obj, name);
118118
},
119119
strictLookup: function (depths, name, loc) {
120-
const len = depths.length;
121-
let depth;
122-
for (let i = 0; i < len; i++) {
123-
const d = depths[i];
124-
if (
125-
d &&
126-
(typeof d === 'object' || typeof d === 'function') &&
127-
name in d
128-
) {
129-
depth = d;
130-
break;
131-
}
132-
}
133-
return container.strict(depth, name, loc);
120+
const result = container.lookup(depths, name);
121+
return result !== undefined
122+
? result
123+
: container.strict(undefined, name, loc);
134124
},
135125
lookupProperty: function (parent, propertyName) {
136126
if (Utils.isMap(parent)) {
@@ -153,9 +143,17 @@ export function template(templateSpec, env) {
153143
lookup: function (depths, name) {
154144
const len = depths.length;
155145
for (let i = 0; i < len; i++) {
156-
let result = depths[i] && container.lookupProperty(depths[i], name);
157-
if (result != null) {
158-
return depths[i][name];
146+
const d = depths[i];
147+
if (d == null) continue;
148+
if (typeof d === 'object' || typeof d === 'function') {
149+
if (name in d) {
150+
return container.lookupProperty(d, name);
151+
}
152+
} else {
153+
const result = container.lookupProperty(d, name);
154+
if (result != null) {
155+
return result;
156+
}
159157
}
160158
}
161159
},

spec/blocks.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,13 @@ describe('blocks', function () {
259259
});
260260

261261
describe('compat mode', function () {
262+
it('should directly return an explicitly null property', function () {
263+
expectTemplate('{{#each items}}{{name}}{{/each}}')
264+
.withCompileOptions({ compat: true })
265+
.withInput({ name: 'root', items: [{ name: null }] })
266+
.toCompileTo('');
267+
});
268+
262269
it('block with deep recursive lookup lookup', function () {
263270
expectTemplate(
264271
'{{#outer}}Goodbye {{#inner}}cruel {{omg}}{{/inner}}{{/outer}}'

spec/security.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,16 @@ describe('security issues', function () {
302302
})
303303
.toCompileTo('abc');
304304
});
305+
306+
it('should not cause recursive lookup if allowed through options(in "compat+strict" mode)', function () {
307+
expectTemplate('{{#aString}}{{trim}}{{/aString}}')
308+
.withInput({ aString: ' abc ', trim: 'trim' })
309+
.withCompileOptions({ compat: true, strict: true })
310+
.withRuntimeOptions({
311+
allowedProtoMethods: { trim: true },
312+
})
313+
.toCompileTo('abc');
314+
});
305315
});
306316

307317
describe('control access to prototype non-methods via "allowedProtoProperties" and "allowProtoPropertiesByDefault', function () {

spec/strict.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ describe('strict', function () {
166166
it('should still perform recursive lookup with a multi-part path not in context', function () {
167167
expectTemplate('{{#with child}}{{name.first}}{{/with}}')
168168
.withCompileOptions({ strict: true, compat: true })
169-
.withInput({ name: { first: 'root' }, child: { name: null } })
169+
.withInput({ name: { first: 'root' }, child: { x: 'y' } })
170170
.toCompileTo('root');
171171
});
172172

0 commit comments

Comments
 (0)