From 28f0808127074d73af05e7676cfd0e3591e2e5ce Mon Sep 17 00:00:00 2001 From: Jeremy Stashewsky Date: Mon, 24 Sep 2018 11:08:55 -0700 Subject: [PATCH] Only call removeAllCookies if actually implemented --- lib/cookie.js | 54 +++++++++++++++------------- test/api_test.js | 93 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 122 insertions(+), 25 deletions(-) diff --git a/lib/cookie.js b/lib/cookie.js index 81e6d14a..7e89eded 100644 --- a/lib/cookie.js +++ b/lib/cookie.js @@ -1388,38 +1388,44 @@ CookieJar.prototype.clone = function(newStore, cb) { CAN_BE_SYNC.push('removeAllCookies'); CookieJar.prototype.removeAllCookies = function(cb) { var store = this.store; - if (store.removeAllCookies instanceof Function) { - store.removeAllCookies(cb); - } else { - store.getAllCookies(function(err, cookies) { - if (err) { - return cb(err); - } - if (cookies.length === 0) { - return cb(null); - } + // Check that the store implements its own removeAllCookies(). The default + // implementation in Store will immediately call the callback with a "not + // implemented" Error. + if (store.removeAllCookies instanceof Function && + store.removeAllCookies !== Store.prototype.removeAllCookies) + { + return store.removeAllCookies(cb); + } - var completedCount = 0; - var removeErrors = []; + store.getAllCookies(function(err, cookies) { + if (err) { + return cb(err); + } - function removeCookieCb(removeErr) { - if (removeErr) { - removeErrors.push(removeErr); - } + if (cookies.length === 0) { + return cb(null); + } - completedCount++; + var completedCount = 0; + var removeErrors = []; - if (completedCount === cookies.length) { - return removeErrors.length > 0 ? cb(removeErrors[0]) : cb(null); - } + function removeCookieCb(removeErr) { + if (removeErr) { + removeErrors.push(removeErr); } - cookies.forEach(function(cookie) { - store.removeCookie(cookie.domain, cookie.path, cookie.key, removeCookieCb); - }); + completedCount++; + + if (completedCount === cookies.length) { + return removeErrors.length > 0 ? cb(removeErrors[0]) : cb(null); + } + } + + cookies.forEach(function(cookie) { + store.removeCookie(cookie.domain, cookie.path, cookie.key, removeCookieCb); }); - } + }); }; CookieJar.prototype._cloneSync = syncWrap('clone'); diff --git a/test/api_test.js b/test/api_test.js index 8ef5d1ef..1a231515 100644 --- a/test/api_test.js +++ b/test/api_test.js @@ -29,13 +29,15 @@ * POSSIBILITY OF SUCH DAMAGE. */ 'use strict'; +var util = require('util'); var vows = require('vows'); var assert = require('assert'); var async = require('async'); var tough = require('../lib/cookie'); var Cookie = tough.Cookie; var CookieJar = tough.CookieJar; - +var Store = tough.Store; +var MemoryCookieStore = tough.MemoryCookieStore; var atNow = Date.now(); @@ -239,6 +241,95 @@ vows } } }) + .addBatch({ + "Remove All cookies": { + "With a store that doesn't implement removeAllCookies": { + topic: function () { + var stats = { put: 0, getAll: 0, remove: 0 }; + function StoreWithoutRemoveAll() { + Store.call(this); + this.cookies = []; + } + util.inherits(StoreWithoutRemoveAll, Store); + StoreWithoutRemoveAll.prototype.synchronous = true; + StoreWithoutRemoveAll.prototype.cookies = []; + StoreWithoutRemoveAll.prototype.findCookie = function(domain, path, key, cb) { + return cb(null,null); + }; + StoreWithoutRemoveAll.prototype.findCookies = function(domain, path, key, cb) { + return cb(null,[]); + }; + StoreWithoutRemoveAll.prototype.putCookie = function(cookie, cb) { + stats.put++; + this.cookies.push(cookie); + return cb(null); + }; + StoreWithoutRemoveAll.prototype.getAllCookies = function(cb) { + stats.getAll++; + return cb(null, this.cookies.slice()); + }; + StoreWithoutRemoveAll.prototype.removeCookie = function(domain, path, key, cb) { + stats.remove++; + return cb(null, null); + }; + + assert(StoreWithoutRemoveAll.prototype.removeAllCookies === Store.prototype.removeAllCookies); + + var jar = new CookieJar(new StoreWithoutRemoveAll()); + jar.setCookieSync("a=b", 'http://example.com/index.html'); + jar.setCookieSync("c=d", 'http://example.org/index.html'); + var cb = this.callback; + jar.removeAllCookies(function(err) { + return cb(err,stats); + }); + }, + "Cookies are removed one-by-one": function (err, stats) { + assert.equal(err, null); + assert.equal(stats.put, 2); + assert.equal(stats.getAll, 1); + assert.equal(stats.remove, 2); + } + }, + "With a store that does implement removeAllCookies": { + topic: function () { + var stats = { getAll: 0, remove: 0, removeAll: 0 }; + function MemoryStoreExtension() { + MemoryCookieStore.call(this); + this.cookies = []; + } + util.inherits(MemoryStoreExtension, MemoryCookieStore); + MemoryStoreExtension.prototype.getAllCookies = function(cb) { + stats.getAll++; + MemoryCookieStore.prototype.getAllCookies.call(this, cb); + }; + MemoryStoreExtension.prototype.removeCookie = function(domain, path, key, cb) { + stats.remove++; + MemoryCookieStore.prototype.removeCookie.call(this, domain, path, key, cb); + }; + MemoryStoreExtension.prototype.removeAllCookies = function(cb) { + stats.removeAll++; + MemoryCookieStore.prototype.removeAllCookies.call(this, cb); + }; + + var jar = new CookieJar(new MemoryStoreExtension()); + jar.setCookieSync("a=b", 'http://example.com/index.html'); + jar.setCookieSync("c=d", 'http://example.org/index.html'); + var cb = this.callback; + this.jar = jar; + jar.removeAllCookies(function(err) { + return cb(err,stats); + }); + }, + "Cookies are removed as batch": function (err, stats) { + assert.equal(err, null); + assert.equal(stats.getAll, 0); + assert.equal(stats.remove, 0); + assert.equal(stats.removeAll, 1); + assert.deepEqual(this.jar.store.idx, {}); + } + } + } + }) .addBatch({ "Synchronous CookieJar": { "setCookieSync": {