Clean code and fix tests

fiznool · May 12, 2021 · 16534f2 · 16534f2
1 parent 05e39bb
commit 16534f2
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 40 deletions.
diff --git a/index.js b/index.js
@@ -1,13 +1,17 @@
 'use strict';
 
-let TEST_REGEX = /^\$|\./;
+const TEST_REGEX = /^\$|\./;
 const TEST_REGEX_WITHOUT_DOT = /^\$/;
-let REPLACE_REGEX = /^\$|\./g;
+const REPLACE_REGEX = /^\$|\./g;
 
 function isPlainObject(obj) {
   return typeof obj === 'object' && obj !== null;
 }
 
+function getTestRegex(allowDots) {
+  return allowDots ? TEST_REGEX_WITHOUT_DOT : TEST_REGEX;
+}
+
 function withEach(target, cb) {
   (function act(obj) {
     if (Array.isArray(obj)) {
@@ -24,16 +28,14 @@ function withEach(target, cb) {
   })(target);
 }
 
+// target: 'prohibited.key': 'value',
+// allowDots: true
 function has(target, allowDots) {
-  let regex = TEST_REGEX;
-
-  if (allowDots) {
-    regex = TEST_REGEX_WITHOUT_DOT;
-  }
+  const regex = getTestRegex(allowDots);
 
   let hasProhibited = false;
   withEach(target, function (obj, val, key) {
-    if (TEST_REGEX.test(key)) {
+    if (regex.test(key)) {
       hasProhibited = true;
       return { shouldRecurse: false };
     } else {
@@ -44,19 +46,11 @@ function has(target, allowDots) {
   return hasProhibited;
 }
 
-function sanitize(target, options, regex) {
+function sanitize(target, options) {
   options = options || {};
 
   // Regex is not passed from the middleware
-  if (!regex) {
-    regex = TEST_REGEX;
-
-    if (options) {
-      if (options.allowDots) {
-        TEST_REGEX = TEST_REGEX_WITHOUT_DOT;
-      }
-    }
-  }
+  const regex = getTestRegex(options.allowDots);
 
   let replaceWith = null;
   if (!regex.test(options.replaceWith) && options.replaceWith !== '.') {
@@ -73,11 +67,7 @@ function sanitize(target, options, regex) {
         // Avoid to set __proto__ and constructor.prototype
         // https://portswigger.net/daily-swig/prototype-pollution-the-dangerous-and-underrated-vulnerability-impacting-javascript-applications
         // https://snyk.io/vuln/SNYK-JS-LODASH-73638
-        if (
-          key !== '__proto__' &&
-          key !== 'constructor' &&
-          key !== 'prototype'
-        ) {
+        if (key !== '__proto__' && key !== 'constructor' && key !== 'prototype') {
           obj[key] = val;
         }
       } else {
@@ -98,15 +88,7 @@ function middleware(options) {
   return function (req, res, next) {
     ['body', 'params', 'headers', 'query'].forEach(function (k) {
       if (req[k]) {
-        req[k] = sanitize(
-          req[k],
-          options,
-          options
-            ? options.allowDots
-              ? TEST_REGEX_WITHOUT_DOT
-              : TEST_REGEX
-            : null
-        );
+        req[k] = sanitize(req[k], options);
       }
     });
     next();

diff --git a/test.js b/test.js
@@ -1500,11 +1500,11 @@ describe('Express Mongo Sanitize, Dots included', function () {
       expect(sanitize.has(input, true)).to.be.true;
     });
 
-    it('should return true if the object has a key containing a `.`', function () {
+    it('should return false if the object has a key containing a `.`, when allowDots=true', function () {
       const input = {
         'prohibited.key': 'value',
       };
-      expect(sanitize.has(input, true)).to.be.true;
+      expect(sanitize.has(input, true)).to.be.false;
     });
 
     it('should return true if the object has a nested key beginning with a `$`', function () {
@@ -1516,13 +1516,13 @@ describe('Express Mongo Sanitize, Dots included', function () {
       expect(sanitize.has(input, true)).to.be.true;
     });
 
-    it('should return true if the object has a nested key containing a `.`', function () {
+    it('should return true if the object has a nested key containing a `.`, when allowDots=true', function () {
       const input = {
         nested: {
           'prohibited.key': 'value',
         },
       };
-      expect(sanitize.has(input, true)).to.be.true;
+      expect(sanitize.has(input, true)).to.be.false;
     });
 
     it('should return true if the array contains an object with a key beginning with a `$`', function () {
@@ -1534,13 +1534,13 @@ describe('Express Mongo Sanitize, Dots included', function () {
       expect(sanitize.has(input, true)).to.be.true;
     });
 
-    it('should return true if the array contains an object with a key containing a `.`', function () {
+    it('should return true if the array contains an object with a key containing a `.`, when allowDots=true', function () {
       const input = [
         {
           'prohibited.key': 'value',
         },
       ];
-      expect(sanitize.has(input, true)).to.be.true;
+      expect(sanitize.has(input, true)).to.be.false;
     });
 
     it('should return true if the payload contains a deeply nested object with a key beginning with a `$`', function () {
@@ -1560,7 +1560,7 @@ describe('Express Mongo Sanitize, Dots included', function () {
       expect(sanitize.has(input, true)).to.be.true;
     });
 
-    it('should return true if the payload contains a deeply nested object with a key containing a `.`', function () {
+    it('should return true if the payload contains a deeply nested object with a key containing a `.`, when allowDots=true', function () {
       const input = [
         {
           some: {
@@ -1574,7 +1574,7 @@ describe('Express Mongo Sanitize, Dots included', function () {
           },
         },
       ];
-      expect(sanitize.has(input, true)).to.be.true;
+      expect(sanitize.has(input, true)).to.be.false;
     });
 
     it("should return false if the payload doesn't contain any prohibited characters", function () {