From 84422cd7e6779b87f5fcfe71c55348eb92f9185a Mon Sep 17 00:00:00 2001 From: Jeremy Stashewsky Date: Mon, 7 Jan 2019 11:24:06 -0800 Subject: [PATCH] setCookie arg must be Cookie or string Addresses #132, partially. The error message is clearer, indicating that this method only accepts the afformentioned types. The part of 132 that it doesn't address is the whole "Cookie instance from another version of the `tough-cookie` package" thing, which isn't really solvable unless we somehow expose the Cookie class that "belongs to" this CookieJar class, which is maybe another patch. --- lib/cookie.js | 10 +++++++--- test/cookie_jar_test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/cookie.js b/lib/cookie.js index 32dc0f8d..f60f2a35 100644 --- a/lib/cookie.js +++ b/lib/cookie.js @@ -1007,11 +1007,15 @@ CookieJar.prototype.setCookie = function(cookie, url, options, cb) { } // S5.3 step 1 - if (!(cookie instanceof Cookie)) { + if (typeof(cookie) === 'string' || cookie instanceof String) { cookie = Cookie.parse(cookie, { loose: loose }); + if (!cookie) { + err = new Error("Cookie failed to parse"); + return cb(options.ignoreError ? null : err); + } } - if (!cookie) { - err = new Error("Cookie failed to parse"); + else if (!(cookie instanceof Cookie)) { + err = new Error("First argument to setCookie must be a Cookie object or string"); return cb(options.ignoreError ? null : err); } diff --git a/test/cookie_jar_test.js b/test/cookie_jar_test.js index 67809b93..070fc03c 100644 --- a/test/cookie_jar_test.js +++ b/test/cookie_jar_test.js @@ -541,4 +541,31 @@ vows } } }) + .addBatch({ + "Issue 132 - setCookie": { + "with foreign object": { + topic: function() { + var jar = new CookieJar(); + jar.setCookie({key:"x",value:"y"}, "http://example.com/", this.callback); + }, + "results in an error": function(err, cookie) { + assert(err != null); + assert(!cookie); + assert.equal(err.message, "First argument to setCookie must be a Cookie object or string"); + }, + }, + "with String instance": { + topic: function() { + var jar = new CookieJar(); + jar.setCookie(new String("x=y; Domain=example.com; Path=/"), "http://example.com/", this.callback); + }, + "is fine": function(err, cookie) { + assert(!err); + assert(!!cookie); + assert.instanceOf(cookie, Cookie); + assert.equal(cookie.key, "x"); + }, + } + } + }) .export(module);