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

net,tls: add abort signal support to connect #37735

Closed

Conversation

Linkgoron
Copy link
Member

@Linkgoron Linkgoron commented Mar 13, 2021

A previous PR added support for AbortSignal to the net.Socket constructor and net.connect, however there was some missing documentation. I've added documentation (albeit minor) - I'd be happy for more suggestions if there are any other places that need it added, and also added tests for net.Socket and net.connect.

In addition, I've extended the AbortSignal support from net.Socket to tls.TLSSocket as it was a relatively minor change, and added tests for tls.connect, tls.TLSSocket, http2.connect and https.Agent.

There are https.Agent and https.request tests that fail without #37730, so this PR is blocked on that one.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem. labels Mar 13, 2021
@Linkgoron Linkgoron force-pushed the net-tls-add-connect-abort-signal branch 2 times, most recently from e099f3f to bcc1e4f Compare March 13, 2021 02:54
@aduh95 aduh95 added blocked PRs that are blocked by other issues or PRs. semver-minor PRs that contain new features and should be released in the next minor version. labels Mar 13, 2021
@Linkgoron Linkgoron force-pushed the net-tls-add-connect-abort-signal branch from bcc1e4f to 332ab4f Compare March 13, 2021 10:50
await testConstructorPost();
await testConstructorPostTick();

// Killing the net.socket without connecting hangs the server.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably easier to call socket.setTimeout with a low timeout - but probably what you did is also fine

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after #37730 lands

@Linkgoron Linkgoron force-pushed the net-tls-add-connect-abort-signal branch from 332ab4f to 1fd2961 Compare March 13, 2021 22:48
Add documentation for net.connect AbortSignal,
and add the support to tls.connect as well
@Linkgoron Linkgoron force-pushed the net-tls-add-connect-abort-signal branch from 1fd2961 to ce73ba1 Compare March 20, 2021 20:04
@benjamingr benjamingr added the notable-change PRs with changes that should be highlighted in changelogs. label Mar 20, 2021
@Linkgoron Linkgoron removed the blocked PRs that are blocked by other issues or PRs. label Mar 20, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Linkgoron Linkgoron added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 1, 2021
@jasnell
Copy link
Member

jasnell commented Apr 1, 2021

Landed in f87c4d1

@jasnell jasnell closed this Apr 1, 2021
jasnell pushed a commit that referenced this pull request Apr 1, 2021
Add documentation for net.connect AbortSignal,
and add the support to tls.connect as well

PR-URL: #37735
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 4, 2021
Add documentation for net.connect AbortSignal,
and add the support to tls.connect as well

PR-URL: #37735
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins added a commit that referenced this pull request Apr 4, 2021
This is a security release

Vulnerabilities fixed:

Coming Soon

Other Notable changes:

fs:
  * (SEMVER-MINOR) add support for async iterators to `fsPromises.writeFile` (HiroyukiYagihashi) #37490
net:
  * (SEMVER-MINOR) allow net.BlockList to use net.SocketAddress objects (James M Snell) #37917
  * (SEMVER-MINOR) add SocketAddress class (James M Snell) #37917
  * (SEMVER-MINOR) make net.BlockList cloneable (James M Snell) #37917
net,tls:
  * (SEMVER-MINOR) add abort signal support to connect (Nitzan Uziely) #37735
readline:
  * (SEMVER-MINOR) add AbortSignal support to interface (Nitzan Uziely) #37932

PR-URL: TODO
@MylesBorins MylesBorins mentioned this pull request Apr 4, 2021
MylesBorins added a commit that referenced this pull request Apr 5, 2021
Vulnerabilities fixed:

- **CVE-2021-3450**: OpenSSL - CA certificate check bypass with X509_V_FLAG_X509_STRICT (High)
- **CVE-2021-3449**: OpenSSL - NULL pointer deref in signature_algorithms processing (High)
- **CVE-2020-7774**: npm upgrade - Update y18n to fix Prototype-Pollution (High)

Other Notable changes:

fs:
  * (SEMVER-MINOR) add support for async iterators to `fsPromises.writeFile` (HiroyukiYagihashi) #37490
