New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Zlib InflateRaw null reference #6034

Closed

headlessme opened this issue Apr 4, 2016 · 1 comment

Labels

headlessme commented Apr 4, 2016

Version: v5.10.0 (also seen on 4.3.0)
Platform: Darwin Kernel Version 14.5.0
Subsystem: zlib

Zlib references a null object on close after an error:

var zlib = require('zlib');
var inflate = zlib.createInflateRaw(15);
inflate.on('error', function(e) {  });
inflate.write('something invalid');
setTimeout(function(){
    inflate.close();  
}, 100);

Output:

$ node inflate-close.js 
zlib.js:465
  this._handle.close();
              ^

TypeError: Cannot read property 'close' of null
    at InflateRaw.Zlib.close (zlib.js:465:15)
    at null._onTimeout (/tmp/inflate-close.js:8:13)
    at Timer.listOnTimeout (timers.js:92:15)

See websockets/ws#664

The text was updated successfully, but these errors were encountered:

Author

headlessme commented Apr 4, 2016

Looks like it's already being fixed: #5982

headlessme closed this as completed

ChALkeR added duplicate zlib labels

addaleax mentioned this issue

zlib: Fix use after null when calling .close #5982

Closed

4 tasks

addaleax mentioned this issue

test: Add zlib close-after-error regression test #6270

Closed

3 tasks

addaleax pushed a commit that referenced this issue


          zlib: fix use after null when calling .close

c7fef3d

An internal zlib error may cause _handle to be set to null.
Close now will check if there is a _handle prior to calling .close on
it.

PR-URL: #5982
Fixes: #6034
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

addaleax added a commit to addaleax/node that referenced this issue


          test: add zlib close-after-error regression test

96f04d6

Add a regression test based on the report in
nodejs#6034.

addaleax added a commit that referenced this issue


          test: add zlib close-after-error regression test

f8e507e

Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this issue


          zlib: fix use after null when calling .close

bad006f

An internal zlib error may cause _handle to be set to null.
Close now will check if there is a _handle prior to calling .close on
it.

PR-URL: #5982
Fixes: #6034
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this issue


          test: add zlib close-after-error regression test

4bfed26

Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this issue


          zlib: fix use after null when calling .close

4f6f599

An internal zlib error may cause _handle to be set to null.
Close now will check if there is a _handle prior to calling .close on
it.

PR-URL: #5982
Fixes: #6034
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this issue


          test: add zlib close-after-error regression test

3bacd19

Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

joelostrowski pushed a commit to joelostrowski/node that referenced this issue


          zlib: fix use after null when calling .close

8dbd5c5

An internal zlib error may cause _handle to be set to null.
Close now will check if there is a _handle prior to calling .close on
it.

PR-URL: nodejs#5982
Fixes: nodejs#6034
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

joelostrowski pushed a commit to joelostrowski/node that referenced this issue


          test: add zlib close-after-error regression test

f5532a1

Add a regression test based on the report in
nodejs#6034.

PR-URL: nodejs#6270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell pushed a commit that referenced this issue


          zlib: fix use after null when calling .close

bd944e3

An internal zlib error may cause _handle to be set to null.
Close now will check if there is a _handle prior to calling .close on
it.

PR-URL: #5982
Fixes: #6034
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

jasnell pushed a commit that referenced this issue


          test: add zlib close-after-error regression test

6e3301f

Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

addaleax added a commit to addaleax/node that referenced this issue


          zlib: remove _closed in source

0b5f44a

This is purely cleanup and carries no visible behavioural changes.
Up to now, `this._closed` was used in zlib.js as a
synonym of `!this._handle`. This change makes this connection
explicit and removes the `_closed` property from zlib streams,
as the previous duplication has been the cause of subtle errors
like nodejs#6034.

This also makes zlib errors lead to an explicit `_close()` call
rather than waiting for garbage collection to clean up the handle,
thus returning memory resources earlier in the case of an error.

addaleax mentioned this issue

zlib: remove _closed #6574

Closed

2 tasks

addaleax added a commit that referenced this issue


          zlib: remove _closed in source

b53473f

This is purely cleanup and carries no visible behavioural changes.
Up to now, `this._closed` was used in zlib.js as a
synonym of `!this._handle`. This change makes this connection
explicit and removes the `_closed` property from zlib streams,
as the previous duplication has been the cause of subtle errors
like #6034.

This also makes zlib errors lead to an explicit `_close()` call
rather than waiting for garbage collection to clean up the handle,
thus returning memory resources earlier in the case of an error.

Add a getter for `_closed` so that the property remains accessible
by legacy code.

PR-URL: #6574
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this issue


          zlib: fix use after null when calling .close

0cc1e0f

An internal zlib error may cause _handle to be set to null.
Close now will check if there is a _handle prior to calling .close on
it.

PR-URL: #5982
Fixes: #6034
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this issue


          test: add zlib close-after-error regression test

Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this issue


          zlib: fix use after null when calling .close

48684af

An internal zlib error may cause _handle to be set to null.
Close now will check if there is a _handle prior to calling .close on
it.

PR-URL: #5982
Fixes: #6034
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this issue


          test: add zlib close-after-error regression test

f60ba54

Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Fishrock123 pushed a commit that referenced this issue


          zlib: remove _closed in source

46003f0

This is purely cleanup and carries no visible behavioural changes.
Up to now, `this._closed` was used in zlib.js as a
synonym of `!this._handle`. This change makes this connection
explicit and removes the `_closed` property from zlib streams,
as the previous duplication has been the cause of subtle errors
like #6034.

This also makes zlib errors lead to an explicit `_close()` call
rather than waiting for garbage collection to clean up the handle,
thus returning memory resources earlier in the case of an error.

Add a getter for `_closed` so that the property remains accessible
by legacy code.

PR-URL: #6574
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

rvagg pushed a commit that referenced this issue


          zlib: remove _closed in source

This is purely cleanup and carries no visible behavioural changes.
Up to now, `this._closed` was used in zlib.js as a
synonym of `!this._handle`. This change makes this connection
explicit and removes the `_closed` property from zlib streams,
as the previous duplication has been the cause of subtle errors
like #6034.

This also makes zlib errors lead to an explicit `_close()` call
rather than waiting for garbage collection to clean up the handle,
thus returning memory resources earlier in the case of an error.

Add a getter for `_closed` so that the property remains accessible
by legacy code.

PR-URL: #6574
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

fippo mentioned this issue

crashes in zlib.js fippo/rtcstats-server#149

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment