From 44a89dd6dc7864822ddb48d030af519160de90a2 Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Tue, 18 Oct 2022 11:28:27 +0900 Subject: fix(ext/net): return an error from `startTls` and `serveHttp` if the original connection is captured elsewhere (#16242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit removes the calls to `expect()` on `std::rc::Rc`, which caused Deno to panic under certain situations. We now return an error if `Rc` is referenced by other variables. Fixes #9360 Fixes #13345 Fixes #13926 Fixes #16241 Co-authored-by: Bartek Iwańczuk --- ext/net/lib.deno_net.d.ts | 9 +++++++++ ext/net/ops_tls.rs | 6 +++++- 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'ext') diff --git a/ext/net/lib.deno_net.d.ts b/ext/net/lib.deno_net.d.ts index d6c7d4afd..df569f93a 100644 --- a/ext/net/lib.deno_net.d.ts +++ b/ext/net/lib.deno_net.d.ts @@ -253,9 +253,18 @@ declare namespace Deno { * this function requires that the other end of the connection is prepared for * a TLS handshake. * + * Note that this function *consumes* the TCP connection passed to it, thus the + * original TCP connection will be unusable after calling this. Additionally, + * you need to ensure that the TCP connection is not being used elsewhere when + * calling this function in order for the TCP connection to be consumed properly. + * For instance, if there is a `Promise` that is waiting for read operation on + * the TCP connection to complete, it is considered that the TCP connection is + * being used elsewhere. In such a case, this function will fail. + * * ```ts * const conn = await Deno.connect({ port: 80, hostname: "127.0.0.1" }); * const caCert = await Deno.readTextFile("./certs/my_custom_root_CA.pem"); + * // `conn` becomes unusable after calling `Deno.startTls` * const tlsConn = await Deno.startTls(conn, { caCerts: [caCert], hostname: "localhost" }); * ``` * diff --git a/ext/net/ops_tls.rs b/ext/net/ops_tls.rs index a1b48b84e..a59cd747e 100644 --- a/ext/net/ops_tls.rs +++ b/ext/net/ops_tls.rs @@ -812,12 +812,16 @@ where .borrow::() .root_cert_store .clone(); + let resource_rc = state .borrow_mut() .resource_table .take::(rid)?; + // This TCP connection might be used somewhere else. If it's the case, we cannot proceed with the + // process of starting a TLS connection on top of this TCP connection, so we just return a bad + // resource error. See also: https://github.com/denoland/deno/pull/16242 let resource = Rc::try_unwrap(resource_rc) - .expect("Only a single use of this resource should happen"); + .map_err(|_| bad_resource("TCP stream is currently in use"))?; let (read_half, write_half) = resource.into_inner(); let tcp_stream = read_half.reunite(write_half)?; -- cgit v1.2.3