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

domains: fix handling of uncaught exceptions #3887

Conversation

misterdjules
Copy link

Fix node exiting due to an exception being thrown rather than emitting
an 'uncaughtException' event on the process object when:

  1. no error handler is set on the domain within which an error is thrown
  2. an 'uncaughtException' event listener is set on the process

Also fix an issue where the process would not abort in the proper
function call if an error is thrown within a domain with no error
handler and --abort-on-uncaught-exception is used.

Fixes #3607 and #3653.

@misterdjules misterdjules added domain Issues and PRs related to the domain subsystem. lts-watch-v0.10 labels Nov 17, 2015
@misterdjules
Copy link
Author

This is a continuation of #3637, but this time targeted to the v0.10-staging branch and without the change that was introducing the behavior change mentioned in #3654 (comment).

/cc @nodejs/lts @nodejs/ctc @nodejs/collaborators

@jasnell
Copy link
Member

jasnell commented Nov 18, 2015

LGTM. Would appreciate a few more eyes on it tho @nodejs/lts

@misterdjules
Copy link
Author

Updated this PR according to the code review comments in #3654 that apply to the v0.10 branch.

Fix node exiting due to an exception being thrown rather than emitting
an 'uncaughtException' event on the process object when:
1. no error handler is set on the domain within which an error is thrown
2. an 'uncaughtException' event listener is set on the process

Also fix an issue where the process would not abort in the proper
function call if an error is thrown within a domain with no error
handler and --abort-on-uncaught-exception is used.

Fixes nodejs#3607 and nodejs#3653.
@misterdjules misterdjules force-pushed the fix-uncaught-exceptions-domains-v0.10 branch from fbb4581 to 82ce3a6 Compare November 24, 2015 19:07
@misterdjules
Copy link
Author

Updated this PR again according to the code review comments in #3654 that apply to the v0.10 branch.

@misterdjules
Copy link
Author

CI tests running.

@priyavivek1
Copy link

@misterdjules : Would you please let me know when will this be availble

@misterdjules
Copy link
Author

@priyavivek1 You will get notifications about any change in this pull-request, including when the changes are merged in the v0.10-staging branch. After that point, the fix will be published in the next v0.10.x release.

@piscisaureus
Copy link
Contributor

@misterdjules

What's holding up this patch getting landed? You need another review?

@misterdjules
Copy link
Author

@piscisaureus I need to add one test to #3654, then back port some of the changes in that PR into this one, and then get a second review. I can finish that tomorrow, I'm at Node Interactive right now and can't work on that.

@misterdjules
Copy link
Author

Back ported latest changes according to code review in #3654 and started CI tests again.

@misterdjules
Copy link
Author

