Skip to content
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.

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

Zlib InflateRaw null reference #6034

Closed
headlessme opened this issue Apr 4, 2016 · 1 comment
Closed

Zlib InflateRaw null reference #6034

headlessme opened this issue Apr 4, 2016 · 1 comment
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. zlib Issues and PRs related to the zlib subsystem.

Comments

@headlessme
Copy link

  • 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

@headlessme
Copy link
Author

Looks like it's already being fixed: #5982

@ChALkeR ChALkeR added duplicate Issues and PRs that are duplicates of other issues or PRs. zlib Issues and PRs related to the zlib subsystem. labels Apr 4, 2016
addaleax pushed a commit that referenced this issue Apr 19, 2016
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
addaleax added a commit to addaleax/node that referenced this issue Apr 19, 2016
Add a regression test based on the report in
nodejs#6034.
addaleax added a commit that referenced this issue Apr 20, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 21, 2016
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 21, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
joelostrowski pushed a commit to joelostrowski/node that referenced this issue Apr 25, 2016
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
joelostrowski pushed a commit to joelostrowski/node that referenced this issue Apr 25, 2016
Add a regression test based on the report in
nodejs#6034.

PR-URL: nodejs#6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
jasnell pushed a commit that referenced this issue Apr 26, 2016
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
jasnell pushed a commit that referenced this issue Apr 26, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax added a commit to addaleax/node that referenced this issue May 4, 2016
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 addaleax mentioned this issue May 4, 2016
2 tasks
addaleax added a commit that referenced this issue May 17, 2016
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue May 17, 2016
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this issue May 17, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue May 18, 2016
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this issue May 18, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Fishrock123 pushed a commit that referenced this issue May 23, 2016
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
rvagg pushed a commit that referenced this issue Jun 2, 2016
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants