FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tls: honor pauseOnConnect option by r1b · Pull Request #29635 · nodejs/node · GitHub

/ node Public

tls: honor pauseOnConnect option - #29635

Closed
r1b wants to merge 3 commits into
nodejs:masterfrom
r1b:master
ConversationCommits (3)ChecksFiles changed
Closed

tls: honor pauseOnConnect option#29635
r1b wants to merge 3 commits into
nodejs:masterfrom
r1b:master

Conversation

r1b commented Sep 20, 2019

Copy link
Copy Markdown
Contributor

pauseOnConnect is now passed along to the net.Socket constructor from
the tls.Socket constructor. The readable flag must match the value of
pauseOnConnect. Tests were added to cover all available net.Server
options when used in the tls.Server constructor.

Fixes: #29620
Refs: #27665

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

nodejs-github-bot added the tls Issues and PRs related to the tls subsystem. label Sep 20, 2019
r1b commented Sep 20, 2019
View reviewed changes
Comment thread lib/_tls_wrap.js Outdated
r1b commented Sep 20, 2019
View reviewed changes
Comment thread lib/_tls_wrap.js Outdated
tls: honor pauseOnConnect option
ac50259 
`pauseOnConnect` is now passed along to the net.Socket constructor from
the tls.Socket constructor. The `readable` flag must match the value of
`pauseOnConnect`. Tests were added to cover all available net.Server
options when used in the tls.Server constructor.

Fixes: #29620
Refs: #27665
r1b requested review from lpinca and mscdex September 20, 2019 22:31

nodejs-github-bot commented Sep 21, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25622/

lpinca approved these changes Sep 21, 2019
View reviewed changes

lpinca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

SGTM.

Trott previously approved these changes Sep 24, 2019
View reviewed changes
Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Sep 24, 2019
Trott dismissed their stale review September 24, 2019 06:03

I'm a little uncertain about pauseOnCreate vs. pauseOnConnect and should defer to subject-matter experts anyway.

Trott commented Sep 25, 2019

Copy link
Copy Markdown
Member

@nodejs/collaborators This could use another review.

jasnell commented Sep 25, 2019

Copy link
Copy Markdown
Member

Are documentation updates needed for this?

Update lib/_tls_wrap.js
3b0c463 
Co-Authored-By: Rich Trott <rtrott@gmail.com>

r1b commented Sep 25, 2019

Copy link
Copy Markdown
Contributor Author

@jasnell Currently the tls.createServer docs indicate that:

Any net.createServer() option can be provided.

This will be correct once this is merged. Currently only allowHalfOpen has any effect.

nodejs-github-bot commented Sep 27, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25745/

nodejs-github-bot commented Sep 28, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25758/

jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 30, 2019

nodejs-github-bot commented Oct 2, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25826/

nodejs-github-bot commented Oct 3, 2019 •
edited by Trott
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25828/ ✔️

Trott commented Oct 4, 2019

Copy link
Copy Markdown
Member

Landed in 1e12859

Trott closed this Oct 4, 2019
Trott pushed a commit that referenced this pull request Oct 4, 2019
tls: honor pauseOnConnect option
1e12859 
`pauseOnConnect` is now passed along to the net.Socket constructor from
the tls.Socket constructor. The `readable` flag must match the value of
`pauseOnConnect`. Tests were added to cover all available net.Server
options when used in the tls.Server constructor.

Fixes: #29620
Refs: #27665

PR-URL: #29635
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BridgeAR pushed a commit that referenced this pull request Oct 9, 2019
tls: honor pauseOnConnect option
a7b56a5 
`pauseOnConnect` is now passed along to the net.Socket constructor from
the tls.Socket constructor. The `readable` flag must match the value of
`pauseOnConnect`. Tests were added to cover all available net.Server
options when used in the tls.Server constructor.

Fixes: #29620
Refs: #27665

PR-URL: #29635
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BridgeAR mentioned this pull request Oct 10, 2019
Merged
BridgeAR added a commit that referenced this pull request Oct 10, 2019
2019-10-10, Version 12.12.0 (Current)
75baa7e 
Notable changes:

* build:
  * Add `--force-context-aware` flag to prevent usage of native node
    addons that aren't context aware
    #29631
* deprecations:
  * Add documentation-only deprecation for `process._tickCallback()`
    #29781
* esm:
  * Using JSON modules is experimental again
    #29754
* fs:
  * Introduce `opendir()` and `fs.Dir` to iterate through directories
    #29349
* process:
  * Add source-map support to stack traces by using
    `--source-map-support` #29564
* tls:
  * Honor `pauseOnConnect` option
    #29635
  * Add option for private keys for OpenSSL engines
    #28973

PR-URL: #29919
BridgeAR added a commit that referenced this pull request Oct 11, 2019
2019-10-11, Version 12.12.0 (Current)
921493e 
Notable changes:

* build:
  * Add `--force-context-aware` flag to prevent usage of native node
    addons that aren't context aware
    #29631
* deprecations:
  * Add documentation-only deprecation for `process._tickCallback()`
    #29781
* esm:
  * Using JSON modules is experimental again
    #29754
* fs:
  * Introduce `opendir()` and `fs.Dir` to iterate through directories
    #29349
* process:
  * Add source-map support to stack traces by using
    `--source-map-support` #29564
* tls:
  * Honor `pauseOnConnect` option
    #29635
  * Add option for private keys for OpenSSL engines
    #28973

PR-URL: #29919
BridgeAR added a commit that referenced this pull request Oct 11, 2019
2019-10-11, Version 12.12.0 (Current)
06f6d66 
Notable changes:

* build:
  * Add `--force-context-aware` flag to prevent usage of native node
    addons that aren't context aware
    #29631
* deprecations:
  * Add documentation-only deprecation for `process._tickCallback()`
    #29781
* esm:
  * Using JSON modules is experimental again
    #29754
* fs:
  * Introduce `opendir()` and `fs.Dir` to iterate through directories
    #29349
* process:
  * Add source-map support to stack traces by using
    `--source-map-support` #29564
* tls:
  * Honor `pauseOnConnect` option
    #29635
  * Add option for private keys for OpenSSL engines
    #28973

PR-URL: #29919
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Trott Trott left review comments
lpinca lpinca approved these changes
mscdex Awaiting requested review from mscdex

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tls: pauseOnConnect option not honored

6 participants

Back | FazBrowse Home | New Git URL