Missing param validation issue145

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 All notable changes to this project will be documented in this file.
 
+## 4.X.X
+
+### Minor Changes
+- Added parameter checking to setCookie so as to error out when no URL was passed in
+
 ## 4.0.0
 
 ### Breaking Changes (Major Version)

lib/cookie.js

@@ -1,5 +1,5 @@
 /*!
- * Copyright (c) 2015, Salesforce.com, Inc.
+ * Copyright (c) 2015-2020, Salesforce.com, Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -36,6 +36,7 @@ const pubsuffix = require("./pubsuffix-psl");
 const Store = require("./store").Store;
 const MemoryCookieStore = require("./memstore").MemoryCookieStore;
 const pathMatch = require("./pathMatch").pathMatch;
+const validators = require("./validators.js");
 const VERSION = require("./version");
 const { fromCallback } = require("universalify");
 
@@ -79,6 +80,7 @@ const SAME_SITE_CONTEXT_VAL_ERR =
   'Invalid sameSiteContext option for getCookies(); expected one of "strict", "lax", or "none"';
 
 function checkSameSiteContext(value) {
+  validators.validate(validators.isNonEmptyString(value), value);
   const context = String(value).toLowerCase();
   if (context === "none" || context === "lax" || context === "strict") {
     return context;
@@ -97,7 +99,7 @@ const PrefixSecurityEnum = Object.freeze({
 // * all capturing groups converted to non-capturing -- "(?:)"
 // * support for IPv6 Scoped Literal ("%eth1") removed
 // * lowercase hexadecimal only
-var IP_REGEX_LOWERCASE =/(?:^(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}$)|(?:^(?:(?:[a-f\d]{1,4}:){7}(?:[a-f\d]{1,4}|:)|(?:[a-f\d]{1,4}:){6}(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|:[a-f\d]{1,4}|:)|(?:[a-f\d]{1,4}:){5}(?::(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,2}|:)|(?:[a-f\d]{1,4}:){4}(?:(?::[a-f\d]{1,4}){0,1}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,3}|:)|(?:[a-f\d]{1,4}:){3}(?:(?::[a-f\d]{1,4}){0,2}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,4}|:)|(?:[a-f\d]{1,4}:){2}(?:(?::[a-f\d]{1,4}){0,3}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,5}|:)|(?:[a-f\d]{1,4}:){1}(?:(?::[a-f\d]{1,4}){0,4}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,6}|:)|(?::(?:(?::[a-f\d]{1,4}){0,5}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,7}|:)))$)/;
+const IP_REGEX_LOWERCASE =/(?:^(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}$)|(?:^(?:(?:[a-f\d]{1,4}:){7}(?:[a-f\d]{1,4}|:)|(?:[a-f\d]{1,4}:){6}(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|:[a-f\d]{1,4}|:)|(?:[a-f\d]{1,4}:){5}(?::(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,2}|:)|(?:[a-f\d]{1,4}:){4}(?:(?::[a-f\d]{1,4}){0,1}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,3}|:)|(?:[a-f\d]{1,4}:){3}(?:(?::[a-f\d]{1,4}){0,2}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,4}|:)|(?:[a-f\d]{1,4}:){2}(?:(?::[a-f\d]{1,4}){0,3}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,5}|:)|(?:[a-f\d]{1,4}:){1}(?:(?::[a-f\d]{1,4}){0,4}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,6}|:)|(?::(?:(?::[a-f\d]{1,4}){0,5}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-f\d]{1,4}){1,7}|:)))$)/;
 
 /*
  * Parses a Natural number (i.e., non-negative integer) with either the
@@ -301,6 +303,7 @@ function parseDate(str) {
 }
 
 function formatDate(date) {
+  validators.validate(validators.isDate(date), date);
   return date.toUTCString();
 }
 
@@ -403,6 +406,7 @@ function defaultPath(path) {
 }
 
 function trimTerminator(str) {
+  if (validators.isEmptyString(str)) return str;
   for (let t = 0; t < TERMINATORS.length; t++) {
     const terminatorIdx = str.indexOf(TERMINATORS[t]);
     if (terminatorIdx !== -1) {
@@ -415,6 +419,7 @@ function trimTerminator(str) {
 
 function parseCookiePair(cookiePair, looseMode) {
   cookiePair = trimTerminator(cookiePair);
+  validators.validate(validators.isString(cookiePair), cookiePair);
 
   let firstEq = cookiePair.indexOf("=");
   if (looseMode) {
@@ -616,6 +621,7 @@ function parse(str, options) {
  * @returns boolean
  */
 function isSecurePrefixConditionMet(cookie) {
+  validators.validate(validators.isObject(cookie), cookie);
   return !cookie.key.startsWith("__Secure-") || cookie.secure;
 }
 
