Skip to content

Commit

Permalink
zlib: fix assert fail for bad write in object mode
Browse files Browse the repository at this point in the history
add4b0a introduced a regression from Node 8 to Node 9 by removing the
`ArrayBuffer.isView(chunk)` check in `Zlib.prototype._transform` without
properly forcing `opts.objectMode` to `false` in the constructor because
the change to `opts` occurs after `opts` has been passed to the
`Transform` constructor.  This commit fixes the issue by moving the call
to `Transform` after the changes to `opts`.

The regression can be demonstrated by running

    node -e 'require("zlib").Gunzip({objectMode: true}).write({})'

On Node 8 this correctly throws a `TypeError`:

    events.js:183
          throw er; // Unhandled 'error' event
          ^

    TypeError: invalid input
        at Gunzip._transform (zlib.js:367:15)
        at Gunzip.Transform._read (_stream_transform.js:186:10)
        at Gunzip.Transform._write (_stream_transform.js:174:12)
        at doWrite (_stream_writable.js:387:12)
        at writeOrBuffer (_stream_writable.js:373:5)
        at Gunzip.Writable.write (_stream_writable.js:290:11)
        at [eval]:1:44
        at ContextifyScript.Script.runInThisContext (vm.js:50:33)
        at Object.runInThisContext (vm.js:139:38)
        at Object.<anonymous> ([eval]-wrapper:6:22)

On Node 9 this causes an assertion failure:

    node[21732]: ../src/node_zlib.cc:179:static void node::{anonymous}::ZCtx::Write(const v8::FunctionCallbackInfo<v8::Value>&) [with bool async = true]: Assertion `Buffer::HasInstance(args[1])' failed.
     1: node::Abort() [node]
     2: node::Assert(char const* const (*) [4]) [node]
     3: 0x1250916 [node]
     4: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [node]
     5: 0xb7547c [node]
     6: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [node]
     7: 0x20c44b8842fd
    Aborted

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
PR-URL: #16960
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
kevinoid authored and MylesBorins committed Dec 11, 2017
1 parent 9a4abe4 commit 45ca714
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/zlib.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ function flushCallback(level, strategy, callback) {
// true or false if there is anything in the queue when
// you call the .write() method.
function Zlib(opts, mode) {
Transform.call(this, opts);
var chunkSize = Z_DEFAULT_CHUNK;
var flush = Z_NO_FLUSH;
var finishFlush = Z_FINISH;
Expand Down Expand Up @@ -259,6 +258,7 @@ function Zlib(opts, mode) {
opts.writableObjectMode = false;
}
}
Transform.call(this, opts);
this.bytesRead = 0;
this._handle = new binding.Zlib(mode);
this._handle.jsref = this; // Used by processCallback() and zlibOnError()
Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-zlib-object-write.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

require('../common');
const assert = require('assert');
const { Gunzip } = require('zlib');

const gunzip = new Gunzip({ objectMode: true });
assert.throws(
() => gunzip.write({}),
TypeError
);

0 comments on commit 45ca714

Please sign in to comment.