net:
  * (SEMVER-MINOR) allow net.BlockList to use net.SocketAddress objects (James M Snell) #37917
  * (SEMVER-MINOR) add SocketAddress class (James M Snell) #37917
  * (SEMVER-MINOR) make net.BlockList cloneable (James M Snell) #37917
net,tls:
  * (SEMVER-MINOR) add abort signal support to connect (Nitzan Uziely) #37735
readline:
  * (SEMVER-MINOR) add AbortSignal support to interface (Nitzan Uziely) #37932

PR-URL: #38084
MylesBorins added a commit that referenced this pull request Apr 6, 2021
Notable Changes:

This is a security release.

Vulnerabilities fixed:

- **CVE-2021-3450**: OpenSSL - CA certificate check bypass with X509_V_FLAG_X509_STRICT (High)
- **CVE-2021-3449**: OpenSSL - NULL pointer deref in signature_algorithms processing (High)
- **CVE-2020-7774**: npm upgrade - Update y18n to fix Prototype-Pollution (High)

Other Notable changes:

fs:
  * (SEMVER-MINOR) add support for async iterators to `fsPromises.writeFile` (HiroyukiYagihashi) #37490
net:
  * (SEMVER-MINOR) allow net.BlockList to use net.SocketAddress objects (James M Snell) #37917
  * (SEMVER-MINOR) add SocketAddress class (James M Snell) #37917
  * (SEMVER-MINOR) make net.BlockList cloneable (James M Snell) #37917
net,tls:
  * (SEMVER-MINOR) add abort signal support to connect (Nitzan Uziely) #37735
readline:
  * (SEMVER-MINOR) add AbortSignal support to interface (Nitzan Uziely) #37932

PR-URL: #38084
MylesBorins added a commit that referenced this pull request Apr 6, 2021
Notable Changes:

This is a security release.

Vulnerabilities fixed:

- **CVE-2021-3450**: OpenSSL - CA certificate check bypass with X509_V_FLAG_X509_STRICT (High)
- **CVE-2021-3449**: OpenSSL - NULL pointer deref in signature_algorithms processing (High)
- **CVE-2020-7774**: npm upgrade - Update y18n to fix Prototype-Pollution (High)

Other Notable changes:

fs:
  * (SEMVER-MINOR) add support for async iterators to `fsPromises.writeFile` (HiroyukiYagihashi) #37490
net:
  * (SEMVER-MINOR) allow net.BlockList to use net.SocketAddress objects (James M Snell) #37917
  * (SEMVER-MINOR) add SocketAddress class (James M Snell) #37917
  * (SEMVER-MINOR) make net.BlockList cloneable (James M Snell) #37917
net,tls:
  * (SEMVER-MINOR) add abort signal support to connect (Nitzan Uziely) #37735
readline:
  * (SEMVER-MINOR) add AbortSignal support to interface (Nitzan Uziely) #37932

PR-URL: #38084
MylesBorins added a commit that referenced this pull request Apr 6, 2021
Notable Changes:

This is a security release.

Vulnerabilities fixed:

- **CVE-2021-3450**: OpenSSL - CA certificate check bypass with X509_V_FLAG_X509_STRICT (High)
- **CVE-2021-3449**: OpenSSL - NULL pointer deref in signature_algorithms processing (High)
- **CVE-2020-7774**: npm upgrade - Update y18n to fix Prototype-Pollution (High)

Other Notable changes:

fs:
  * (SEMVER-MINOR) add support for async iterators to `fsPromises.writeFile` (HiroyukiYagihashi) #37490
net:
  * (SEMVER-MINOR) allow net.BlockList to use net.SocketAddress objects (James M Snell) #37917
  * (SEMVER-MINOR) add SocketAddress class (James M Snell) #37917
  * (SEMVER-MINOR) make net.BlockList cloneable (James M Snell) #37917
net,tls:
  * (SEMVER-MINOR) add abort signal support to connect (Nitzan Uziely) #37735
readline:
  * (SEMVER-MINOR) add AbortSignal support to interface (Nitzan Uziely) #37932

PR-URL: #38084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants