Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Returned null in case of empty cookie value #196

Merged
merged 7 commits into from Feb 16, 2021
Merged

Returned null in case of empty cookie value #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c3c044a
Returned null in case of empty cookie value
Jun 17, 2020
6e4f05a
Merge branch 'master' into checkForEmptyCookie
vsin12 Aug 19, 2020
cb6cdf9
Update lib/cookie.js
vsin12 Sep 10, 2020
8b753a4
Added in a test and fixed a few issues with the if-statement based on…
medelibero-sfdc Nov 24, 2020
8ab92a5
Merge branch 'master' into checkForEmptyCookie
vsin12 Nov 24, 2020
dc95a90
Changes based on review and fixing the if-statement so a test does no…
medelibero-sfdc Nov 24, 2020
7e2c51b
Merge branch 'checkForEmptyCookie' of https://github.com/vsin12/tough…
medelibero-sfdc Nov 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/cookie.js
View file
Open in desktop
Expand Up @@ -1100,8 +1100,13 @@ class CookieJar {
cb = options;
options = {};
}

validators.validate(validators.isFunction(cb), cb);
vsin12 marked this conversation as resolved.
Show resolved Hide resolved

if(!validators.isNonEmptyString(cookie) && !validators.isObject(cookie) && ( cookie instanceof String && cookie.length == 0)) {
return cb(null);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just looked at this in context, and I think this is fine. Will discuss today.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comment below; I think we may want an Error here.

const host = canonicalDomain(context.hostname);
const loose = options.loose || this.enableLooseMode;

Expand Down
17 changes: 17 additions & 0 deletions test/cookie_jar_test.js
View file
Open in desktop
Expand Up @@ -699,4 +699,21 @@ vows
}
}
})
.addBatch({
"Issue #197 - CookieJar().setCookie crashes when empty cookie is passed": {
"with missing parameters": {
topic: function() {
const jar = new tough.CookieJar();
jar.setCookie(
'',
"https://google.com",
this.callback
);
},
"results in a error being returned because of missing parameters": function(err, cookies) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't actually return an error, we are invoking the callback with a Null here. Should we invoke the CB with an Error()?

assert(cookies == undefined);
}
}
}
})
.export(module);