/ping @nodejs/lts @nodejs/ctc All tests pass, except for flaky tests on Windows, and for the linter (which hasn't passed for a long while in v0.10).

@jasnell
Copy link
Member

jasnell commented Dec 16, 2015

LGTM

misterdjules pushed a commit that referenced this pull request Dec 17, 2015
Fix node exiting due to an exception being thrown rather than emitting
an 'uncaughtException' event on the process object when:
1. no error handler is set on the domain within which an error is thrown
2. an 'uncaughtException' event listener is set on the process

Also fix an issue where the process would not abort in the proper
function call if an error is thrown within a domain with no error
handler and --abort-on-uncaught-exception is used.

Fixes #3607 and #3653.

PR: #3887
PR-URL: #3887
Reviewed-By: James M Snell <[email protected]>
@misterdjules
Copy link
Author

Thank you @jasnell for the review, landed in 9cae9b2.

MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
Fix node exiting due to an exception being thrown rather than emitting
an 'uncaughtException' event on the process object when:
1. no error handler is set on the domain within which an error is thrown
2. an 'uncaughtException' event listener is set on the process

Also fix an issue where the process would not abort in the proper
function call if an error is thrown within a domain with no error
handler and --abort-on-uncaught-exception is used.

Fixes #3607 and #3653.

PR: #3887
PR-URL: #3887
Reviewed-By: James M Snell <[email protected]>
rvagg added a commit that referenced this pull request Feb 24, 2016
Notable changes:

* http_parser: Update to http-parser 1.2 to fix an unintentionally
  strict limitation of allowable header characters
  (James M Snell) #5242
* domains:
  - Prevent an exit due to an exception being thrown rather than
    emitting an `'uncaughtException'` event on the `process` object
    when no error handler is set on the domain within which an error
    is thrown and an `'uncaughtException'` event listener is set on
    `process`. (Julien Gilli) #3887
  - Fix an issue where the process would not abort in the proper
    function call if an error is thrown within a domain with no error
    handler and `--abort-on-uncaught-exception` is used.
    (Julien Gilli) #3887
rvagg added a commit that referenced this pull request Feb 26, 2016
Notable changes:

* http_parser: Update to http-parser 1.2 to fix an unintentionally
  strict limitation of allowable header characters.
  (James M Snell) #5242
* domains:
  - Prevent an exit due to an exception being thrown rather than
    emitting an `'uncaughtException'` event on the `process` object
    when no error handler is set on the domain within which an error
    is thrown and an `'uncaughtException'` event listener is set on
    `process`. (Julien Gilli) #3887
  - Fix an issue where the process would not abort in the proper
    function call if an error is thrown within a domain with no error
    handler and `--abort-on-uncaught-exception` is used.
    (Julien Gilli) #3887
rvagg added a commit that referenced this pull request Mar 3, 2016
Notable changes:

* http_parser: Update to http-parser 1.2 to fix an unintentionally
  strict limitation of allowable header characters.
  (James M Snell) #5242
* domains:
  - Prevent an exit due to an exception being thrown rather than
  emitting an 'uncaughtException' event on the `process` object when
  no error handler is set on the domain within which an error is
  thrown and an 'uncaughtException' event listener is set on
  `process`. (Julien Gilli) #3887
  - Fix an issue where the process would not abort in the proper
  function call if an error is thrown within a domain with no error
  handler and `--abort-on-uncaught-exception` is used.
  (Julien Gilli) #3887
* openssl: Upgrade from 1.0.1r to 1.0.1s
  (Ben Noordhuis) #5508
  - Fix a double-free defect in parsing malformed DSA keys that may
    potentially be used for DoS or memory corruption attacks. It is
    likely to be very difficult to use this defect for a practical
    attack and is therefore considered low severity for Node.js users.
    More info is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0705
  - Fix a defect that can cause memory corruption in certain very rare
    cases relating to the internal `BN_hex2bn()` and `BN_dec2bn()`
    functions. It is believed that Node.js is not invoking the code
    paths that use these functions so practical attacks via Node.js
    using this defect are _unlikely_ to be possible. More info is
    available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0797
  - Fix a defect that makes the CacheBleed Attack
    (https://ssrg.nicta.com.au/projects/TS/cachebleed/) possible.
    This defect enables attackers to execute side-channel attacks
    leading to the potential recovery of entire RSA private keys. It
    only affects the Intel Sandy Bridge (and possibly older)
    microarchitecture when using hyper-threading. Newer
    microarchitectures, including Haswell, are unaffected. More info
    is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0702
  - Remove SSLv2 support, the `--enable-ssl2` command line argument
    will now produce an error. The DROWN Attack
    (https://drownattack.com/) creates a vulnerability where SSLv2 is
    enabled by a server, even if a client connection is not using
    SSLv2. The SSLv2 protocol is widely considered unacceptably broken
    and should not be supported. More information is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0800

PR-URL: #5404
rvagg added a commit that referenced this pull request Mar 4, 2016
Notable changes:

* http_parser: Update to http-parser 1.2 to fix an unintentionally
  strict limitation of allowable header characters.
  (James M Snell) #5242
* domains:
  - Prevent an exit due to an exception being thrown rather than
  emitting an 'uncaughtException' event on the `process` object when
  no error handler is set on the domain within which an error is
  thrown and an 'uncaughtException' event listener is set on
  `process`. (Julien Gilli) #3887
  - Fix an issue where the process would not abort in the proper
  function call if an error is thrown within a domain with no error
  handler and `--abort-on-uncaught-exception` is used.
  (Julien Gilli) #3887
* openssl: Upgrade from 1.0.1r to 1.0.1s
  (Ben Noordhuis) #5508
  - Fix a double-free defect in parsing malformed DSA keys that may
    potentially be used for DoS or memory corruption attacks. It is
    likely to be very difficult to use this defect for a practical
    attack and is therefore considered low severity for Node.js users.
    More info is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0705
  - Fix a defect that can cause memory corruption in certain very rare
    cases relating to the internal `BN_hex2bn()` and `BN_dec2bn()`
    functions. It is believed that Node.js is not invoking the code
    paths that use these functions so practical attacks via Node.js
    using this defect are _unlikely_ to be possible. More info is
    available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0797
  - Fix a defect that makes the CacheBleed Attack
    (https://ssrg.nicta.com.au/projects/TS/cachebleed/) possible.
    This defect enables attackers to execute side-channel attacks
    leading to the potential recovery of entire RSA private keys. It
    only affects the Intel Sandy Bridge (and possibly older)
    microarchitecture when using hyper-threading. Newer
    microarchitectures, including Haswell, are unaffected. More info
    is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0702
  - Remove SSLv2 support, the `--enable-ssl2` command line argument
    will now produce an error. The DROWN Attack
    (https://drownattack.com/) creates a vulnerability where SSLv2 is
    enabled by a server, even if a client connection is not using
    SSLv2. The SSLv2 protocol is widely considered unacceptably broken
    and should not be supported. More information is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0800

PR-URL: #5404
rvagg added a commit that referenced this pull request Mar 4, 2016
Notable changes:

* http_parser: Update to http-parser 1.2 to fix an unintentionally
  strict limitation of allowable header characters.
  (James M Snell) #5242
* domains:
  - Prevent an exit due to an exception being thrown rather than
  emitting an 'uncaughtException' event on the `process` object when
  no error handler is set on the domain within which an error is
  thrown and an 'uncaughtException' event listener is set on
  `process`. (Julien Gilli) #3887
  - Fix an issue where the process would not abort in the proper
  function call if an error is thrown within a domain with no error
  handler and `--abort-on-uncaught-exception` is used.
  (Julien Gilli) #3887
* openssl: Upgrade from 1.0.1r to 1.0.1s
  (Ben Noordhuis) #5508
  - Fix a double-free defect in parsing malformed DSA keys that may
    potentially be used for DoS or memory corruption attacks. It is
    likely to be very difficult to use this defect for a practical
    attack and is therefore considered low severity for Node.js users.
    More info is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0705
  - Fix a defect that can cause memory corruption in certain very rare
    cases relating to the internal `BN_hex2bn()` and `BN_dec2bn()`
    functions. It is believed that Node.js is not invoking the code
    paths that use these functions so practical attacks via Node.js
    using this defect are _unlikely_ to be possible. More info is
    available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0797
  - Fix a defect that makes the CacheBleed Attack
    (https://ssrg.nicta.com.au/projects/TS/cachebleed/) possible.
    This defect enables attackers to execute side-channel attacks
    leading to the potential recovery of entire RSA private keys. It
    only affects the Intel Sandy Bridge (and possibly older)
    microarchitecture when using hyper-threading. Newer
    microarchitectures, including Haswell, are unaffected. More info
    is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0702
  - Remove SSLv2 support, the `--enable-ssl2` command line argument
    will now produce an error. The DROWN Attack
    (https://drownattack.com/) creates a vulnerability where SSLv2 is
    enabled by a server, even if a client connection is not using
    SSLv2. The SSLv2 protocol is widely considered unacceptably broken
    and should not be supported. More information is available at
    https://www.openssl.org/news/vulnerabilities.html#2016-0800

PR-URL: #5404
@misterdjules misterdjules deleted the fix-uncaught-exceptions-domains-v0.10 branch July 24, 2017 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain Issues and PRs related to the domain subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants