Prevent macros from seeing or affecting their calling scope.

mozilla · Feb 5, 2016 · c9c6abd · c9c6abd
1 parent 3f98533
commit c9c6abd
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -43,6 +43,10 @@ master (unreleased)
 * Fix whitespace control around nested tags/variables/comments. Thanks Ouyang
   Yadong. Merge of [#631](https://github.com/mozilla/nunjucks/pull/631).
 
+* [Breaking Change] Prevent macros from seeing or affecting their calling
+  scope; prevent includes from writing to the including scope. Merge of
+  [#667](https://github.com/mozilla/nunjucks/pull/667).
+
 * [Breaking Change] Prevent filter.escape from escaping SafeString. Thanks
   atian25. Merge of [#623](https://github.com/mozilla/nunjucks/pull/623).
 

diff --git a/src/compiler.js b/src/compiler.js
@@ -831,7 +831,8 @@ var Compiler = Object.extend({
             '[' + argNames.join(', ') + '], ',
             '[' + kwargNames.join(', ') + '], ',
             'function (' + realNames.join(', ') + ') {',
-            'frame = frame.push(true);',
+            'callerFrame = frame;',
+            'frame = new runtime.Frame();',
             'kwargs = kwargs || {};',
             'if (kwargs.hasOwnProperty("caller")) {',
             'frame.set("caller", kwargs.caller); }'
@@ -866,7 +867,7 @@ var Compiler = Object.extend({
         });
 
         frame = frame.pop();
-        this.emitLine('frame = frame.pop();');
+        this.emitLine('frame = callerFrame;');
         this.emitLine('return new runtime.SafeString(' + bufferId + ');');
         this.emitLine('});');
         this.popBufferId();

diff --git a/tests/compiler.js b/tests/compiler.js
@@ -584,6 +584,21 @@
             finish(done);
         });
 
+        it('should not leak variables set in nested scope within macro out to calling scope', function(done) {
+            equal('{% macro setFoo() %}' +
+                  '{% for y in [1] %}{% set x = "foo" %}{{ x }}{% endfor %}' +
+                  '{% endmacro %}' +
+                  '{% macro display() %}' +
+                  '{% set x = "bar" %}' +
+                  '{{ setFoo() }}' +
+                  '{{ x }}' +
+                  '{% endmacro %}' +
+                  '{{ display() }}',
+                  'foobar');
+
+            finish(done);
+        });
+
         it('should compile macros without leaking set to calling scope', function(done) {
             // This test checks that the issue #577 is resolved.
             // If the bug is not fixed, and set variables leak into the
@@ -607,6 +622,14 @@
             finish(done);
         });
 
+        it('should compile macros that cannot see variables in caller scope', function(done) {
+            equal('{% macro one(var) %}{{ two() }}{% endmacro %}' +
+                  '{% macro two() %}{{ var }}{% endmacro %}' +
+                  '{{ one("foo") }}',
+                  '');
+            finish(done);
+        });
+
         it('should compile call blocks', function(done) {
             equal('{% macro wrap(el) %}' +
                   '<{{ el }}>{{ caller() }}</{{ el }}>' +