From dc71a3b3e4f49b7935f084644f47d62219bdfba0 Mon Sep 17 00:00:00 2001
From: Paul Melnikow <email@paulmelnikow.com>
Date: Fri, 18 Jan 2019 15:27:46 -0500
Subject: [PATCH] feat(request_overrider): Set `IncomingMessage.client` for
 parity with real requests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`ClientMessage.socket` is an instance of our `socket` class. As that class’ constructor sets `authorized` to `true` on `https` requests, the code here that sets `response.socket.authorized` is pointless.

`ClientMessage.client` was deprecated in iojs 2.2 and per https://github.com/request/request/pull/1615  but is still aliased: https://github.com/nodejs/node/blob/2e613a9c301165d121b19b86e382860323abc22f/lib/_http_incoming.js#L67

The old code, which sets `response.client.authorized`, is being replaced with new code which aliases `response.client`. This ensures that `response.client` and `response.socket` point to the same thing, which provides parity with a non-overridden request. This may help with compatibility with libraries which depend on that undocumented property, such as some very old versions of `request`.

This also ensures that `response.client.authorized` remains set, without needing to set it directly.

This is being changed now to remove two bits of unreachable code in the conditional.
---
 lib/request_overrider.js | 22 +++++++++++-----------
 lib/socket.js            |  1 +
 tests/test_intercept.js  |  1 +
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/lib/request_overrider.js b/lib/request_overrider.js
index b0d55c7f9..3114c30c8 100644
--- a/lib/request_overrider.js
+++ b/lib/request_overrider.js
@@ -471,17 +471,17 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
         return
       }
 
-      /// response.client.authorized = true
-      /// fixes https://github.com/pgte/nock/issues/158
-      response.client = _.extend(response.client || {}, {
-        authorized: true,
-      })
-
-      // Account for updates to Node.js response interface
-      // cf https://github.com/request/request/pull/1615
-      response.socket = _.extend(response.socket || {}, {
-        authorized: true,
-      })
+      // `IncomingMessage.client` is an undocumented alias for
+      // `IncomingMessage.socket`. Assigning it here may help with
+      // compatibility, including with very old versions of `request` which
+      // inspect `response.client.authorized`. Modern versions of request
+      // inspect `response.socket.authorized` which is set to true in our
+      // `Socket` constructor.
+      // https://github.com/pgte/nock/issues/158
+      // https://github.com/request/request/pull/1615
+      // https://nodejs.org/api/http.html#http_response_socket
+      // https://github.com/nodejs/node/blob/2e613a9c301165d121b19b86e382860323abc22f/lib/_http_incoming.js#L67
+      response.client = response.socket
 
       // Evaluate functional headers.
       const evaluatedHeaders = {}
diff --git a/lib/socket.js b/lib/socket.js
index c63de305f..34911ad0f 100644
--- a/lib/socket.js
+++ b/lib/socket.js
@@ -14,6 +14,7 @@ function Socket(options) {
   EventEmitter.apply(this)
 
   if (options.proto === 'https') {
+    // https://github.com/nock/nock/issues/158
     this.authorized = true
   }
 
diff --git a/tests/test_intercept.js b/tests/test_intercept.js
index 4db3b6b7e..4b73cef8d 100644
--- a/tests/test_intercept.js
+++ b/tests/test_intercept.js
@@ -3828,6 +3828,7 @@ test('mocking succeeds even when host request header is not specified', t => {
   )
 })
 
+// https://github.com/nock/nock/issues/158
 test('mikeal/request with strictSSL: true', t => {
   nock('https://strictssl.com')
     .post('/what')