Skip to content

Commit

Permalink
zlib: be strict about what strategies are accepted
Browse files Browse the repository at this point in the history
Currently, strategy constants are integers but Node.js will accept
string versions of those integers. Users should be using the provided
zlib constants and not hardcoding numbers, strings, or anything else. As
such, Node.js should be strict about accepting only exactly those values
that are in the provided zlib constants.

PR-URL: #10934
Fixes: #10932
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
Trott committed Jan 25, 2017
1 parent 01b90ee commit dd928b0
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 16 deletions.
26 changes: 10 additions & 16 deletions lib/zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,12 @@ function isValidFlushFlag(flag) {
flag === constants.Z_BLOCK;
}

const strategies = [constants.Z_FILTERED,
constants.Z_HUFFMAN_ONLY,
constants.Z_RLE,
constants.Z_FIXED,
constants.Z_DEFAULT_STRATEGY];

// 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 Down Expand Up @@ -326,15 +332,8 @@ function Zlib(opts, mode) {
}
}

if (opts.strategy) {
if (opts.strategy != constants.Z_FILTERED &&
opts.strategy != constants.Z_HUFFMAN_ONLY &&
opts.strategy != constants.Z_RLE &&
opts.strategy != constants.Z_FIXED &&
opts.strategy != constants.Z_DEFAULT_STRATEGY) {
throw new Error('Invalid strategy: ' + opts.strategy);
}
}
if (opts.strategy && !(strategies.includes(opts.strategy)))
throw new Error('Invalid strategy: ' + opts.strategy);

if (opts.dictionary) {
if (!(opts.dictionary instanceof Buffer)) {
Expand Down Expand Up @@ -378,7 +377,7 @@ function Zlib(opts, mode) {
this.once('end', this.close);

Object.defineProperty(this, '_closed', {
get: () => { return !this._handle; },
get: () => !this._handle,
configurable: true,
enumerable: true
});
Expand All @@ -391,13 +390,8 @@ Zlib.prototype.params = function(level, strategy, callback) {
level > constants.Z_MAX_LEVEL) {
throw new RangeError('Invalid compression level: ' + level);
}
if (strategy != constants.Z_FILTERED &&
strategy != constants.Z_HUFFMAN_ONLY &&
strategy != constants.Z_RLE &&
strategy != constants.Z_FIXED &&
strategy != constants.Z_DEFAULT_STRATEGY) {
if (!(strategies.includes(strategy)))
throw new TypeError('Invalid strategy: ' + strategy);
}

if (this._level !== level || this._strategy !== strategy) {
var self = this;
Expand Down
6 changes: 6 additions & 0 deletions test/parallel/test-zlib-deflate-constructors.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ assert.doesNotThrow(
() => { new zlib.Deflate({ strategy: zlib.constants.Z_DEFAULT_STRATEGY}); }
);

// Throws if opt.strategy is the wrong type.
assert.throws(
() => { new zlib.Deflate({strategy: '' + zlib.constants.Z_RLE }); },
/^Error: Invalid strategy: 3$/
);

// Throws if opts.strategy is invalid
assert.throws(
() => { new zlib.Deflate({strategy: 'this is a bogus strategy'}); },
Expand Down

0 comments on commit dd928b0

Please sign in to comment.