From dd928b04fc621a1d38bd8667a47baa9d106248c8 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Fri, 20 Jan 2017 21:54:56 -0800
Subject: [PATCH] zlib: be strict about what strategies are accepted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: https://github.com/nodejs/node/pull/10934
Fixes: https://github.com/nodejs/node/issues/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>
---
 lib/zlib.js                                   | 26 +++++++------------
 .../test-zlib-deflate-constructors.js         |  6 +++++
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/zlib.js b/lib/zlib.js
index 7742e68ad6eb75..b6817749bae35d 100644
--- a/lib/zlib.js
+++ b/lib/zlib.js
@@ -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
@@ -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)) {
@@ -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
   });
@@ -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;
diff --git a/test/parallel/test-zlib-deflate-constructors.js b/test/parallel/test-zlib-deflate-constructors.js
index 2f23dc595ec0b0..3aef067162f048 100644
--- a/test/parallel/test-zlib-deflate-constructors.js
+++ b/test/parallel/test-zlib-deflate-constructors.js
@@ -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'}); },