Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Allow custom TLS options #182

Merged
merged 1 commit into from

2 participants

@naholyr
  • Option secure can now be a hash of options to be passed to tls.connect
  • If secure is simply set to true, default TLS option rejectUnauthorized is set to false to force pre-0.9.2 behavior (fixing #181)
@mscdex
Owner

What if we just re-used the 'secure' property? If 'secure' is set to some truthy value, we know we want SSL/TLS anyway, so a value of true means default options and a value of an object means specific SSL/TLS settings.

@naholyr

Yep I love it :) sending the change…

@naholyr

What do you think of this one ?

lib/imap.js
@@ -47,7 +47,9 @@ function ImapConnection(options) {
password: options.password || '',
host: options.host || 'localhost',
port: options.port || 143,
- secure: options.secure || false,
+ secure: options.secure === true ? { // secure = true means default behavior
+ rejectUnauthorized: false // Force pre-0.9.2 behavior
@mscdex Owner
mscdex added a note

should probably clarify that it's pre- node 0.9.2 behavior not node-imap 0.9.2

@naholyr
naholyr added a note

right :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mscdex
Owner

Other than the comment issue, it looks fine to me. Can you squash the commits?

lib/imap.js
@@ -124,10 +126,11 @@ ImapConnection.prototype.connect = function(loginCb) {
socket.setTimeout(0);
if (this._options.secure) {
+ this._options.secure.socket = state.conn;
@naholyr
naholyr added a note

not fond of this one, but I'm not sure it's worth a merge

@mscdex Owner
mscdex added a note

Hrmm... how about we just create a copy of this._options.secure, add the 'socket' property to the copy, and pass that to tls.connect? That way we don't mutate the original.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@naholyr

yep

@naholyr

And voilà!

@mscdex mscdex merged commit 5610cb9 into mscdex:master
@mscdex
Owner

Thanks!

@naholyr naholyr deleted the lmtm:tls-compat branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 12, 2013
  1. @naholyr
This page is out of date. Refresh to see the latest.
Showing with 10 additions and 3 deletions.
  1. +10 −3 lib/imap.js
View
13 lib/imap.js
@@ -47,7 +47,9 @@ function ImapConnection(options) {
password: options.password || '',
host: options.host || 'localhost',
port: options.port || 143,
- secure: options.secure || false,
+ secure: options.secure === true ? { // secure = true means default behavior
+ rejectUnauthorized: false // Force pre-node-0.9.2 behavior
+ } : (options.secure || false),
connTimeout: options.connTimeout || 10000, // connection timeout in msecs
xoauth: options.xoauth,
xoauth2: options.xoauth2
@@ -124,10 +126,15 @@ ImapConnection.prototype.connect = function(loginCb) {
socket.setTimeout(0);
if (this._options.secure) {
+ var tlsOptions = {};
+ for (var k in this._options.secure) {
+ tlsOptions[k] = this._options.secure[k];
+ }
+ tlsOptions.socket = state.conn;
if (process.version.indexOf('v0.6.') > -1)
- socket = tls.connect(null, { socket: state.conn }, onconnect);
+ socket = tls.connect(null, tlsOptions, onconnect);
else
- socket = tls.connect({ socket: state.conn }, onconnect);
+ socket = tls.connect(tlsOptions, onconnect);
} else
state.conn.once('connect', onconnect);
Something went wrong with that request. Please try again.