Skip to content

Commit

Permalink
tls: TLSSocket options default isServer false
Browse files Browse the repository at this point in the history
Upon creating a TLSSocket object, set the default isServer option to false
Updated tls docs and added test-tls-socket-default-options

PR-URL: #2614
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information
jhamhader authored and indutny committed Oct 20, 2015
1 parent b78de12 commit adfd20b
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 5 deletions.
7 changes: 4 additions & 3 deletions doc/api/tls.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -454,18 +454,19 @@ Or
Wrapper for instance of [net.Socket][], replaces internal socket read/write
routines to perform transparent encryption/decryption of incoming/outgoing data.

## new tls.TLSSocket(socket, options)
## new tls.TLSSocket(socket[, options])

Construct a new TLSSocket object from existing TCP socket.

`socket` is an instance of [net.Socket][]

`options` is an object that might contain following properties:
`options` is an optional object that might contain following properties:

- `secureContext`: An optional TLS context object from
`tls.createSecureContext( ... )`

- `isServer`: If true - TLS socket will be instantiated in server-mode
- `isServer`: If `true` - TLS socket will be instantiated in server-mode.
Default: `false`

- `server`: An optional [net.Server][] instance

Expand Down
7 changes: 5 additions & 2 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,10 @@ function initRead(tls, wrapped) {
*/

function TLSSocket(socket, options) {
this._tlsOptions = options;
if (options === undefined)

This comment has been minimized.

Copy link
@milne-dev

milne-dev Oct 20, 2015

could it not just be
this._tlsOptions = options || {};

:)

This comment has been minimized.

Copy link
@cjihrig

cjihrig Oct 20, 2015

Contributor

Or this._tlsOptions = Object.assign({}, options);

This comment has been minimized.

Copy link
@milne-dev

milne-dev Oct 20, 2015

^^ that is a cool way to do it. never knew about that method until now!

This comment has been minimized.

Copy link
@indutny

indutny Oct 20, 2015

Member

@cjihrig but it is a bit pointless? I agree that it could be options || {}.

This comment has been minimized.

Copy link
@milne-dev

milne-dev Oct 20, 2015

@indutny you replied OMGOSH I feel starstruck!!

This comment has been minimized.

Copy link
@cjihrig

cjihrig Oct 20, 2015

Contributor

@indutny I just added that because Object.assign() is a little more resistant to errors if the function is called incorrectly. For example, if the function was mistakenly called with 5 as the second argument. options || {} would give 5, which would likely crash later on. Object.assign() would still give an object. It's not a big deal, since they shouldn't be calling the function like that, but it's better than a crash.

this._tlsOptions = {};
else
this._tlsOptions = options;
this._secureEstablished = false;
this._securePending = false;
this._newSessionPending = false;
Expand Down Expand Up @@ -321,7 +324,7 @@ TLSSocket.prototype._wrapHandle = function(wrap) {
tls.createSecureContext();
res = tls_wrap.wrap(handle._externalStream,
context.context,
options.isServer);
!!options.isServer);
res._parent = handle;
res._parentWrap = wrap;
res._secureContext = context;
Expand Down
56 changes: 56 additions & 0 deletions test/parallel/test-tls-socket-default-options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
'use strict';
const common = require('../common');
const assert = require('assert');

if (!common.hasCrypto) {
console.log('1..0 # Skipped: missing crypto');
return;
}
const tls = require('tls');

const fs = require('fs');
const net = require('net');

const sent = 'hello world';

const serverOptions = {
isServer: true,
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
};

function testSocketOptions(socket, socketOptions) {
let received = '';
const server = tls.createServer(serverOptions, function(s) {
s.on('data', function(chunk) {
received += chunk;
});

s.on('end', function() {
server.close();
s.destroy();
assert.equal(received, sent);
setImmediate(runTests);
});
}).listen(common.PORT, function() {
let c = new tls.TLSSocket(socket, socketOptions);
c.connect(common.PORT, function() {
c.end(sent);
});
});

}

const testArgs = [
[],
[undefined, {}]
];

let n = 0;
function runTests() {
if (n++ < testArgs.length) {
testSocketOptions.apply(null, testArgs[n]);
}
}

runTests();

0 comments on commit adfd20b

Please sign in to comment.