Prevent options from leaking (#1595)

The options object passed into `render` and `renderSync` is only
shallow copied. This is an issue when nested objects and arrays like
custom importer and functions. In the case of custom function we
wrap the provided function. This wrapped version replaces the provided
one which can then leak back into the calling code. This is an issue
if `render*` is called in a loop like in gulp-sass.

Fixes #1168

lib/index.js

@@ -4,6 +4,7 @@
 
 var path = require('path'),
     util = require('util'),
+    clonedeep = require('lodash.clonedeep'),
     errors = require('./errors'),
     sass = require('./extensions');
 
@@ -170,8 +171,8 @@ function getLinefeed(options) {
  * @api private
  */
 
-function getOptions(options, cb) {
-  options = options || {};
+function getOptions(opts, cb) {
+  var options = clonedeep(opts || {});
   options.sourceComments = options.sourceComments || false;
   if (options.hasOwnProperty('file')) {
     options.file = getInputFile(options);
@@ -263,8 +264,8 @@ function normalizeFunctionSignature(signature, callback) {
  * @api public
  */
 
-module.exports.render = function(options, cb) {
-  options = getOptions(options, cb);
+module.exports.render = function(opts, cb) {
+  var options = getOptions(opts, cb);
 
   // options.error and options.success are for libsass binding
   options.error = function(err) {
@@ -293,6 +294,7 @@ module.exports.render = function(options, cb) {
 
   if (importer) {
     if (Array.isArray(importer)) {
+      options.importer = [];
       importer.forEach(function(subject, index) {
         options.importer[index] = function(file, prev, bridge) {
           function done(result) {
@@ -321,7 +323,7 @@ module.exports.render = function(options, cb) {
     }
   }
 
-  var functions = options.functions;
+  var functions = clonedeep(options.functions);
 
   if (functions) {
     options.functions = {};
@@ -362,13 +364,13 @@ module.exports.render = function(options, cb) {
  * @api public
  */
 
-module.exports.renderSync = function(options) {
-  options = getOptions(options);
-
+module.exports.renderSync = function(opts) {
+  var options = getOptions(opts);
   var importer = options.importer;
 
   if (importer) {
     if (Array.isArray(importer)) {
+      options.importer = [];
       importer.forEach(function(subject, index) {
         options.importer[index] = function(file, prev) {
           var result = subject.call(options.context, file, prev);
@@ -385,7 +387,7 @@ module.exports.renderSync = function(options) {
     }
   }
 
-  var functions = options.functions;
+  var functions = clonedeep(options.functions);
 
   if (options.functions) {
     options.functions = {};

package.json

@@ -58,8 +58,9 @@
     "gaze": "^1.0.0",
     "get-stdin": "^4.0.1",
     "glob": "^7.0.3",
-    "meow": "^3.7.0",
     "in-publish": "^2.0.0",
+    "lodash.clonedeep": "^4.3.2",
+    "meow": "^3.7.0",
     "mkdirp": "^0.5.1",
     "nan": "^2.3.2",
     "node-gyp": "^3.3.1",

test/api.js

@@ -535,19 +535,18 @@ describe('api', function() {
       });
     });
 
-    it('should copy all options properties', function(done) {
+    it('should wrap importer options', function(done) {
       var options;
       options = {
         data: src,
         importer: function() {
-          assert.strictEqual(this.options.importer, options.importer);
+          assert.notStrictEqual(this.options.importer, options.importer);
           return {
             contents: 'div {color: yellow;}'
           };
         }
       };
       sass.render(options, function() {
-        assert.strictEqual(this.options, options);
         done();
       });
     });