From ea3ec4bd65199da6b4666113a030d745f8ff7c34 Mon Sep 17 00:00:00 2001
From: Stefan Siegl <stesie@brokenpipe.de>
Date: Sat, 1 Aug 2015 19:30:55 +0200
Subject: [PATCH] Fix commonjs memory leaks (and increase test coverage to
 100%)

---
 tests/commonjs_normalise_001.phpt | 23 +++++++++++++++++++++++
 tests/commonjs_normalise_002.phpt | 23 +++++++++++++++++++++++
 tests/commonjs_normalise_003.phpt | 25 +++++++++++++++++++++++++
 tests/commonjs_normalise_004.phpt | 30 ++++++++++++++++++++++++++++++
 tests/commonjs_normalise_005.phpt | 30 ++++++++++++++++++++++++++++++
 v8js_commonjs.cc                  | 10 +++++++++-
 6 files changed, 140 insertions(+), 1 deletion(-)
 create mode 100644 tests/commonjs_normalise_001.phpt
 create mode 100644 tests/commonjs_normalise_002.phpt
 create mode 100644 tests/commonjs_normalise_003.phpt
 create mode 100644 tests/commonjs_normalise_004.phpt
 create mode 100644 tests/commonjs_normalise_005.phpt

diff --git a/tests/commonjs_normalise_001.phpt b/tests/commonjs_normalise_001.phpt
new file mode 100644
index 0000000..ada367f
--- /dev/null
+++ b/tests/commonjs_normalise_001.phpt
@@ -0,0 +1,23 @@
+--TEST--
+Test V8Js::setModuleLoader : Path normalisation #001
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+$JS = <<< EOT
+var foo = require("./test");
+EOT;
+
+$v8 = new V8Js();
+$v8->setModuleLoader(function($module) {
+    print("setModuleLoader called for ".$module."\n");
+    return 'exports.bar = 23;';
+});
+
+$v8->executeString($JS, 'module.js');
+?>
+===EOF===
+--EXPECT--
+setModuleLoader called for test
+===EOF===
diff --git a/tests/commonjs_normalise_002.phpt b/tests/commonjs_normalise_002.phpt
new file mode 100644
index 0000000..b93dff4
--- /dev/null
+++ b/tests/commonjs_normalise_002.phpt
@@ -0,0 +1,23 @@
+--TEST--
+Test V8Js::setModuleLoader : Path normalisation #002
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+$JS = <<< EOT
+var foo = require("../../../test");
+EOT;
+
+$v8 = new V8Js();
+$v8->setModuleLoader(function($module) {
+    print("setModuleLoader called for ".$module."\n");
+    return 'exports.bar = 23;';
+});
+
+$v8->executeString($JS, 'module.js');
+?>
+===EOF===
+--EXPECT--
+setModuleLoader called for test
+===EOF===
diff --git a/tests/commonjs_normalise_003.phpt b/tests/commonjs_normalise_003.phpt
new file mode 100644
index 0000000..e8a95ff
--- /dev/null
+++ b/tests/commonjs_normalise_003.phpt
@@ -0,0 +1,25 @@
+--TEST--
+Test V8Js::setModuleLoader : Path normalisation #003
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+$JS = <<< EOT
+var foo = require("foo/test");
+var foo = require("foo/bar/baz/test");
+EOT;
+
+$v8 = new V8Js();
+$v8->setModuleLoader(function($module) {
+    print("setModuleLoader called for ".$module."\n");
+    return 'exports.bar = 23;';
+});
+
+$v8->executeString($JS, 'module.js');
+?>
+===EOF===
+--EXPECT--
+setModuleLoader called for foo/test
+setModuleLoader called for foo/bar/baz/test
+===EOF===
diff --git a/tests/commonjs_normalise_004.phpt b/tests/commonjs_normalise_004.phpt
new file mode 100644
index 0000000..f8f2b8f
--- /dev/null
+++ b/tests/commonjs_normalise_004.phpt
@@ -0,0 +1,30 @@
+--TEST--
+Test V8Js::setModuleLoader : Path normalisation #004
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+$JS = <<< EOT
+var foo = require("foo/test");
+EOT;
+
+$v8 = new V8Js();
+$v8->setModuleLoader(function($module) {
+    print("setModuleLoader called for ".$module."\n");
+
+    switch($module) {
+    case 'foo/test':
+        return 'require("./blar");';
+    case 'foo/blar':
+        return 'exports.bar = 23;';
+    }
+});
+
+$v8->executeString($JS, 'module.js');
+?>
+===EOF===
+--EXPECT--
+setModuleLoader called for foo/test
+setModuleLoader called for foo/blar
+===EOF===
diff --git a/tests/commonjs_normalise_005.phpt b/tests/commonjs_normalise_005.phpt
new file mode 100644
index 0000000..73be992
--- /dev/null
+++ b/tests/commonjs_normalise_005.phpt
@@ -0,0 +1,30 @@
+--TEST--
+Test V8Js::setModuleLoader : Path normalisation #005
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+$JS = <<< EOT
+var foo = require("foo/test");
+EOT;
+
+$v8 = new V8Js();
+$v8->setModuleLoader(function($module) {
+    print("setModuleLoader called for ".$module."\n");
+
+    switch($module) {
+    case 'foo/test':
+        return 'require("../blar");';
+    case 'blar':
+        return 'exports.bar = 23;';
+    }
+});
+
+$v8->executeString($JS, 'module.js');
+?>
+===EOF===
+--EXPECT--
+setModuleLoader called for foo/test
+setModuleLoader called for blar
+===EOF===
diff --git a/v8js_commonjs.cc b/v8js_commonjs.cc
index 41155b3..2d67e3c 100644
--- a/v8js_commonjs.cc
+++ b/v8js_commonjs.cc
@@ -76,12 +76,19 @@ void v8js_commonjs_normalise_identifier(char *base, char *identifier, char *norm
         if (!strcmp(term, "..")) {
             // Ignore parent term (..) if it's the first normalised term
             if (normalised_terms.size() > 0) {
-                // Remove the parent normalized term
+                // Remove the parent normalized term (and free it)
+                efree(normalised_terms.back());
                 normalised_terms.pop_back();
             }
+
+            // free the ".." term
+            efree(term);
         } else if (strcmp(term, ".")) {
             // Add the term if it's not the current term (.)
             normalised_terms.push_back(term);
+        } else {
+            // Discard "." term
+            efree(term);
         }
     }
 
@@ -102,5 +109,6 @@ void v8js_commonjs_normalise_identifier(char *base, char *identifier, char *norm
         }
 
         strcat(normalised_path, term);
+        efree(term);
     }
 }