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.

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

Missing parameter validation on setCookie function causes TypeError #145

Closed
fabiante opened this issue Feb 5, 2019 · 2 comments
Closed
Assignees
Milestone

Comments

@fabiante
Copy link

fabiante commented Feb 5, 2019

I noticed missing parameter validation in the setCookie function:

tough-cookie/lib/cookie.js

Lines 995 to 1001 in 4350705

CookieJar.prototype.setCookie = function(cookie, url, options, cb) {
var err;
var context = getCookieContext(url);
if (options instanceof Function) {
cb = options;
options = {};
}

There is no parameter validation which may cause confusion when not setting url. If only cookies then the function will assume that the options parameter is an object and tries to access the loose property. This will cause a TypeException.

There should either be parameter validation throwing an Error or some kind of handling.

This was found when investigating this bug: valeriangalliat/fetch-cookie#35

@stash
Copy link
Collaborator

stash commented Mar 11, 2019

Agree that this could have a much better error raised than TypeError. Pull request would be welcome :)

Other methods besides setCookie could benefit from clearer error messages too, for what it's worth.

@awaterma awaterma added major We expect this work to be a major semver change and removed major We expect this work to be a major semver change labels Dec 9, 2019
@awaterma awaterma added this to the 4.x Release milestone Dec 9, 2019
@medelibero-sfdc medelibero-sfdc self-assigned this Apr 22, 2020
@ruoho
Copy link
Contributor

ruoho commented Aug 24, 2020

@fabiante Sorry for the delay, this should be fixed in the latest pull request. Can you verify?

@ruoho ruoho closed this as completed Aug 24, 2020
wjhsf added a commit that referenced this issue Mar 18, 2023
Targeting ES5 causes the "fixes issue #145" test in cookieJar.spec.ts
to fail. Changing the target to the next newest (ES6) requires adding
"moduleResolution" to the tsconfig, which then causes issues with using
`import` in our TypeScript, but `require` in the legacy vows tests. To
support both, we have to bump to a newer version of TypeScript to use
the new "node16" module/moduleResolution. (Note: I didn't jump to the
_latest_ TypeScript to try to support older projects, as well.)
wjhsf added a commit that referenced this issue Apr 17, 2023
* Update config to support ES6 classes.

Targeting ES5 causes the "fixes issue #145" test in cookieJar.spec.ts
to fail. Changing the target to the next newest (ES6) requires adding
"moduleResolution" to the tsconfig, which then causes issues with using
`import` in our TypeScript, but `require` in the legacy vows tests. To
support both, we have to bump to a newer version of TypeScript to use
the new "node16" module/moduleResolution. (Note: I didn't jump to the
_latest_ TypeScript to try to support older projects, as well.)

* Avoid unnecessarily running compiled test files.

* Fix flaky test.

* Avoid hacky reliance on inherited property of global object.

* Avoid needing to use `resolveJsonModule`.

* Remove config that's just the default.

See: https://huafu.github.io/ts-jest/user/config/isolatedModules

* Update tsconfig to target node 16 and only compile ./lib

* Chang `dist/lib` imports to just `dist`.

* add granularity to npm scripts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants