Skip to content

Commit

Permalink
zlib: improve zlib errors
Browse files Browse the repository at this point in the history 
- Use assert to check mode in the Zlib constructor since it should
  only be passed by us.
- Introduce checkRangesOrGetDefault() and checkFiniteNumber()
  to simplify type and range checking for numeric arguments
- Instead of `ERR_INVALID_OPT_VALUE`, throw `ERR_OUT_OF_RANGE` and
  `ERR_INVALID_ARG_TYPE` with descriptions of the expected ranges
  or types to make the errors more user-friendly.
- Add message tests for the changed errors

PR-URL: #18675
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
@joyeecheung
joyeecheung committed Feb 24, 2018
1 parent 15e41a9 commit da886d9
Show file tree
Hide file tree
Showing 4 changed files with 317 additions and 130 deletions.
158 changes: 78 additions & 80 deletions lib/zlib.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,52 @@ function flushCallback(level, strategy, callback) {
}
}

// 1. Returns false for undefined and NaN
// 2. Returns true for finite numbers
// 3. Throws ERR_INVALID_ARG_TYPE for non-numbers
// 4. Throws ERR_OUT_OF_RANGE for infinite numbers
function checkFiniteNumber(number, name) {
// Common case
if (number === undefined || Number.isNaN(number)) {
return false;
}

if (Number.isFinite(number)) {
return true; // is a valid number
}

// Other non-numbers
if (typeof number !== 'number') {
const err = new errors.TypeError('ERR_INVALID_ARG_TYPE', name,
'number', number);
Error.captureStackTrace(err, checkFiniteNumber);
throw err;
}

// Infinite numbers
const err = new errors.RangeError('ERR_OUT_OF_RANGE', name,
'a finite number', number);
Error.captureStackTrace(err, checkFiniteNumber);
throw err;
}

// 1. Returns def for undefined and NaN
// 2. Returns number for finite numbers >= lower and <= upper
// 3. Throws ERR_INVALID_ARG_TYPE for non-numbers
// 4. Throws ERR_OUT_OF_RANGE for infinite numbers or numbers > upper or < lower
function checkRangesOrGetDefault(number, name, lower, upper, def) {
if (!checkFiniteNumber(number, name, lower, upper)) {
return def;
}
if (number < lower || number > upper) {
const err = new errors.RangeError('ERR_OUT_OF_RANGE', name,
`>= ${lower} and <= ${upper}`, number);
Error.captureStackTrace(err, checkRangesOrGetDefault);
throw err;
}
return number;
}

// the Zlib class they all inherit from
// This thing manages the queue of requests, and returns
// true or false if there is anything in the queue when
Expand All @@ -170,95 +216,53 @@ function Zlib(opts, mode) {
var strategy = Z_DEFAULT_STRATEGY;
var dictionary;

if (typeof mode !== 'number')
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'mode', 'number');
if (mode < DEFLATE || mode > UNZIP)
throw new errors.RangeError('ERR_OUT_OF_RANGE', 'mode');
// The Zlib class is not exported to user land, the mode should only be
// passed in by us.
assert(typeof mode === 'number');
assert(mode >= DEFLATE && mode <= UNZIP);

if (opts) {
chunkSize = opts.chunkSize;
if (chunkSize !== undefined && !Number.isNaN(chunkSize)) {
if (chunkSize < Z_MIN_CHUNK || !Number.isFinite(chunkSize))
throw new errors.RangeError('ERR_INVALID_OPT_VALUE',
'chunkSize',
chunkSize);
} else {
if (!checkFiniteNumber(chunkSize, 'options.chunkSize')) {
chunkSize = Z_DEFAULT_CHUNK;
} else if (chunkSize < Z_MIN_CHUNK) {
throw new errors.RangeError('ERR_OUT_OF_RANGE', 'options.chunkSize',
`>= ${Z_MIN_CHUNK}`, chunkSize);
}

flush = opts.flush;
if (flush !== undefined && !Number.isNaN(flush)) {
if (flush < Z_NO_FLUSH || flush > Z_BLOCK || !Number.isFinite(flush))
throw new errors.RangeError('ERR_INVALID_OPT_VALUE', 'flush', flush);
} else {
flush = Z_NO_FLUSH;
}
flush = checkRangesOrGetDefault(
opts.flush, 'options.flush',
Z_NO_FLUSH, Z_BLOCK, Z_NO_FLUSH);

finishFlush = opts.finishFlush;
if (finishFlush !== undefined && !Number.isNaN(finishFlush)) {
if (finishFlush < Z_NO_FLUSH || finishFlush > Z_BLOCK ||
!Number.isFinite(finishFlush)) {
throw new errors.RangeError('ERR_INVALID_OPT_VALUE',
'finishFlush',
finishFlush);
}
} else {
finishFlush = Z_FINISH;
}
finishFlush = checkRangesOrGetDefault(
opts.finishFlush, 'options.finishFlush',
Z_NO_FLUSH, Z_BLOCK, Z_FINISH);

windowBits = opts.windowBits;
if (windowBits !== undefined && !Number.isNaN(windowBits)) {
if (windowBits < Z_MIN_WINDOWBITS || windowBits > Z_MAX_WINDOWBITS ||
!Number.isFinite(windowBits)) {
throw new errors.RangeError('ERR_INVALID_OPT_VALUE',
'windowBits',
windowBits);
}
} else {
windowBits = Z_DEFAULT_WINDOWBITS;
}
windowBits = checkRangesOrGetDefault(
opts.windowBits, 'options.windowBits',
Z_MIN_WINDOWBITS, Z_MAX_WINDOWBITS, Z_DEFAULT_WINDOWBITS);

level = opts.level;
if (level !== undefined && !Number.isNaN(level)) {
if (level < Z_MIN_LEVEL || level > Z_MAX_LEVEL ||
!Number.isFinite(level)) {
throw new errors.RangeError('ERR_INVALID_OPT_VALUE',
'level', level);
}
} else {
level = Z_DEFAULT_COMPRESSION;
}
level = checkRangesOrGetDefault(
opts.level, 'options.level',
Z_MIN_LEVEL, Z_MAX_LEVEL, Z_DEFAULT_COMPRESSION);

memLevel = opts.memLevel;
if (memLevel !== undefined && !Number.isNaN(memLevel)) {
if (memLevel < Z_MIN_MEMLEVEL || memLevel > Z_MAX_MEMLEVEL ||
!Number.isFinite(memLevel)) {
throw new errors.RangeError('ERR_INVALID_OPT_VALUE',
'memLevel', memLevel);
}
} else {
memLevel = Z_DEFAULT_MEMLEVEL;
}
memLevel = checkRangesOrGetDefault(
opts.memLevel, 'options.memLevel',
Z_MIN_MEMLEVEL, Z_MAX_MEMLEVEL, Z_DEFAULT_MEMLEVEL);

strategy = opts.strategy;
if (strategy !== undefined && !Number.isNaN(strategy)) {
if (strategy < Z_DEFAULT_STRATEGY || strategy > Z_FIXED ||
!Number.isFinite(strategy)) {
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
'strategy', strategy);
}
} else {
strategy = Z_DEFAULT_STRATEGY;
}
strategy = checkRangesOrGetDefault(
opts.strategy, 'options.strategy',
Z_DEFAULT_STRATEGY, Z_FIXED, Z_DEFAULT_STRATEGY);

dictionary = opts.dictionary;
if (dictionary !== undefined && !isArrayBufferView(dictionary)) {
if (isAnyArrayBuffer(dictionary)) {
dictionary = Buffer.from(dictionary);
} else {
throw new errors.TypeError('ERR_INVALID_OPT_VALUE',
'dictionary',
dictionary);
throw new errors.TypeError(
'ERR_INVALID_ARG_TYPE', 'options.dictionary',
['Buffer', 'TypedArray', 'DataView', 'ArrayBuffer'],
dictionary);
}
}

Expand Down Expand Up @@ -310,14 +314,8 @@ Object.defineProperty(Zlib.prototype, '_closed', {
});

Zlib.prototype.params = function params(level, strategy, callback) {
if (level < Z_MIN_LEVEL || level > Z_MAX_LEVEL)
throw new errors.RangeError('ERR_INVALID_ARG_VALUE', 'level', level);

if (strategy !== undefined &&
(strategy < Z_DEFAULT_STRATEGY || strategy > Z_FIXED ||
!Number.isFinite(strategy))) {
throw new errors.TypeError('ERR_INVALID_ARG_VALUE', 'strategy', strategy);
}
checkRangesOrGetDefault(level, 'level', Z_MIN_LEVEL, Z_MAX_LEVEL);
checkRangesOrGetDefault(strategy, 'strategy', Z_DEFAULT_STRATEGY, Z_FIXED);

if (this._level !== level || this._strategy !== strategy) {
this.flush(Z_SYNC_FLUSH,
Expand Down
Loading

0 comments on commit da886d9

Please sign in to comment.