@@ -631,6 +637,7 @@ function isSecurePrefixConditionMet(cookie) {
  * @returns boolean
  */
 function isHostPrefixConditionMet(cookie) {
+  validators.validate(validators.isObject(cookie));
   return (
     !cookie.key.startsWith("__Host-") ||
     (cookie.secure &&
@@ -698,6 +705,8 @@ function fromJSON(str) {
  */
 
 function cookieCompare(a, b) {
+  validators.validate(validators.isObject(a), a);
+  validators.validate(validators.isObject(b), b);
   let cmp = 0;
 
   // descending for length: b CMP a
@@ -725,6 +734,7 @@ function cookieCompare(a, b) {
 // Gives the permutation of all possible pathMatch()es of a given path. The
 // array is in longest-to-shortest order.  Handy for indexing.
 function permutePath(path) {
+  validators.validate(validators.isString(path));
   if (path === "/") {
     return ["/"];
   }
@@ -1060,6 +1070,7 @@ class CookieJar {
     if (typeof options === "boolean") {
       options = { rejectPublicSuffixes: options };
     }
+    validators.validate(validators.isObject(options), options);
     this.rejectPublicSuffixes = options.rejectPublicSuffixes;
     this.enableLooseMode = !!options.looseMode;
     this.allowSpecialUseDomain = !!options.allowSpecialUseDomain;
@@ -1076,12 +1087,20 @@ class CookieJar {
   }
 
   setCookie(cookie, url, options, cb) {
+    validators.validate(validators.isNonEmptyString(url), cb, options);   
     let err;
+
+    if (validators.isFunction(url)) {
+      cb = url;
+      return cb(new Error("No URL was specified"));
+    }
+
     const context = getCookieContext(url);
-    if (typeof options === "function") {
+    if (validators.isFunction(options)) {
       cb = options;
       options = {};
     }
+    validators.validate(validators.isFunction(cb), cb);
 
     const host = canonicalDomain(context.hostname);
     const loose = options.loose || this.enableLooseMode;
@@ -1249,11 +1268,14 @@ class CookieJar {
 
   // RFC6365 S5.4
   getCookies(url, options, cb) {
+    validators.validate(validators.isNonEmptyString(url), cb, url);
     const context = getCookieContext(url);
-    if (typeof options === "function") {
+    if (validators.isFunction(options)) {
       cb = options;
       options = {};
     }
+    validators.validate(validators.isObject(options), cb, options);
+    validators.validate(validators.isFunction(cb), cb);
 
     const host = canonicalDomain(context.hostname);
     const path = context.pathname || "/";
@@ -1369,6 +1391,7 @@ class CookieJar {
 
   getCookieString(...args) {
     const cb = args.pop();
+    validators.validate(validators.isFunction(cb), cb);
     const next = function(err, cookies) {
       if (err) {
         cb(err);
@@ -1388,6 +1411,7 @@ class CookieJar {
 
   getSetCookieStrings(...args) {
     const cb = args.pop();
+    validators.validate(validators.isFunction(cb), cb);
     const next = function(err, cookies) {
       if (err) {
         cb(err);
@@ -1405,8 +1429,9 @@ class CookieJar {
   }
 
   serialize(cb) {
+    validators.validate(validators.isFunction(cb), cb);
     let type = this.store.constructor.name;
-    if (type === "Object") {
+    if (validators.isObject(type)) {
       type = null;
     }
 
@@ -1524,6 +1549,7 @@ class CookieJar {
   }
 
   removeAllCookies(cb) {
+    validators.validate(validators.isFunction(cb), cb);
     const store = this.store;
 
     // Check that the store implements its own removeAllCookies(). The default
@@ -1577,6 +1603,7 @@ class CookieJar {
       cb = store;
       store = null;
     }
+    validators.validate(validators.isFunction(cb), cb);
 
     let serialized;
     if (typeof strOrObj === "string") {
@@ -1669,3 +1696,4 @@ exports.permuteDomain = require("./permuteDomain").permuteDomain;
 exports.permutePath = permutePath;
 exports.canonicalDomain = canonicalDomain;
 exports.PrefixSecurityEnum = PrefixSecurityEnum;
+exports.ParameterError = validators.ParameterError;

lib/validators.js

@@ -0,0 +1,95 @@
+/* ************************************************************************************
+Extracted from check-types.js
+https://gitlab.com/philbooth/check-types.js
+
+MIT License
+
+Copyright (c) 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 Phil Booth
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+
+************************************************************************************ */
+"use strict";
+
+/* Validation functions copied from check-types package - https://www.npmjs.com/package/check-types */
+function isFunction(data) {
+  return typeof data === 'function';
+}
+
+function isNonEmptyString(data) {
+  return isString(data) && data !== '';
+}
+
+function isDate(data) {
+  return isInstanceStrict(data, Date) && isInteger(data.getTime());
+}
+
+function isEmptyString(data) {
+  return data === '';
+}
+
+function isString(data) {
+  return typeof data === 'string';
+}
+
+function isObject(data) {
+  return toString.call(data) === '[object Object]';
+}
+function isInstanceStrict(data, prototype) {
+  try {
+    return data instanceof prototype;
+  } catch (error) {
+    return false;
+  }
+}
+
+function isInteger(data) {
+  return typeof data === 'number' && data % 1 === 0;
+}
+/* End validation functions */
+
+function validate(bool, cb, options) {
+  if (!isFunction(cb)) {
+    options = cb;
+    cb = null;
+  }
+  if (!isObject(options)) options = { Error: "Failed Check" };
+  if (!bool) {
+    if (cb) {
+      cb(new ParameterError(options));
+    } else {
+      throw new ParameterError(options);
+    }
+  }
+}
+
+class ParameterError extends Error {
+  constructor(...params) {
+    super(...params);
+  }
+};
+
+exports.ParameterError = ParameterError;
+exports.isFunction = isFunction;
+exports.isNonEmptyString = isNonEmptyString;
+exports.isDate = isDate;
+exports.isEmptyString = isEmptyString;
+exports.isString = isString;
+exports.isObject = isObject;
+exports.validate = validate;

test/cookie_jar_test.js

@@ -669,4 +669,21 @@ vows
       }
     }
   })
+  .addBatch({
+    "Issue #145 - Missing parameter validation on setCookie function causes TypeError": {
+      "with missing parameters": {
+        topic: function() {
+          const jar = new tough.CookieJar();
+          jar.setCookie(
+            new String("x=y; Domain=example.com; Path=/"),
+            this.callback
+          );
+        },
+        "results in a error being returned because of missing parameters": function(err, cookies) {
+          assert(err != null);
+          assert(err instanceof tough.ParameterError);
+        }
+      }
+    }
+  })
   .export(module);