Skip to content
New issue

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

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

Already on GitHub? # to your account

Cookie set to '' when pass the value as 0 #101

Open
mrbone opened this issue Aug 22, 2018 · 3 comments · May be fixed by #114
Open

Cookie set to '' when pass the value as 0 #101

mrbone opened this issue Aug 22, 2018 · 3 comments · May be fixed by #114

Comments

@mrbone
Copy link

mrbone commented Aug 22, 2018

When I want to record the user visit time in the header, I found the 0 value cannot set to cookie only if we convert it to string type, but other number type value can be saved.
I create one test to reproduce the issue:

it('when set value to 0', () => {
      var cookie = new cookies.Cookie('foo', 0 );
      assert.equal(cookie.toHeader(), 'foo=0; path=/; httponly')
 }) //this will faile

it('when set value to 1', () => {
        var cookie = new cookies.Cookie('foo', 1 );
        assert.equal(cookie.toHeader(), 'foo=1; path=/; httponly')
}) //this will pass

And I check the source code found this issue happens because of the index.js L:131

  this.value = value || ""

This will convert the 0 to empty string, so I am woundering is this a bug or not, if not, I think we should notice the user we should always put he String type value.

@dougwilson
Copy link
Contributor

Yea, I think it's definitely a bug. There are two choices: (a) coerce the value or (b) require it to be a string.

Just from the pros and cons and pitfalls, I think (b) is the better choice.

You're welcome to make a PR with that change, otherwise I'll get to it at some point 👍

@dougwilson dougwilson added this to the 0.8 milestone Sep 10, 2018
@mrbone
Copy link
Author

mrbone commented Oct 19, 2018

Hi, I create a PR to fix this issue, please reivew it. #104

@AjayPoshak
Copy link

Yup, same thing is happening with passing false as cookie value.

@dougwilson dougwilson removed this from the 0.8 milestone Oct 11, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants