diff --git a/CHANGELOG.md b/CHANGELOG.md index 5328f244..8fb1426f 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/cookie.js b/lib/cookie.js index a042893e..f7c491e7 100644 --- a/lib/cookie.js +++ b/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; diff --git a/lib/validators.js b/lib/validators.js new file mode 100644 index 00000000..fd46034c --- /dev/null +++ b/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; \ No newline at end of file diff --git a/test/cookie_jar_test.js b/test/cookie_jar_test.js index 9a5d3821..8b8af0a5 100644 --- a/test/cookie_jar_test.js +++ b/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);