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

Reuse TLS sessions in HTTPS client #1499

Closed
bajtos opened this issue Apr 22, 2015 · 4 comments
Closed

Reuse TLS sessions in HTTPS client #1499

bajtos opened this issue Apr 22, 2015 · 4 comments
Labels
feature request Issues that request new features to be added to Node.js. https Issues or PRs related to the https subsystem.

Comments

@bajtos
Copy link
Contributor

bajtos commented Apr 22, 2015

The HTTPS client created via require('https').request() should reuse TLS sessions by default.

Sample code:

var sessions = [];

get(function() {
  get(function() {
    require('assert').equal(sessions[0].toString('hex'), sessions[1].toString('hex'));
    console.log('PASS');
  });
});

function get(cb) {
  return new Promise(function(resolve, reject) {
    require('https').request(
      { host: 'github.com', headers: { connection: 'close' } },
      function(res) {
        sessions.push(res.connection.getSession());
        res.resume();
        res.on('end', cb);
      })
      .end();
    })
}

/cc @indutny

@bajtos
Copy link
Contributor Author

bajtos commented Apr 22, 2015

Strangely enough, passing the result of getSession to the second request does not work as expected.

var sessions = [];

get(function() {
  get(function() {
    require('assert').equal(sessions[0].toString('hex'), sessions[1].toString('hex'));
    console.log('PASS');
  });
});

function get(cb) {
  return new Promise(function(resolve, reject) {
    require('https').request(
      { host: 'github.com', headers: { connection: 'close' }, session: sessions[0] },
      function(res) {
        sessions.push(res.connection.getSession());
        res.resume();
        res.on('end', function() { cb(); });
      })
      .end();
    })
}

The first request passes, the second request fails with the following error:

tls.js:176
      var commonNames = cert.subject.CN;
                                    ^
TypeError: Cannot read property 'CN' of undefined
    at Object.checkServerIdentity (tls.js:176:37)
    at TLSSocket.<anonymous> (_tls_wrap.js:913:29)
    at emitNone (events.js:67:13)
    at TLSSocket.emit (events.js:163:7)
    at TLSSocket._finishInit (_tls_wrap.js:496:8)

io.js version 1.8.1.

@Fishrock123 Fishrock123 added the https Issues or PRs related to the https subsystem. label Apr 22, 2015
@ChALkeR
Copy link
Member

ChALkeR commented Apr 25, 2015

The first request passes, the second request fails

Add checkServerIdentity: function(){} to the options of the second request.

@indutny
Copy link
Member

indutny commented Jul 23, 2015

Fix is here: #2228

@brendanashworth brendanashworth added the feature request Issues that request new features to be added to Node.js. label Jul 23, 2015
indutny added a commit to indutny/io.js that referenced this issue Jul 27, 2015
Fix: nodejs#1499
PR-URL: nodejs#2228
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
indutny added a commit that referenced this issue Jul 27, 2015
Fix: #1499
PR-URL: #2228
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@brendanashworth
Copy link
Contributor

This was fixed in 2ca5a3d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. https Issues or PRs related to the https subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants