Added socket caching to Auto Agent to handle the case when http2 requ…

…est and http1 requests are made to the same host+port

lib/autohttp/agent.js

@@ -61,59 +61,75 @@ class AutoHttp2Agent extends EventEmitter {
     cb,
     socketCb
   ) {
-    const options = Object.assign({}, reqOptions, this.options)
+    let options = Object.assign({}, reqOptions, this.options)
+    options = Object.assign(options, {
+      port: Number(options.port || this.defaultPort),
+      host: options.hostname || options.host || 'localhost'
+    })
+
+    // check if ALPN is cached
     const name = this.getName(options)
+    const [protocol, cachedSocket] = this.ALPNCache.get(name) || []
 
-    const uri = httpOptionsToUri(options)
-    const port = Number(options.port || this.defaultPort)
+    if (!protocol || !cachedSocket || cachedSocket.closed || cachedSocket.destroyed) {
+      // No cache exists or the initial socket used to establish the connection has been closed. Perform ALPN again.
+      this.ALPNCache.delete(name)
+      this.createNewSocketConnection(req, options, cb, socketCb)
+      return
+    }
 
-    // check if there is ALPN cached
-    const protocol = this.ALPNCache.get(name)
+    // No need to pass the cachedSocket since the respective protocol's agents will reuse the socket that was initially
+    // passed during ALPN Negotiation
     if (protocol === 'h2') {
-      const newOptions = Object.assign({}, options, {
-        port,
-        path: options.socketPath,
-        host: options.hostname || options.host || 'localhost'
+      const http2Options = Object.assign({}, options, {
+        path: options.socketPath
       })
 
       let connection
       try {
-        connection = this.http2Agent.createConnection(req, uri, newOptions)
+        const uri = httpOptionsToUri(options)
+        connection = this.http2Agent.createConnection(req, uri, http2Options)
       } catch (e) {
         cb(e)
         connection && connection.socket && socketCb(connection.socket)
         return
       }
+
       cb(null, 'http2', connection)
       socketCb(connection.socket)
+
       return
     }
+
     if (protocol === 'http/1.1' || protocol === 'http/1.0') {
-      const requestOptions = Object.assign({}, options, {
-        agent: this.httpsAgent,
-        host: options.hostname || options.host || 'localhost'
+      const http1RequestOptions = Object.assign({}, options, {
+        agent: this.httpsAgent
       })
+
       let request
       try {
-        request = https.request(requestOptions)
+        request = https.request(http1RequestOptions)
       } catch (e) {
         cb(e)
         return
       }
+
       request.on('socket', (socket) => socketCb(socket))
       cb(null, 'http1', request)
-      return
     }
+  }
+
+  createNewSocketConnection (req, options, cb, socketCb) {
+    const uri = httpOptionsToUri(options)
+    const name = this.getName(options)
 
-    const newOptions = Object.assign({}, options, {
-      port,
+    const tlsSocketOptions = Object.assign({}, options, {
       path: options.socketPath,
       ALPNProtocols: ['h2', 'http/1.1', 'http/1.0'],
-      servername: options.servername || calculateServerName(options),
-      host: options.hostname || options.host || 'localhost'
+      servername: options.servername || calculateServerName(options)
     })
 
-    const socket = tls.connect(newOptions)
+    const socket = tls.connect(tlsSocketOptions)
     socketCb(socket)
 
     const socketConnnectionErrorHandler = (e) => {
@@ -132,60 +148,69 @@ class AutoHttp2Agent extends EventEmitter {
         return
       }
 
-      this.ALPNCache.set(name, protocol)
+      if (protocol !== 'h2' && protocol !== 'http/1.1') {
+        cb(new Error('Unknown protocol' + protocol))
+        return
+      }
+
+      // Update the cache
+      this.ALPNCache.set(name, [protocol, socket])
+
+      socket.on('close', () => {
+        // Clean the cache when the socket closes
+        this.ALPNCache.delete(name)
+      })
 
       if (protocol === 'h2') {
-        const newOptions = Object.assign({}, options, {
-          port,
-          path: options.socketPath,
-          host: options.hostname || options.host || 'localhost'
+        const http2Options = Object.assign({}, options, {
+          path: options.socketPath
         })
         try {
           const connection = this.http2Agent.createConnection(
             req,
             uri,
-            newOptions,
+            http2Options,
             socket
           )
           cb(null, 'http2', connection)
         } catch (e) {
           cb(e)
         }
-      } else if (protocol === 'http/1.1') {
-        // Protocol is http1, using the built in
-        // We need to release all free sockets so that new connection is created using the overriden createconnection forcing the agent to reuse the socket used for alpn
+      }
+      if (protocol === 'http/1.1' || protocol === 'http/1.0') {
+        // Protocol is http1, using the built in agent
+        // We need to release all free sockets so that new connection is created using the overridden createconnection
+        // forcing the agent to reuse the socket used for alpn
 
-        // This reassignment works, since all code so far is sync, and happens in the same tick, hence there will be no race conditions
+        // This reassignment works, since all code so far is sync, and happens in the same tick, hence there will be no
+        // race conditions
         const oldCreateConnection = this.httpsAgent.createConnection
 
         this.httpsAgent.createConnection = () => {
           return socket
         }
 
-        const requestOptions = Object.assign({}, options, {
-          agent: this.httpsAgent,
-          host: options.hostname || options.host || 'localhost'
+        const http1RequestOptions = Object.assign({}, options, {
+          agent: this.httpsAgent
         })
         let request
         try {
-          request = https.request(requestOptions)
+          request = https.request(http1RequestOptions)
         } catch (e) {
           cb(e)
           return
         } finally {
           this.httpsAgent.createConnection = oldCreateConnection
         }
         cb(null, 'http1', request)
-      } else {
-        cb(new Error('Unknown protocol' + protocol))
       }
     })
   }
 
-  /*
-   * This function has been borrowed from Node.js HTTPS Agent implementation
-   * Ref: v20.15.0 https://github.com/nodejs/node/blob/6bf148e12b00a3ec596f4c123ec35445a48ab209/lib/https.js
-   */
+    /*
+     * This function has been borrowed from Node.js HTTPS Agent implementation
+     * Ref: v20.15.0 https://github.com/nodejs/node/blob/6bf148e12b00a3ec596f4c123ec35445a48ab209/lib/https.js
+     */
   getName (options) {
     let name = options.host || 'localhost'
 
@@ -200,6 +225,9 @@ class AutoHttp2Agent extends EventEmitter {
     name += ':'
     if (options.ca) { name += options.ca }
 
+    name += ':'
+    if (options.extraCA) { name += options.extraCA }
+
     name += ':'
     if (options.cert) { name += options.cert }
 

lib/autohttp/headerValidations.js

@@ -22,7 +22,6 @@ function checkIsHttpToken (token) {
   return RegExp(tokenRegExp).exec(token) !== null
 }
 
-
 module.exports = {
   assertValidPseudoHeader,
   checkIsHttpToken

lib/http2/agent.js

@@ -15,9 +15,11 @@ class Http2Agent extends EventEmitter {
 
     if (!connection || connection.destroyed || connection.closed) {
       const connectionOptions = Object.assign({}, options,
-        { createConnection: undefined, port: _options.port || 443, settings: {
-          enablePush: false
-        } })
+        { createConnection: undefined,
+          port: _options.port || 443,
+          settings: {
+            enablePush: false
+          } })
 
       // check if a socket is supplied
       if (socket) {
@@ -89,6 +91,9 @@ class Http2Agent extends EventEmitter {
     name += ':'
     if (options.ca) { name += options.ca }
 
+    name += ':'
+    if (options.extraCA) { name += options.extraCA }
+
     name += ':'
     if (options.cert) { name += options.cert }
 

lib/http2/request.js

@@ -165,7 +165,7 @@ class ResponseProxy extends EventEmitter {
     this.response = response
     this.on = this.on.bind(this)
     this.registerRequestListeners()
-    this.socket = this.reqStream.session.socket;
+    this.socket = this.reqStream.session.socket
   }
 
   registerRequestListeners () {

lib/redirect.js

@@ -210,7 +210,7 @@ Redirect.prototype.onResponse = function (response) {
 
   request.emit('redirect')
 
-  request.response.once('end', () =>{
+  request.response.once('end', () => {
     request.init(options)
   })
 

request.js

@@ -1278,7 +1278,6 @@ Request.prototype.onRequestResponse = function (response) {
     }
 
     debug('response end', self.uri.href, response.statusCode, response.headers)
-
   })
 
   if (self._aborted) {