Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
denis-sokolov opened this issue Aug 6, 2024 · 1 comment
Open
Labels

Comments

@denis-sokolov
Copy link

denis-sokolov commented Aug 6, 2024

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 bhousel added the bug label Sep 4, 2024
@bhousel
Copy link
Owner

bhousel commented Sep 4, 2024

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..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants