From a696dd9879ff2bf5a5db6ec57a824944fa56ded8 Mon Sep 17 00:00:00 2001
From: Trent Mick <trent.mick@elastic.co>
Date: Sat, 29 Jul 2023 12:38:30 +0200
Subject: [PATCH] fix: shimming of `http.request()` with node v18.17.0 caused a
 subtle edge case breakage (#3511)

Before this change, a call with a non-Function callback:

    http.request(urlString, {}, 'this-is-not-a-cb-function')

would accidentally *not* fail because of the agent's instrumentation.
This is because the Node.js internal `isURL()` check changed such that
the result of `urlToHttpOptions(url)` passes `isURL()`.
---
 CHANGELOG.asciidoc                 | 4 ++++
 lib/instrumentation/http-shared.js | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 067c57bcec..efc5b03af2 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -43,6 +43,10 @@ Notes:
 [float]
 ===== Bug fixes
 
+* Fix wrapping of `http.request()` for node v18.17.0. Before this change, a
+  call with a non-Function callback -- `http.request(urlString, {}, 'this-is-not-a-cb-function')`
+  -- would accidentally *not* fail because of the agent's instrumentation.
+
 * Fix tedious instrumentation to recognize "connection.prepare()" usage in
   tedious@16.2.0 and later.
 
diff --git a/lib/instrumentation/http-shared.js b/lib/instrumentation/http-shared.js
index 96b8f1a169..a219f80dde 100644
--- a/lib/instrumentation/http-shared.js
+++ b/lib/instrumentation/http-shared.js
@@ -55,13 +55,16 @@ if (!urlToHttpOptions) {
     }
     return options
   }
-} else if (semver.satisfies(process.version, '>=19.9.0 <20')) {
+} else if (semver.satisfies(process.version, '>=19.9.0 <20') ||
+    semver.satisfies(process.version, '>=18.17.0 <19')) {
   // Starting in node v19.9.0 (as of https://github.com/nodejs/node/pull/46989)
   // `urlToHttpOptions(url)` returns an object which is considered a `url`
   // argument by `http.request()` -- because of the internal `isURL(url)` test.
+  // Starting with node v18.17.0, the same is true with the internal switch
+  // to the "Ada" lib for URL parsing.
   safeUrlToHttpOptions = function (url) {
     const options = urlToHttpOptions(url)
-    // Specically we are dropping the `Symbol(context)` field.
+    // Specifically we are dropping the `Symbol(context)` field.
     Object.getOwnPropertySymbols(options).forEach(sym => {
       delete options[sym]
     })