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

Feature: Throw TypeErrors in serialize/parse with .code #163

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Feature: Throw TypeErrors in serialize/parse with .code #163

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
a6c0499
Amend error-handling tests to account for error.code
MaoShizhong May 2, 2024
1193c13
Add invalid sameSite arge type test
MaoShizhong May 3, 2024
8ff89bf
Add .code properties to all serialize errors
MaoShizhong May 3, 2024
6abe0fc
Amend tests for parse function with error codes
MaoShizhong May 3, 2024
2e660ac
Add .code property to parse function TypeError
MaoShizhong May 3, 2024
e618fd1
Use callback for throw assertion matching
MaoShizhong May 5, 2024
b745b78
Fix indent lint errors
MaoShizhong May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 43 additions & 15 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ var fieldContentRegExp = /^[\u0009\u0020-\u007e\u0080-\u00ff]+$/;

function parse(str, options) {
if (typeof str !== 'string') {
throw new TypeError('argument str must be a string');
var parseError = new TypeError('argument str must be a string')
parseError.code = 'ERR_INVALID_ARG_TYPE'
throw parseError
}

var obj = {}
Expand Down Expand Up @@ -113,17 +115,23 @@ function serialize(name, val, options) {
var enc = opt.encode || encode;

if (typeof enc !== 'function') {
throw new TypeError('option encode is invalid');
var encodeError = new TypeError('option encode is invalid')
encodeError.code = 'ERR_INVALID_ARG_TYPE'
throw encodeError
}

if (!fieldContentRegExp.test(name)) {
throw new TypeError('argument name is invalid');
var nameError = new TypeError('argument name is invalid')
nameError.code = 'ERR_INVALID_ARG_VALUE'
throw nameError;
}

var value = enc(val);

if (value && !fieldContentRegExp.test(value)) {
throw new TypeError('argument val is invalid');
var returnError = new TypeError('argument val is invalid')
returnError.code = 'ERR_INVALID_RETURN_VALUE'
throw returnError;
}

var str = name + '=' + value;
Expand All @@ -132,33 +140,44 @@ function serialize(name, val, options) {
var maxAge = opt.maxAge - 0;

if (isNaN(maxAge) || !isFinite(maxAge)) {
throw new TypeError('option maxAge is invalid')
var maxAgeError = new TypeError('option maxAge is invalid')
maxAgeError.code = 'ERR_INVALID_ARG_VALUE'
throw maxAgeError;
}

str += '; Max-Age=' + Math.floor(maxAge);
}

if (opt.domain) {
if (!fieldContentRegExp.test(opt.domain)) {
throw new TypeError('option domain is invalid');
var domainError = new TypeError('option domain is invalid')
domainError.code = 'ERR_INVALID_ARG_VALUE'
throw domainError;
}

str += '; Domain=' + opt.domain;
}

if (opt.path) {
if (!fieldContentRegExp.test(opt.path)) {
throw new TypeError('option path is invalid');
var pathError = new TypeError('option path is invalid')
pathError.code = 'ERR_INVALID_ARG_VALUE'
throw pathError;
}

str += '; Path=' + opt.path;
}

if (opt.expires) {
var expires = opt.expires

if (!isDate(expires) || isNaN(expires.valueOf())) {
throw new TypeError('option expires is invalid');
var expiresError = new TypeError('option expires is invalid')
Copy link
Member

@jonchurch jonchurch Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

allocating an error object when it's not needed incurs a small performance hit, due to the stack trace generation. The bigger the stack when this is generated, the bigger the hit. From microseconds to possibly miliseconds. Thats an edgecase, in the real world, but is still possible.

This is one of those instances where following Dont Repeat Yourself isn't worth the perf cost. We save a tiny amount of repetition, but at a performance cost.


if (!isDate(expires)) {
expiresError.code = 'ERR_INVALID_ARG_TYPE'
throw expiresError;
} else if (isNaN(expires.valueOf())) {
expiresError.code = 'ERR_INVALID_ARG_VALUE'
throw expiresError;
}

str += '; Expires=' + expires.toUTCString()
Expand All @@ -177,9 +196,13 @@ function serialize(name, val, options) {
}

if (opt.priority) {
var priority = typeof opt.priority === 'string'
? opt.priority.toLowerCase()
: opt.priority
var priorityError = new TypeError('option priority is invalid')
if (typeof opt.priority !== 'string') {
priorityError.code = 'ERR_INVALID_ARG_TYPE'
throw priorityError;
}

var priority = opt.priority.toLowerCase()

switch (priority) {
case 'low':
Expand All @@ -192,7 +215,8 @@ function serialize(name, val, options) {
str += '; Priority=High'
break
default:
throw new TypeError('option priority is invalid')
priorityError.code = 'ERR_INVALID_ARG_VALUE'
throw priorityError;
}
}

Expand All @@ -214,7 +238,11 @@ function serialize(name, val, options) {
str += '; SameSite=None';
break;
default:
throw new TypeError('option sameSite is invalid');
var sameSiteError = new TypeError('option sameSite is invalid')
sameSiteError.code = typeof opt.sameSite === 'string'
? 'ERR_INVALID_ARG_VALUE'
: 'ERR_INVALID_ARG_TYPE'
throw sameSiteError;
}
}

Expand Down
13 changes: 13 additions & 0 deletions test/compare-error.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
function compareError(errorProperties) {
return function (error) {
if (
error instanceof Error &&
error.message === errorProperties.message &&
error.code === errorProperties.code
) {
return true;
}
};
}

module.exports = compareError;
17 changes: 15 additions & 2 deletions test/parse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@

var assert = require('assert');
var Buffer = require('safe-buffer').Buffer
var compareError = require('./compare-error');

var cookie = require('..');

describe('cookie.parse(str)', function () {
it('should throw with no arguments', function () {
assert.throws(cookie.parse.bind(), /argument str must be a string/)
assert.throws(
cookie.parse.bind(),
compareError({
message: 'argument str must be a string',
code: 'ERR_INVALID_ARG_TYPE',
})
)
})

it('should throw when not a string', function () {
assert.throws(cookie.parse.bind(null, 42), /argument str must be a string/)
assert.throws(
cookie.parse.bind(null, 42),
compareError({
message: 'argument str must be a string',
code: 'ERR_INVALID_ARG_TYPE',
})
)
})

it('should parse cookie string to object', function () {
Expand Down
146 changes: 116 additions & 30 deletions test/serialize.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@

var assert = require('assert');
var Buffer = require('safe-buffer').Buffer
var compareError = require('./compare-error');

var cookie = require('..');

Expand All @@ -18,8 +19,20 @@ describe('cookie.serialize(name, value)', function () {
})

it('should throw for invalid name', function () {
assert.throws(cookie.serialize.bind(cookie, 'foo\n', 'bar'), /argument name is invalid/)
assert.throws(cookie.serialize.bind(cookie, 'foo\u280a', 'bar'), /argument name is invalid/)
assert.throws(
cookie.serialize.bind(cookie, 'foo\n', 'bar'),
compareError({
message: 'argument name is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
assert.throws(
cookie.serialize.bind(cookie, 'foo\u280a', 'bar'),
compareError({
message: 'argument name is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
})
})

Expand All @@ -31,15 +44,25 @@ describe('cookie.serialize(name, value, options)', function () {
})

it('should throw for invalid value', function () {
assert.throws(cookie.serialize.bind(cookie, 'foo', 'bar', { domain: 'example.com\n' }),
/option domain is invalid/)
assert.throws(
cookie.serialize.bind(cookie, 'foo', 'bar', { domain: 'example.com\n' }),
compareError({
message: 'option domain is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
})
})

describe('with "encode" option', function () {
it('should throw on non-function value', function () {
assert.throws(cookie.serialize.bind(cookie, 'foo', 'bar', { encode: 42 }),
/option encode is invalid/)
assert.throws(
cookie.serialize.bind(cookie, 'foo', 'bar', { encode: 42 }),
compareError({
message: 'option encode is invalid',
code: 'ERR_INVALID_ARG_TYPE',
})
)
})

it('should specify alternative value encoder', function () {
Expand All @@ -49,21 +72,37 @@ describe('cookie.serialize(name, value, options)', function () {
})

it('should throw when returned value is invalid', function () {
assert.throws(cookie.serialize.bind(cookie, 'foo', '+ \n', {
encode: function (v) { return v }
}), /argument val is invalid/)
assert.throws(
cookie.serialize.bind(cookie, 'foo', '+ \n', {
encode: function (v) { return v }
}),
compareError({
message: 'argument val is invalid',
code: 'ERR_INVALID_RETURN_VALUE',
})
)
})
})

describe('with "expires" option', function () {
it('should throw on non-Date value', function () {
assert.throws(cookie.serialize.bind(cookie, 'foo', 'bar', { expires: 42 }),
/option expires is invalid/)
assert.throws(
cookie.serialize.bind(cookie, 'foo', 'bar', { expires: 42 }),
compareError({
message: 'option expires is invalid',
code: 'ERR_INVALID_ARG_TYPE',
})
)
})

it('should throw on invalid date', function () {
assert.throws(cookie.serialize.bind(cookie, 'foo', 'bar', { expires: new Date(NaN) }),
/option expires is invalid/)
assert.throws(
cookie.serialize.bind(cookie, 'foo', 'bar', { expires: new Date(NaN) }),
compareError({
message: 'option expires is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
})

it('should set expires to given date', function () {
Expand All @@ -85,15 +124,27 @@ describe('cookie.serialize(name, value, options)', function () {

describe('with "maxAge" option', function () {
it('should throw when not a number', function () {
assert.throws(function () {
cookie.serialize('foo', 'bar', { maxAge: 'buzz' })
}, /option maxAge is invalid/)
assert.throws(
function () {
cookie.serialize('foo', 'bar', { maxAge: 'buzz' })
},
compareError({
message: 'option maxAge is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
})

it('should throw when Infinity', function () {
assert.throws(function () {
cookie.serialize('foo', 'bar', { maxAge: Infinity })
}, /option maxAge is invalid/)
assert.throws(
function () {
cookie.serialize('foo', 'bar', { maxAge: Infinity })
},
compareError({
message: 'option maxAge is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
})

it('should set max-age to value', function () {
Expand Down Expand Up @@ -133,22 +184,39 @@ describe('cookie.serialize(name, value, options)', function () {
})

it('should throw for invalid value', function () {
assert.throws(cookie.serialize.bind(cookie, 'foo', 'bar', { path: '/\n' }),
/option path is invalid/)
assert.throws(
cookie.serialize.bind(cookie, 'foo', 'bar', { path: '/\n' }),
compareError({
message: 'option path is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
})
})

describe('with "priority" option', function () {
it('should throw on invalid priority', function () {
assert.throws(function () {
cookie.serialize('foo', 'bar', { priority: 'foo' })
}, /option priority is invalid/)
assert.throws(
function () {
cookie.serialize('foo', 'bar', { priority: 'foo' })
},
compareError({
message: 'option priority is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
})

it('should throw on non-string', function () {
assert.throws(function () {
cookie.serialize('foo', 'bar', { priority: 42 })
}, /option priority is invalid/)
assert.throws(
function () {
cookie.serialize('foo', 'bar', { priority: 42 })
},
compareError({
message: 'option priority is invalid',
code: 'ERR_INVALID_ARG_TYPE',
})
)
})

it('should set priority low', function () {
Expand All @@ -169,9 +237,27 @@ describe('cookie.serialize(name, value, options)', function () {

describe('with "sameSite" option', function () {
it('should throw on invalid sameSite', function () {
assert.throws(function () {
cookie.serialize('foo', 'bar', { sameSite: 'foo' })
}, /option sameSite is invalid/)
assert.throws(
function () {
cookie.serialize('foo', 'bar', { sameSite: 'foo' })
},
compareError({
message: 'option sameSite is invalid',
code: 'ERR_INVALID_ARG_VALUE',
})
)
})

it('should throw on non-string/non-true', function () {
assert.throws(
function () {
cookie.serialize('foo', 'bar', { sameSite: 42 })
},
compareError({
message: 'option sameSite is invalid',
code: 'ERR_INVALID_ARG_TYPE',
})
)
})

it('should set sameSite strict', function () {
Expand Down
Loading