Skip to content

Commit 93b0760

Browse files
kpdeckerkpdecker
committed
Bulletproof AST.helpers.helperExpression
Avoid undefined values and potential false positives from other type values such as partials. Fixes #1055
1 parent 85750f8 commit 93b0760

File tree

2 files changed

+55
-15
lines changed

2 files changed

+55
-15
lines changed

‎lib/handlebars/compiler/ast.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ let AST = {
131131
// * it is an eligible helper, and
132132
// * it has at least one parameter or hash segment
133133
helperExpression: function(node) {
134-
return !!(node.type === 'SubExpression' || node.params.length || node.hash);
134+
return (node.type === 'SubExpression')
135+
|| ((node.type === 'MustacheStatement' || node.type === 'BlockStatement')
136+
&& !!((node.params && node.params.length) || node.hash));
135137
},
136138

137139
scopedId: function(path) {

‎spec/ast.js

+52-14
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ describe('ast', function() {
33
return;
44
}
55

6+
var AST = Handlebars.AST;
7+
68
var LOCATION_INFO = {
79
start: {
810
line: 1,
@@ -23,7 +25,7 @@ describe('ast', function() {
2325

2426
describe('MustacheStatement', function() {
2527
it('should store args', function() {
26-
var mustache = new handlebarsEnv.AST.MustacheStatement({}, null, null, true, {}, LOCATION_INFO);
28+
var mustache = new AST.MustacheStatement({}, null, null, true, {}, LOCATION_INFO);
2729
equals(mustache.type, 'MustacheStatement');
2830
equals(mustache.escaped, true);
2931
testLocationInfoStorage(mustache);
@@ -37,8 +39,8 @@ describe('ast', function() {
3739
});
3840

3941
it('stores location info', function() {
40-
var mustacheNode = new handlebarsEnv.AST.MustacheStatement([{ original: 'foo'}], null, null, false, {});
41-
var block = new handlebarsEnv.AST.BlockStatement(
42+
var mustacheNode = new AST.MustacheStatement([{ original: 'foo'}], null, null, false, {});
43+
var block = new AST.BlockStatement(
4244
mustacheNode,
4345
null, null,
4446
{body: []},
@@ -52,78 +54,114 @@ describe('ast', function() {
5254
});
5355
describe('PathExpression', function() {
5456
it('stores location info', function() {
55-
var idNode = new handlebarsEnv.AST.PathExpression(false, 0, [], 'foo', LOCATION_INFO);
57+
var idNode = new AST.PathExpression(false, 0, [], 'foo', LOCATION_INFO);
5658
testLocationInfoStorage(idNode);
5759
});
5860
});
5961

6062
describe('Hash', function() {
6163
it('stores location info', function() {
62-
var hash = new handlebarsEnv.AST.Hash([], LOCATION_INFO);
64+
var hash = new AST.Hash([], LOCATION_INFO);
6365
testLocationInfoStorage(hash);
6466
});
6567
});
6668

6769
describe('ContentStatement', function() {
6870
it('stores location info', function() {
69-
var content = new handlebarsEnv.AST.ContentStatement('HI', LOCATION_INFO);
71+
var content = new AST.ContentStatement('HI', LOCATION_INFO);
7072
testLocationInfoStorage(content);
7173
});
7274
});
7375

7476
describe('CommentStatement', function() {
7577
it('stores location info', function() {
76-
var comment = new handlebarsEnv.AST.CommentStatement('HI', {}, LOCATION_INFO);
78+
var comment = new AST.CommentStatement('HI', {}, LOCATION_INFO);
7779
testLocationInfoStorage(comment);
7880
});
7981
});
8082

8183
describe('NumberLiteral', function() {
8284
it('stores location info', function() {
83-
var integer = new handlebarsEnv.AST.NumberLiteral('6', LOCATION_INFO);
85+
var integer = new AST.NumberLiteral('6', LOCATION_INFO);
8486
testLocationInfoStorage(integer);
8587
});
8688
});
8789

8890
describe('StringLiteral', function() {
8991
it('stores location info', function() {
90-
var string = new handlebarsEnv.AST.StringLiteral('6', LOCATION_INFO);
92+
var string = new AST.StringLiteral('6', LOCATION_INFO);
9193
testLocationInfoStorage(string);
9294
});
9395
});
9496

9597
describe('BooleanLiteral', function() {
9698
it('stores location info', function() {
97-
var bool = new handlebarsEnv.AST.BooleanLiteral('true', LOCATION_INFO);
99+
var bool = new AST.BooleanLiteral('true', LOCATION_INFO);
98100
testLocationInfoStorage(bool);
99101
});
100102
});
101103

102104
describe('PartialStatement', function() {
103105
it('provides default params', function() {
104-
var pn = new handlebarsEnv.AST.PartialStatement('so_partial', undefined, {}, {}, LOCATION_INFO);
106+
var pn = new AST.PartialStatement('so_partial', undefined, {}, {}, LOCATION_INFO);
105107
equals(pn.params.length, 0);
106108
});
107109
it('stores location info', function() {
108-
var pn = new handlebarsEnv.AST.PartialStatement('so_partial', [], {}, {}, LOCATION_INFO);
110+
var pn = new AST.PartialStatement('so_partial', [], {}, {}, LOCATION_INFO);
109111
testLocationInfoStorage(pn);
110112
});
111113
});
112114

113115
describe('Program', function() {
114116
it('storing location info', function() {
115-
var pn = new handlebarsEnv.AST.Program([], null, {}, LOCATION_INFO);
117+
var pn = new AST.Program([], null, {}, LOCATION_INFO);
116118
testLocationInfoStorage(pn);
117119
});
118120
});
119121

120122
describe('SubExpression', function() {
121123
it('provides default params', function() {
122-
var pn = new handlebarsEnv.AST.SubExpression('path', undefined, {}, LOCATION_INFO);
124+
var pn = new AST.SubExpression('path', undefined, {}, LOCATION_INFO);
123125
equals(pn.params.length, 0);
124126
});
125127
});
126128

129+
describe('helpers', function() {
130+
describe('#helperExpression', function() {
131+
it('should handle mustache statements', function() {
132+
equals(AST.helpers.helperExpression(new AST.MustacheStatement('foo', [], undefined, false, {}, LOCATION_INFO)), false);
133+
equals(AST.helpers.helperExpression(new AST.MustacheStatement('foo', [1], undefined, false, {}, LOCATION_INFO)), true);
134+
equals(AST.helpers.helperExpression(new AST.MustacheStatement('foo', [], {}, false, {}, LOCATION_INFO)), true);
135+
});
136+
it('should handle block statements', function() {
137+
equals(AST.helpers.helperExpression(new AST.BlockStatement('foo', [], undefined, false, {}, LOCATION_INFO)), false);
138+
equals(AST.helpers.helperExpression(new AST.BlockStatement('foo', [1], undefined, false, {}, LOCATION_INFO)), true);
139+
equals(AST.helpers.helperExpression(new AST.BlockStatement('foo', [], {}, false, {}, LOCATION_INFO)), true);
140+
});
141+
it('should handle subexpressions', function() {
142+
equals(AST.helpers.helperExpression(new AST.SubExpression()), true);
143+
});
144+
it('should work with non-helper nodes', function() {
145+
equals(AST.helpers.helperExpression(new AST.Program([], [], {}, LOCATION_INFO)), false);
146+
147+
equals(AST.helpers.helperExpression(new AST.PartialStatement()), false);
148+
equals(AST.helpers.helperExpression(new AST.ContentStatement('a', LOCATION_INFO)), false);
149+
equals(AST.helpers.helperExpression(new AST.CommentStatement('a', {}, LOCATION_INFO)), false);
150+
151+
equals(AST.helpers.helperExpression(new AST.PathExpression(false, 0, ['a'], 'a', LOCATION_INFO)), false);
152+
153+
equals(AST.helpers.helperExpression(new AST.StringLiteral('a', LOCATION_INFO)), false);
154+
equals(AST.helpers.helperExpression(new AST.NumberLiteral(1, LOCATION_INFO)), false);
155+
equals(AST.helpers.helperExpression(new AST.BooleanLiteral(true, LOCATION_INFO)), false);
156+
equals(AST.helpers.helperExpression(new AST.UndefinedLiteral(LOCATION_INFO)), false);
157+
equals(AST.helpers.helperExpression(new AST.NullLiteral(LOCATION_INFO)), false);
158+
159+
equals(AST.helpers.helperExpression(new AST.Hash([], LOCATION_INFO)), false);
160+
equals(AST.helpers.helperExpression(new AST.HashPair('foo', 'bar', LOCATION_INFO)), false);
161+
});
162+
});
163+
});
164+
127165
describe('Line Numbers', function() {
128166
var ast, body;
129167

0 commit comments

Comments
 (0)