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

tls: pauseOnConnect option not honored #29620

Closed
r1b opened this issue Sep 19, 2019 · 4 comments
Closed

tls: pauseOnConnect option not honored #29620

r1b opened this issue Sep 19, 2019 · 4 comments

Comments

@r1b
Copy link
Contributor

r1b commented Sep 19, 2019

Background

  • v12.10.0:
  • Darwin mcdouble 18.6.0 Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64:
  • TLS

Issue

The fix in #27665 does not seem to work with the pauseOnConnect option. The test in that PR only checks the allowHalfOpen option. The docs indicate that Any net.createServer() option can be provided.

Reproduction

// Create self-signed cert at certificate.pem / key.pem

const assert = require('assert');
const fs = require('fs');
const { createServer } = require('tls');

const server = createServer({
  cert: fs.readFileSync('./certificate.pem'),
  key: fs.readFileSync('./key.pem'),
  pauseOnConnect: true,
}, socket => {
  assert(socket.isPaused(), 'Socket is not paused');
});

server.listen(6666);

Expect error 'Socket is not paused' when you ncat --ssl localhost 6666 or wget https://localhost:6666.

@r1b
Copy link
Contributor Author

r1b commented Sep 19, 2019

Our current workaround is to manually pause the socket explicitly with socket.pause() in the secureConnection listener.

@r1b
Copy link
Contributor Author

r1b commented Sep 19, 2019

Here's the cert and key I used.

self-signed-cert-and-key.zip

@cjihrig
Copy link
Contributor

cjihrig commented Sep 19, 2019

I believe the issue is that this code needs to pass pauseOnCreate to the socket like this code does.

I don't have time to work on it right this second, but probably can in the next day or two.

@r1b
Copy link
Contributor Author

r1b commented Sep 19, 2019

Thanks @cjihrig, I can work on a PR later tonight if you'd like.

@Trott Trott closed this as completed in 1e12859 Oct 4, 2019
BridgeAR pushed a commit that referenced this issue Oct 9, 2019
`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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants