node:https: provide proper Agent definition (#11826)

Co-authored-by: Jarred Sumner <[email protected]>
oven-sh · Sep 6, 2024 · ed7741a · ed7741a
1 parent 9adf42b
commit ed7741a
Show file tree

Hide file tree

Showing 12 changed files with 191 additions and 121 deletions.
diff --git a/src/bun.js/bindings/ErrorCode.cpp b/src/bun.js/bindings/ErrorCode.cpp
@@ -69,6 +69,15 @@ extern "C" JSC::EncodedJSValue Bun__ERR_IPC_CHANNEL_CLOSED(JSC::JSGlobalObject*
 
 // clang-format on
 
+#define EXPECT_ARG_COUNT(count__)                                                          \
+    do {                                                                                   \
+        auto argCount = callFrame->argumentCount();                                        \
+        if (argCount < count__) {                                                          \
+            JSC::throwTypeError(globalObject, scope, "requires " #count__ " arguments"_s); \
+            return {};                                                                     \
+        }                                                                                  \
+    } while (false)
+
 namespace Bun {
 
 using namespace JSC;
@@ -196,11 +205,8 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_INVALID_ARG_TYPE, (JSC::JSGlobalObject *
     JSC::VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    auto argCount = callFrame->argumentCount();
-    if (argCount < 3) {
-        JSC::throwTypeError(globalObject, scope, "requires 3 arguments"_s);
-        return {};
-    }
+    EXPECT_ARG_COUNT(3);
+
     auto arg_name = callFrame->argument(0);
     auto expected_type = callFrame->argument(1);
     auto actual_value = callFrame->argument(2);
@@ -246,11 +252,7 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_OUT_OF_RANGE, (JSC::JSGlobalObject * glo
     JSC::VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    auto argCount = callFrame->argumentCount();
-    if (argCount < 3) {
-        JSC::throwTypeError(globalObject, scope, "requires 3 arguments"_s);
-        return {};
-    }
+    EXPECT_ARG_COUNT(3);
 
     auto arg_name = callFrame->argument(0).toWTFString(globalObject);
     RETURN_IF_EXCEPTION(scope, {});
@@ -320,7 +322,7 @@ extern "C" JSC::EncodedJSValue Bun__ERR_MISSING_ARGS_static(JSC::JSGlobalObject*
     JSC::VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    if (arg1 == 0) {
+    if (arg1 == nullptr) {
         JSC::throwTypeError(globalObject, scope, "requires at least 1 argument"_s);
         return {};
     }
@@ -365,6 +367,23 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_SOCKET_BAD_TYPE, (JSC::JSGlobalObject *
     return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_SOCKET_BAD_TYPE, "Bad socket type specified. Valid types are: udp4, udp6"_s));
 }
 
+JSC_DEFINE_HOST_FUNCTION(jsFunction_ERR_INVALID_PROTOCOL, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
+{
+    JSC::VM& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    EXPECT_ARG_COUNT(2);
+
+    auto actual = callFrame->argument(0).toWTFString(globalObject);
+    RETURN_IF_EXCEPTION(scope, {});
+
+    auto expected = callFrame->argument(1).toWTFString(globalObject);
+    RETURN_IF_EXCEPTION(scope, {});
+
+    auto message = makeString("Protocol \""_s, actual, "\" not supported. Expected \""_s, expected, "\""_s);
+    return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_INVALID_PROTOCOL, message));
+}
+
 } // namespace Bun
 
 JSC::JSValue WebCore::toJS(JSC::JSGlobalObject* globalObject, CommonAbortReason abortReason)

diff --git a/src/bun.js/bindings/ErrorCode.h b/src/bun.js/bindings/ErrorCode.h
@@ -61,5 +61,6 @@ JSC_DECLARE_HOST_FUNCTION(jsFunction_ERR_IPC_DISCONNECTED);
 JSC_DECLARE_HOST_FUNCTION(jsFunction_ERR_SERVER_NOT_RUNNING);
 JSC_DECLARE_HOST_FUNCTION(jsFunction_ERR_IPC_CHANNEL_CLOSED);
 JSC_DECLARE_HOST_FUNCTION(jsFunction_ERR_SOCKET_BAD_TYPE);
+JSC_DECLARE_HOST_FUNCTION(jsFunction_ERR_INVALID_PROTOCOL);
 
 }
diff --git a/src/bun.js/bindings/ErrorCode.ts b/src/bun.js/bindings/ErrorCode.ts
@@ -18,6 +18,7 @@ export default [
   ["ERR_HTTP2_INVALID_SINGLE_VALUE_HEADER", TypeError, "TypeError"],
   ["ERR_INVALID_ARG_TYPE", TypeError, "TypeError"],
   ["ERR_INVALID_ARG_VALUE", TypeError, "TypeError"],
+  ["ERR_INVALID_PROTOCOL", TypeError, "TypeError"],
   ["ERR_INVALID_THIS", TypeError, "TypeError"],
   ["ERR_IPC_CHANNEL_CLOSED", Error, "Error"],
   ["ERR_IPC_DISCONNECTED", Error, "Error"],
@@ -33,10 +34,11 @@ export default [
   ["ERR_STREAM_DESTROYED", TypeError, "TypeError"],
   ["ERR_STREAM_NULL_VALUES", TypeError, "TypeError"],
   ["ERR_STREAM_WRITE_AFTER_END", TypeError, "TypeError"],
-  ["ERR_ZLIB_INITIALIZATION_FAILED", TypeError, "TypeError"],
   ["ERR_STRING_TOO_LONG", Error, "Error"],
+  ["ERR_ZLIB_INITIALIZATION_FAILED", TypeError, "TypeError"],
   ["ERR_ILLEGAL_CONSTRUCTOR", TypeError, "TypeError"],
   ["ERR_INVALID_URL", TypeError, "TypeError"],
+
   // Bun-specific
   ["ERR_FORMDATA_PARSE_ERROR", TypeError, "TypeError"],
   ["ERR_BODY_ALREADY_USED", Error, "Error"],

diff --git a/src/js/internal/cluster/Worker.ts b/src/js/internal/cluster/Worker.ts
@@ -23,8 +23,7 @@ function Worker(options) {
     this.process.on("message", (message, handle) => this.emit("message", message, handle));
   }
 }
-Worker.prototype = {};
-ObjectSetPrototypeOf(Worker.prototype, EventEmitter.prototype);
+Worker.prototype = Object.create(EventEmitter.prototype);
 
 Worker.prototype.kill = function () {
   this.destroy.$apply(this, arguments);

diff --git a/src/js/internal/errors.ts b/src/js/internal/errors.ts
@@ -5,4 +5,5 @@ export default {
   ERR_SERVER_NOT_RUNNING: $newCppFunction("ErrorCode.cpp", "jsFunction_ERR_SERVER_NOT_RUNNING", 0),
   ERR_IPC_CHANNEL_CLOSED: $newCppFunction("ErrorCode.cpp", "jsFunction_ERR_IPC_CHANNEL_CLOSED", 0),
   ERR_SOCKET_BAD_TYPE: $newCppFunction("ErrorCode.cpp", "jsFunction_ERR_SOCKET_BAD_TYPE", 0),
+  ERR_INVALID_PROTOCOL: $newCppFunction("ErrorCode.cpp", "jsFunction_ERR_INVALID_PROTOCOL", 0),
 };
diff --git a/src/js/internal/url.ts b/src/js/internal/url.ts
@@ -0,0 +1,23 @@
+function urlToHttpOptions(url) {
+  const options = {
+    protocol: url.protocol,
+    hostname:
+      typeof url.hostname === "string" && url.hostname.startsWith("[") ? url.hostname.slice(1, -1) : url.hostname,
+    hash: url.hash,
+    search: url.search,
+    pathname: url.pathname,
+    path: `${url.pathname || ""}${url.search || ""}`,
+    href: url.href,
+  };
+  if (url.port !== "") {
+    options.port = Number(url.port);
+  }
+  if (url.username || url.password) {
+    options.auth = `${decodeURIComponent(url.username)}:${decodeURIComponent(url.password)}`;
+  }
+  return options;
+}
+
+export default {
+  urlToHttpOptions,
+};
diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts
@@ -862,8 +862,7 @@ function ReadStream(this: typeof ReadStream, pathOrFd, options) {
   $assert(overridden_fs);
   this[kFs] = overridden_fs;
 }
-ReadStream.prototype = {};
-ObjectSetPrototypeOf(ReadStream.prototype, NativeReadable.prototype);
+ReadStream.prototype = Object.create(NativeReadable.prototype);
 
 ReadStream.prototype._construct = function (callback) {
   if (NativeReadablePrototype._construct) {

diff --git a/src/js/node/http.ts b/src/js/node/http.ts
@@ -2,9 +2,10 @@
 const EventEmitter = require("node:events");
 const { isTypedArray } = require("node:util/types");
 const { Duplex, Readable, Writable } = require("node:stream");
-const { ERR_INVALID_ARG_TYPE } = require("internal/errors");
+const { ERR_INVALID_ARG_TYPE, ERR_INVALID_PROTOCOL } = require("internal/errors");
 const { isPrimary } = require("internal/cluster/isPrimary");
 const { kAutoDestroyed } = require("internal/shared");
+const { urlToHttpOptions } = require("internal/url");
 
 const {
   getHeader,
@@ -97,7 +98,6 @@ const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
 const NODE_HTTP_WARNING =
   "WARN: Agent is mostly unused in Bun's implementation of http. If you see strange behavior, this is probably the cause.";
 
-var _defaultHTTPSAgent;
 var kInternalRequest = Symbol("kInternalRequest");
 const kInternalSocketData = Symbol.for("::bunternal::");
 const serverSymbol = Symbol.for("::bunternal::");
@@ -273,8 +273,7 @@ function Agent(options = kEmptyObject) {
   this.defaultPort = options.defaultPort || 80;
   this.protocol = options.protocol || "http:";
 }
-Agent.prototype = {};
-$setPrototypeDirect.$call(Agent.prototype, EventEmitter.prototype);
+Agent.prototype = Object.create(EventEmitter.prototype);
 
 ObjectDefineProperty(Agent, "globalAgent", {
   get: function () {
@@ -747,10 +746,6 @@ function assignHeaders(object, req) {
 
 var defaultIncomingOpts = { type: "request" };
 
-function getDefaultHTTPSAgent() {
-  return (_defaultHTTPSAgent ??= new Agent({ defaultPort: 443, protocol: "https:" }));
-}
-
 function requestHasNoBody(method, req) {
   if ("GET" === method || "HEAD" === method || "TRACE" === method || "CONNECT" === method || "OPTIONS" === method)
     return true;
@@ -1654,38 +1649,26 @@ class ClientRequest extends OutgoingMessage {
       options = ObjectAssign(input || {}, options);
     }
 
-    var defaultAgent = options._defaultAgent || Agent.globalAgent;
-
-    let protocol = options.protocol;
-    if (!protocol) {
-      if (options.port === 443) {
-        protocol = "https:";
-      } else {
-        protocol = defaultAgent.protocol || "http:";
-      }
+    let agent = options.agent;
+    const defaultAgent = options._defaultAgent || Agent.globalAgent;
+    if (agent === false) {
+      agent = new defaultAgent.constructor();
+    } else if (agent == null) {
+      agent = defaultAgent;
+    } else if (typeof agent.addRequest !== "function") {
+      throw ERR_INVALID_ARG_TYPE("options.agent", "Agent-like Object, undefined, or false", agent);
     }
-    this.#protocol = protocol;
+    this.#agent = agent;
 
-    switch (this.#agent?.protocol) {
-      case undefined: {
-        break;
-      }
-      case "http:": {
-        if (protocol === "https:") {
-          defaultAgent = this.#agent = getDefaultHTTPSAgent();
-          break;
-        }
-      }
-      case "https:": {
-        if (protocol === "https") {
-          defaultAgent = this.#agent = Agent.globalAgent;
-          break;
-        }
-      }
-      default: {
-        break;
-      }
+    const protocol = options.protocol || defaultAgent.protocol;
+    let expectedProtocol = defaultAgent.protocol;
+    if (this.agent.protocol) {
+      expectedProtocol = this.agent.protocol;
     }
+    if (protocol !== expectedProtocol) {
+      throw ERR_INVALID_PROTOCOL(protocol, expectedProtocol);
+    }
+    this.#protocol = protocol;
 
     if (options.path) {
       const path = String(options.path);
@@ -1696,16 +1679,8 @@ class ClientRequest extends OutgoingMessage {
       }
     }
 
-    // Since we don't implement Agent, we don't need this
-    if (protocol !== "http:" && protocol !== "https:" && protocol) {
-      const expectedProtocol = defaultAgent?.protocol ?? "http:";
-      throw new Error(`Protocol mismatch. Expected: ${expectedProtocol}. Got: ${protocol}`);
-      // throw new ERR_INVALID_PROTOCOL(protocol, expectedProtocol);
-    }
-
-    const defaultPort = protocol === "https:" ? 443 : 80;
-
-    this.#port = options.port || options.defaultPort || this.#agent?.defaultPort || defaultPort;
+    const defaultPort = options.defaultPort || this.#agent.defaultPort;
+    this.#port = options.port || defaultPort || 80;
     this.#useDefaultPort = this.#port === defaultPort;
     const host =
       (this.#host =
@@ -1952,24 +1927,6 @@ class ClientRequest extends OutgoingMessage {
   }
 }
 
-function urlToHttpOptions(url) {
-  var { protocol, hostname, hash, search, pathname, href, port, username, password } = url;
-  return {
-    protocol,
-    hostname:
-      typeof hostname === "string" && StringPrototypeStartsWith.$call(hostname, "[")
-        ? StringPrototypeSlice.$call(hostname, 1, -1)
-        : hostname,
-    hash,
-    search,
-    pathname,
-    path: `${pathname || ""}${search || ""}`,
-    href,
-    port: port ? Number(port) : protocol === "https:" ? 443 : protocol === "http:" ? 80 : undefined,
-    auth: username || password ? `${decodeURIComponent(username)}:${decodeURIComponent(password)}` : undefined,
-  };
-}
-
 function validateHost(host, name) {
   if (host !== null && host !== undefined && typeof host !== "string") {
     // throw ERR_INVALID_ARG_TYPE(

diff --git a/src/js/node/https.ts b/src/js/node/https.ts
@@ -1,14 +1,30 @@
 // Hardcoded module "node:https"
 const http = require("node:http");
+const { urlToHttpOptions } = require("internal/url");
 
-function request(input, options, cb) {
-  if (input && typeof input === "object" && !(input instanceof URL)) {
-    input.protocol ??= "https:";
-  } else if (typeof options === "object") {
-    options.protocol ??= "https:";
+const ObjectSetPrototypeOf = Object.setPrototypeOf;
+const ArrayPrototypeShift = Array.prototype.shift;
+const ObjectAssign = Object.assign;
+const ArrayPrototypeUnshift = Array.prototype.unshift;
+
+function request(...args) {
+  let options = {};
+
+  if (typeof args[0] === "string") {
+    const urlStr = ArrayPrototypeShift.$call(args);
+    options = urlToHttpOptions(new URL(urlStr));
+  } else if (args[0] instanceof URL) {
+    options = urlToHttpOptions(ArrayPrototypeShift.$call(args));
+  }
+
+  if (args[0] && typeof args[0] !== "function") {
+    ObjectAssign.$call(null, options, ArrayPrototypeShift.$call(args));
   }
 
-  return http.request(input, options, cb);
+  options._defaultAgent = https.globalAgent;
+  ArrayPrototypeUnshift.$call(args, options);
+
+  return new http.ClientRequest(...args);
 }
 
 function get(input, options, cb) {
@@ -17,8 +33,24 @@ function get(input, options, cb) {
   return req;
 }
 
-export default {
-  ...http,
+function Agent(options) {
+  if (!(this instanceof Agent)) return new Agent(options);
+
+  http.Agent.$apply(this, [options]);
+  this.defaultPort = 443;
+  this.protocol = "https:";
+  this.maxCachedSessions = this.options.maxCachedSessions;
+  if (this.maxCachedSessions === undefined) this.maxCachedSessions = 100;
+}
+Agent.prototype = Object.create(http.Agent.prototype);
+Agent.prototype.createConnection = http.createConnection;
+
+var https = {
+  Agent,
+  globalAgent: new Agent({ keepAlive: true, scheduling: "lifo", timeout: 5000 }),
+  Server: http.Server,
+  createServer: http.createServer,
   get,
   request,
 };
+export default https;
diff --git a/src/js/node/url.ts b/src/js/node/url.ts
@@ -25,6 +25,7 @@
 
 const { URL, URLSearchParams } = globalThis;
 const [domainToASCII, domainToUnicode] = $cpp("NodeURL.cpp", "Bun::createNodeURLBinding");
+const { urlToHttpOptions } = require("internal/url");
 
 function Url() {
   this.protocol = null;
@@ -803,25 +804,6 @@ Url.prototype.parseHost = function () {
     this.hostname = host;
   }
 };
-function urlToHttpOptions(url) {
-  const options = {
-    protocol: url.protocol,
-    hostname:
-      typeof url.hostname === "string" && url.hostname.startsWith("[") ? url.hostname.slice(1, -1) : url.hostname,
-    hash: url.hash,
-    search: url.search,
-    pathname: url.pathname,
-    path: `${url.pathname || ""}${url.search || ""}`,
-    href: url.href,
-  };
-  if (url.port !== "") {
-    options.port = Number(url.port);
-  }
-  if (url.username || url.password) {
-    options.auth = `${decodeURIComponent(url.username)}:${decodeURIComponent(url.password)}`;
-  }
-  return options;
-}
 
 export default {
   parse: urlParse,