Skip to content

Commit 8c4fab0

Browse files
addaleaxevanlucas
authored andcommitted
stream: fix Writable subclass instanceof checks
2a4b068 introduced a regression in where checking `instanceof` would fail for `Writable` subclasses inside the subclass constructor, i.e. before `Writable()` was called. Also, calling `null instanceof Writable` or `undefined instanceof Writable` would fail due to accessing the `_writableState` property of the target object. This fixes these problems. PR-URL: #9088 Ref: #8834 (comment) Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
1 parent 7171bd6 commit 8c4fab0

2 files changed

Lines changed: 24 additions & 3 deletions

File tree

‎lib/_stream_writable.js‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,10 @@ if (typeof Symbol === 'function' && Symbol.hasInstance) {
141141
realHasInstance=Function.prototype[Symbol.hasInstance];
142142
Object.defineProperty(Writable,Symbol.hasInstance,{
143143
value: function(object){
144-
// Trying to move the `realHasInstance` from the Writable constructor
145-
// to here will break the Node.js LazyTransform implementation.
146-
returnobject._writableStateinstanceofWritableState;
144+
if(realHasInstance.call(this,object))
145+
returntrue;
146+
147+
returnobject&&object._writableStateinstanceofWritableState;
147148
}
148149
});
149150
}else{
@@ -156,6 +157,10 @@ function Writable(options) {
156157
// Writable ctor is applied to Duplexes, too.
157158
// `realHasInstance` is necessary because using plain `instanceof`
158159
// would return false, as no `_writableState` property is attached.
160+
161+
// Trying to use the custom `instanceof` for Writable here will also break the
162+
// Node.js LazyTransform implementation, which has a non-trivial getter for
163+
// `_writableState` that would lead to infinite recursion.
159164
if(!(realHasInstance.call(Writable,this))&&
160165
!(thisinstanceofStream.Duplex)){
161166
returnnewWritable(options);

‎test/parallel/test-stream-inheritance.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,19 @@ assert.ok(!(readable instanceof Transform));
2727
assert.ok(!(writableinstanceofTransform));
2828
assert.ok(!(duplexinstanceofTransform));
2929
assert.ok(transforminstanceofTransform);
30+
31+
assert.ok(!(nullinstanceofWritable));
32+
assert.ok(!(undefinedinstanceofWritable));
33+
34+
// Simple inheritance check for `Writable` works fine in a subclass constructor.
35+
functionCustomWritable(){
36+
assert.ok(thisinstanceofWritable,'inherits from Writable');
37+
assert.ok(thisinstanceofCustomWritable,'inherits from CustomWritable');
38+
}
39+
40+
Object.setPrototypeOf(CustomWritable,Writable);
41+
Object.setPrototypeOf(CustomWritable.prototype,Writable.prototype);
42+
43+
newCustomWritable();
44+
45+
assert.throws(CustomWritable,/AssertionError:inheritsfromWritable/);

0 commit comments

Comments
 (0)