Skip to content

Require cookie value to be string (unset omitted or null)#114

Open
zachasme wants to merge 2 commits intopillarjs:masterfrom
zachasme:feature-require-value-to-be-string
Open

Require cookie value to be string (unset omitted or null)#114
zachasme wants to merge 2 commits intopillarjs:masterfrom
zachasme:feature-require-value-to-be-string

Conversation

@zachasme
Copy link
Copy Markdown

@zachasme zachasme commented Sep 11, 2019

Resolves #101.

This enforces string input as suggested by @dougwilson.

Two considerations:

  • Is input a string if wrapped in String constructor? I suggest no, as do Node. That is also what I went with in the PR.
  • When is input omitted? Logically undefined. But there are tests for expiring cookies using null. What about false? I feel undefined and null should cover it (an explicit Cookies.remove would avoid ambiguity, see Add cookies.remove(name[, options]) method #98).

@dougwilson dougwilson added the pr label Oct 9, 2019
@dougwilson
Copy link
Copy Markdown
Contributor

Is input a string if wrapped in String constructor? I suggest no, as do Node. That is also what I went with in the PR.

Yea, I have just done the same type of validations that Node.js APIs use, which is just typeof, only considering primitive types and not the boxed types.

When is input omitted? Logically undefined. But there are tests for expiring cookies using null. What about false? I feel undefined and null should cover it

From a JavaScript POV, it is just undefined. The two tests passing in null, are testing that any falsy value works, so that would mean the API contract there does indeed false and 0.

@zachasme
Copy link
Copy Markdown
Author

Would you prefer only considering input as omitted on undefined then? If so I can quickly update the PR accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cookie set to '' when pass the value as 0

2 participants