Exposed socket in http2 response to handle false positive strictSSL e…

…rror * Refactoring changes
postmanlabs · Jul 10, 2024 · ffd8667 · ffd8667
1 parent 859226d
commit ffd8667
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 11 deletions.
diff --git a/lib/autohttp/agent.js b/lib/autohttp/agent.js
@@ -5,6 +5,7 @@ const { EventEmitter } = require('events')
 const net = require('net')
 const url = require('url')
 
+// Referenced from https://github.com/nodejs/node/blob/0bf200b49a9a6eacdea6d5e5939cc2466506d532/lib/_http_agent.js#L350
 function calculateServerName (options) {
   let servername = options.host || ''
   const hostHeader = options.headers && options.headers.host
@@ -114,11 +115,19 @@ class AutoHttp2Agent extends EventEmitter {
 
     const socket = tls.connect(newOptions)
     socketCb(socket)
-    socket.on('error', (e) => cb(e))
+
+    const socketConnnectionErrorHandler = (e) => {
+      cb(e)
+    }
+    socket.on('error', socketConnnectionErrorHandler)
+
     socket.once('secureConnect', () => {
+      socket.removeListener('error', socketConnnectionErrorHandler)
+
       const protocol = socket.alpnProtocol
+
       if (!protocol) {
-        cb(socket.authorizationError, undefined, undefined)
+        cb(socket.authorizationError)
         socket.end()
         return
       }

diff --git a/lib/autohttp/utils/headers.js → lib/autohttp/headerValidations.js b/lib/autohttp/utils/headers.js → lib/autohttp/headerValidations.js
@@ -1,9 +1,6 @@
-/*
- * The following code has been borrowed from the Node.js project
- * v20.15.0 /Users/parth.verma@postman.com/node/lib/internal/http2/compat.js
- */
 const {constants = {}} = require('http2')
 
+// Referenced from https://github.com/nodejs/node/blob/0bf200b49a9a6eacdea6d5e5939cc2466506d532/lib/internal/http2/util.js#L107
 const kValidPseudoHeaders = new Set([
   constants.HTTP2_HEADER_STATUS,
   constants.HTTP2_HEADER_METHOD,
@@ -12,17 +9,20 @@ const kValidPseudoHeaders = new Set([
   constants.HTTP2_HEADER_PATH
 ])
 
+// Referenced from https://github.com/nodejs/node/blob/0bf200b49a9a6eacdea6d5e5939cc2466506d532/lib/internal/http2/util.js#L573
 function assertValidPseudoHeader (header) {
   if (!kValidPseudoHeaders.has(header)) {
     throw new Error('Invalid PseudoHeader ' + header)
   }
 }
 
+// Referenced from https://github.com/nodejs/node/blob/0bf200b49a9a6eacdea6d5e5939cc2466506d532/lib/_http_common.js#L206
 const tokenRegExp = /^[\^_`a-zA-Z\-0-9!#$%&'*+.|~]+$/
 function checkIsHttpToken (token) {
   return RegExp(tokenRegExp).exec(token) !== null
 }
 
+
 module.exports = {
   assertValidPseudoHeader,
   checkIsHttpToken

diff --git a/lib/autohttp/request.js b/lib/autohttp/request.js
@@ -1,7 +1,7 @@
 const { EventEmitter } = require('events')
 const { Http2Request: HTTP2Request } = require('../../lib/http2/request')
 const { globalAgent } = require('./agent')
-const { assertValidPseudoHeader, checkIsHttpToken } = require('./utils/headers')
+const { assertValidPseudoHeader, checkIsHttpToken } = require('./headerValidations')
 
 const kJobs = Symbol('kJobs')
 
@@ -119,7 +119,7 @@ class MultiProtocolRequest extends EventEmitter {
     if (this._req && this._req._header) {
       return this._req._header
     }
-    return new Promise((resolve, reject) => {
+    return new Promise((resolve) => {
       const action = () => resolve(this._req._header)
       this[kJobs].push(action)
     })

diff --git a/lib/http2/agent.js b/lib/http2/agent.js
@@ -15,7 +15,9 @@ class Http2Agent extends EventEmitter {
 
     if (!connection || connection.destroyed || connection.closed) {
       const connectionOptions = Object.assign({}, options,
-        { createConnection: undefined, port: _options.port || 443 })
+        { createConnection: undefined, port: _options.port || 443, settings: {
+          enablePush: false
+        } })
 
       // check if a socket is supplied
       if (socket) {
@@ -54,8 +56,9 @@ class Http2Agent extends EventEmitter {
       }
 
       // Add a default error listener to HTTP2 session object to transparently swallow errors incase no streams are active
+      // Remove the connection from the connections map if the connection has errored out
       connection.on('error', () => {
-        // Do nothing
+        delete connectionsMap[name]
       })
 
       this.connections[name] = connection

diff --git a/lib/http2/request.js b/lib/http2/request.js
@@ -2,7 +2,7 @@ const url = require('url')
 const http2 = require('http2')
 const { EventEmitter } = require('events')
 const { globalAgent } = require('./agent')
-const { assertValidPseudoHeader, checkIsHttpToken } = require('../autohttp/utils/headers')
+const { assertValidPseudoHeader, checkIsHttpToken } = require('../autohttp/headerValidations')
 
 function httpOptionsToUri (options) {
   return url.format({
@@ -165,6 +165,7 @@ class ResponseProxy extends EventEmitter {
     this.response = response
     this.on = this.on.bind(this)
     this.registerRequestListeners()
+    this.socket = this.reqStream.session.socket;
   }
 
   registerRequestListeners () {