fix(node/http): use fake socket and proper url handling (denoland#19340)

Fixes denoland#19349 --------- Co-authored-by: Bartek Iwańczuk <[email protected]>
fanbaby · Jun 6, 2023 · 5aca8b9 · 5aca8b9
1 parent c76f9a0
commit 5aca8b9
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 41 deletions.
diff --git a/cli/tests/unit_node/http_test.ts b/cli/tests/unit_node/http_test.ts
@@ -195,11 +195,14 @@ Deno.test("[node/http] request default protocol", async () => {
  // @ts-ignore IncomingMessageForClient
  // deno-lint-ignore no-explicit-any
  let clientRes: any;
+ // deno-lint-ignore no-explicit-any
+ let clientReq: any;
  server.listen(() => {
- const req = http.request(
+ clientReq = http.request(
  // deno-lint-ignore no-explicit-any
  { host: "localhost", port: (server.address() as any).port },
  (res) => {
+ assert(res.socket instanceof EventEmitter);
  assertEquals(res.complete, false);
  res.on("data", () => {});
  res.on("end", () => {
@@ -210,13 +213,14 @@ Deno.test("[node/http] request default protocol", async () => {
  promise2.resolve();
  },
  );
- req.end();
+ clientReq.end();
  });
  server.on("close", () => {
  promise.resolve();
  });
  await promise;
  await promise2;
+ assert(clientReq.socket instanceof EventEmitter);
  assertEquals(clientRes!.complete, true);
 });
 
@@ -596,3 +600,24 @@ Deno.test("[node/http] ClientRequest PUT", async () => {
  await def;
  assertEquals(body, "hello world");
 });
+
+Deno.test("[node/http] ClientRequest search params", async () => {
+ let body = "";
+ const def = deferred();
+ const req = http.request({
+ host: "localhost:4545",
+ path: "search_params?foo=bar",
+ }, (resp) => {
+ resp.on("data", (chunk) => {
+ body += chunk;
+ });
+
+ resp.on("end", () => {
+ def.resolve();
+ });
+ });
+ req.once("error", (e) => def.reject(e));
+ req.end();
+ await def;
+ assertEquals(body, "foo=bar");
+});
diff --git a/ext/node/polyfills/http.ts b/ext/node/polyfills/http.ts
@@ -267,6 +267,9 @@ const kError = Symbol("kError");
 
 const kUniqueHeaders = Symbol("kUniqueHeaders");
 
+class FakeSocket extends EventEmitter {
+}
+
 /** ClientRequest represents the http(s) request from the client */
 class ClientRequest extends OutgoingMessage {
  defaultProtocol = "http:";
@@ -541,6 +544,7 @@ class ClientRequest extends OutgoingMessage {
  this.onSocket(createConnection(optsWithoutSignal));
  }
  }*/
+ this.onSocket(new FakeSocket());
 
  const url = this._createUrlStrFromOptions();
 
@@ -570,41 +574,12 @@ class ClientRequest extends OutgoingMessage {
  return undefined;
  }
 
- onSocket(socket, err) {
- if (this.destroyed || err) {
- this.destroyed = true;
-
- // deno-lint-ignore no-inner-declarations
- function _destroy(req, err) {
- if (!req.aborted && !err) {
- err = connResetException("socket hang up");
- }
- if (err) {
- req.emit("error", err);
- }
- req._closed = true;
- req.emit("close");
- }
-
- if (socket) {
- if (!err && this.agent && !socket.destroyed) {
- socket.emit("free");
- } else {
- finished(socket.destroy(err || this[kError]), (er) => {
- if (er?.code === "ERR_STREAM_PREMATURE_CLOSE") {
- er = null;
- }
- _destroy(this, er || err);
- });
- return;
- }
- }
-
- _destroy(this, err || this[kError]);
- } else {
- //tickOnSocket(this, socket);
- //this._flush();
- }
+ // TODO(bartlomieju): handle error
+ onSocket(socket, _err) {
+ nextTick(() => {
+ this.socket = socket;
+ this.emit("socket", socket);
+ });
  }
 
  // deno-lint-ignore no-explicit-any
@@ -737,16 +712,19 @@ class ClientRequest extends OutgoingMessage {
  const auth = this.auth;
  const host = this.host ?? this.hostname ?? "localhost";
  const hash = this.hash ? `#${this.hash}` : "";
- const search = this.search ? this.search : "";
  const defaultPort = this.agent?.defaultPort;
  const port = this.port ?? defaultPort ?? 80;
  let path = this.path ?? "/";
  if (!path.startsWith("/")) {
  path = "/" + path;
  }
- return `${protocol}//${auth ? `${auth}@` : ""}${host}${
- port === 80 ? "" : `:${port}`
- }${path}${search}${hash}`;
+ const url = new URL(
+ `${protocol}//${auth ? `${auth}@` : ""}${host}${
+ port === 80 ? "" : `:${port}`
+ }${path}`,
+ );
+ url.hash = hash;
+ return url.href;
  }
 
  setTimeout(msecs: number, callback?: () => void) {

diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs
@@ -1085,6 +1085,11 @@ async fn main_server(
  ));
  Ok(res)
  }
+ (_, "/search_params") => {
+ let query = req.uri().query().map(|s| s.to_string());
+ let res = Response::new(Body::from(query.unwrap_or_default()));
+ Ok(res)
+ }
  _ => {
  let mut file_path = testdata_path();
  file_path.push(&req.uri().path()[1..]);