bhousel / node-diff3

A node.js library for text diffing and three-way-merge
Other
91 stars 13 forks source link

Crashes when comparing buffers with a word “__proto__” #83

Open denis-sokolov opened 3 months ago

denis-sokolov commented 3 months ago

When the second string contains the word __proto__, the library crashes inside the LCS function.

The cause is the arbitrary strings are used to key into a JS object with a prototype, so __proto__ is available even on ampty object. To solve it, one needs either an object with no prototype (Object.create(null)) or, even better, a Map. Here is the fix in a diff:

diff --git a/node_modules/node-diff3/index.mjs b/node_modules/node-diff3/index.mjs
index 91bf408..71dde1d 100644
--- a/node_modules/node-diff3/index.mjs
+++ b/node_modules/node-diff3/index.mjs
@@ -23,13 +23,13 @@ export {
 // Expects two arrays, finds longest common sequence
 function LCS(buffer1, buffer2) {

-  let equivalenceClasses = {};
+  let equivalenceClasses = new Map();
   for (let j = 0; j < buffer2.length; j++) {
     const item = buffer2[j];
-    if (equivalenceClasses[item]) {
-      equivalenceClasses[item].push(j);
+    if (equivalenceClasses.has(item)) {
+      equivalenceClasses.get(item).push(j);
     } else {
-      equivalenceClasses[item] = [j];
+      equivalenceClasses.set(item, [j]);
     }
   }

@@ -38,7 +38,7 @@ function LCS(buffer1, buffer2) {

   for (let i = 0; i < buffer1.length; i++) {
     const item = buffer1[i];
-    const buffer2indices = equivalenceClasses[item] || [];
+    const buffer2indices = equivalenceClasses.get(item) || [];
     let r = 0;
     let c = candidates[0];

(Thanks to patch-package.)

bhousel commented 2 months ago

Thanks for catching this! I haven't had the bandwidth to do much with this library, but I agree it's due for a refresh. Using modern keyed collections like Map and Set would improve the code greatly..