Slight improvements to options object member validation #40
Conversation
| if (opt.expires) pairs.push('Expires=' + opt.expires.toUTCString()); | ||
| if (opt.httpOnly) pairs.push('HttpOnly'); | ||
| if (opt.secure) pairs.push('Secure'); | ||
| typeof opt.domain === 'string' && pairs.push('Domain=' + opt.domain); |
There was a problem hiding this comment.
Please undo these changes so they are actual if statements.
There was a problem hiding this comment.
Modified to be if statements as per your request
| test('encoded method validation', function() { | ||
| assert.throws(cookie.serialize.bind(null, 'cat', ' ', { | ||
| encode: 42 | ||
| }), /number is not a function/); |
There was a problem hiding this comment.
Looks like this is failing on io.js 2.x due to a difference in the v8 error message.
There was a problem hiding this comment.
I saw that earlier and threw in a regex that should cover it in either case. Let me know though.
There was a problem hiding this comment.
Why not just throw our own error, so the message is predictable (and probably more helpful to users)?
There was a problem hiding this comment.
Okay, added code to throw after parsing encode
|
Sorry @arizonatribe that I had neglected this pull request. I have cherry-picked the couple checks that were definitely backwards-compatible so we can use at least something from the pull request (it's not always easy to accept a PR when it does several different things, and some have issues while other changes do not). Please feel free to make a new PR adding any additional checks you would like and we can get it into the 1.0 coming up soon! |
Explicitly checking for options object members expected data types (
typeof 'function'for encode/decode, validDateor date strings forexpiration, validstringvalues fordomainandpath), also using shorthand for "if value is acceptable add to array" logic.