Use tiny_http for handling login HTTP request

Max Brunsfeld and Antonio Scandurra created

Our manual HTTP parsing and encoding was not working with Safari.

Co-Authored-By: Antonio Scandurra <me@as-cii.com>

Change summary

Cargo.lock           | 26 ++++++++++++
gpui/src/executor.rs |  3 
zed-rpc/src/proto.rs | 39 +++++++++---------
zed/Cargo.toml       |  1 
zed/src/lib.rs       | 96 +++++++++++++++++++--------------------------
5 files changed, 88 insertions(+), 77 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -117,6 +117,12 @@ version = "0.5.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b"
 
+[[package]]
+name = "ascii"
+version = "1.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bbf56136a5198c7b01a49e3afcbef6cf84597273d298f54432926024107b0109"
+
 [[package]]
 name = "async-channel"
 version = "1.6.1"
@@ -558,6 +564,12 @@ dependencies = [
  "winapi 0.3.9",
 ]
 
+[[package]]
+name = "chunked_transfer"
+version = "1.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "fff857943da45f546682664a79488be82e69e43c1a7a2307679ab9afb3a66d2e"
+
 [[package]]
 name = "cipher"
 version = "0.2.5"
@@ -3773,6 +3785,19 @@ dependencies = [
  "safe_arch",
 ]
 
+[[package]]
+name = "tiny_http"
+version = "0.8.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9ce51b50006056f590c9b7c3808c3bd70f0d1101666629713866c227d6e58d39"
+dependencies = [
+ "ascii",
+ "chrono",
+ "chunked_transfer",
+ "log",
+ "url",
+]
+
 [[package]]
 name = "tinyvec"
 version = "1.2.0"
@@ -4301,6 +4326,7 @@ dependencies = [
  "smol",
  "surf",
  "tempdir",
+ "tiny_http",
  "toml 0.5.8",
  "tree-sitter",
  "tree-sitter-rust",

gpui/src/executor.rs 🔗

@@ -79,9 +79,10 @@ impl Background {
         }
     }
 
-    pub fn spawn<T>(&self, future: impl Send + Future<Output = T> + 'static) -> Task<T>
+    pub fn spawn<T, F>(&self, future: F) -> Task<T>
     where
         T: 'static + Send,
+        F: Send + Future<Output = T> + 'static,
     {
         self.executor.spawn(future)
     }

zed-rpc/src/proto.rs 🔗

@@ -116,8 +116,6 @@ mod tests {
                 chunk_size: 3,
             };
 
-            // In reality there will never be both `FromClient` and `FromServer` messages
-            // sent in the same direction on the same stream.
             let message1 = FromClient {
                 id: 3,
                 variant: Some(from_client::Variant::Auth(from_client::Auth {
@@ -125,16 +123,15 @@ mod tests {
                     access_token: "the-access-token".into(),
                 })),
             };
-            let message2 = FromServer {
-                request_id: Some(4),
-                variant: Some(from_server::Variant::Ack(from_server::Ack {
-                    error_message: Some(
-                        format!(
-                            "a {}long error message that requires a two-byte length delimiter",
-                            "very ".repeat(60)
-                        )
-                        .into(),
-                    ),
+            let message2 = FromClient {
+                id: 4,
+                variant: Some(from_client::Variant::UploadFile(from_client::UploadFile {
+                    path: Vec::new(),
+                    content: format!(
+                        "a {}long error message that requires a two-byte length delimiter",
+                        "very ".repeat(60)
+                    )
+                    .into(),
                 })),
             };
 
@@ -142,7 +139,7 @@ mod tests {
             message_stream.write_message(&message1).await.unwrap();
             message_stream.write_message(&message2).await.unwrap();
             let decoded_message1 = message_stream.read_message::<FromClient>().await.unwrap();
-            let decoded_message2 = message_stream.read_message::<FromServer>().await.unwrap();
+            let decoded_message2 = message_stream.read_message::<FromClient>().await.unwrap();
             assert_eq!(decoded_message1, message1);
             assert_eq!(decoded_message2, message2);
         });
@@ -159,17 +156,18 @@ mod tests {
 
             // This message is so long that its length delimiter requires three bytes,
             // so it won't be delivered in a single read from the chunked byte stream.
-            let message = FromServer {
-                request_id: Some(4),
-                variant: Some(from_server::Variant::Ack(from_server::Ack {
-                    error_message: Some("long ".repeat(256 * 256).into()),
+            let message = FromClient {
+                id: 4,
+                variant: Some(from_client::Variant::UploadFile(from_client::UploadFile {
+                    path: Vec::new(),
+                    content: "long ".repeat(256 * 256).into(),
                 })),
             };
             assert!(prost::length_delimiter_len(message.encoded_len()) > byte_stream.chunk_size);
 
             let mut message_stream = MessageStream::new(byte_stream);
             message_stream.write_message(&message).await.unwrap();
-            let decoded_message = message_stream.read_message::<FromServer>().await.unwrap();
+            let decoded_message = message_stream.read_message::<FromClient>().await.unwrap();
             assert_eq!(decoded_message, message);
         });
     }
@@ -177,7 +175,7 @@ mod tests {
     #[test]
     fn test_protobuf_parse_error() {
         smol::block_on(async {
-            let byte_stream = ChunkedStream {
+            let mut byte_stream = ChunkedStream {
                 bytes: Vec::new(),
                 read_offset: 0,
                 chunk_size: 2,
@@ -191,12 +189,13 @@ mod tests {
                 })),
             };
 
+            byte_stream.write_all(b"omg").await.unwrap();
             let mut message_stream = MessageStream::new(byte_stream);
             message_stream.write_message(&message).await.unwrap();
 
             // Read the wrong type of message from the stream.
             let result = message_stream.read_message::<FromServer>().await;
-            assert_eq!(result.unwrap_err().kind(), io::ErrorKind::InvalidData);
+            assert!(result.is_err());
         });
     }
 

zed/Cargo.toml 🔗

@@ -41,6 +41,7 @@ simplelog = "0.9"
 smallvec = { version = "1.6", features = ["union"] }
 smol = "1.2.5"
 surf = "2.2"
+tiny_http = "0.8"
 toml = "0.5"
 tree-sitter = "0.19.5"
 tree-sitter-rust = "0.19.0"

zed/src/lib.rs 🔗

@@ -1,7 +1,7 @@
 use anyhow::{anyhow, Context, Result};
 use gpui::{AsyncAppContext, MutableAppContext, Task};
-use smol::io::{AsyncBufReadExt, AsyncWriteExt};
-use std::convert::TryFrom;
+use std::{convert::TryFrom, time::Duration};
+use tiny_http::{Header, Response, Server};
 use url::Url;
 use util::SurfResultExt;
 
@@ -72,7 +72,8 @@ fn share_worktree(_: &(), cx: &mut MutableAppContext) {
 
 fn login(zed_url: String, cx: &AsyncAppContext) -> Task<Result<(String, String)>> {
     let platform = cx.platform();
-    cx.background_executor().spawn(async move {
+    let executor = cx.background_executor();
+    executor.clone().spawn(async move {
         if let Some((user_id, access_token)) = platform.read_credentials(&zed_url) {
             log::info!("already signed in. user_id: {}", user_id);
             return Ok((user_id, String::from_utf8(access_token).unwrap()));
@@ -86,10 +87,9 @@ fn login(zed_url: String, cx: &AsyncAppContext) -> Task<Result<(String, String)>
         let public_key_string =
             String::try_from(public_key).expect("failed to serialize public key for auth");
 
-        // Listen on an open TCP port. This port will be used by the web browser to notify the
-        // application that the login is complete, and to send the user's id and access token.
-        let listener = smol::net::TcpListener::bind("127.0.0.1:0").await?;
-        let port = listener.local_addr()?.port();
+        // Start an HTTP server to receive the redirect from Zed's sign-in page.
+        let server = Server::http("127.0.0.1:0").expect("failed to find open port");
+        let port = server.server_addr().port();
 
         // Open the Zed sign-in page in the user's browser, with query parameters that indicate
         // that the user is signing in from a Zed app running on the same device.
@@ -98,56 +98,44 @@ fn login(zed_url: String, cx: &AsyncAppContext) -> Task<Result<(String, String)>
             zed_url, port, public_key_string
         ));
 
-        // Receive the HTTP request from the user's browser. Parse the first line, which contains
-        // the HTTP method and path.
-        let (mut stream, _) = listener.accept().await?;
-        let mut reader = smol::io::BufReader::new(&mut stream);
-        let mut line = String::new();
-        reader.read_line(&mut line).await?;
-        let mut parts = line.split(" ");
-        let http_method = parts.next();
-        if http_method != Some("GET") {
-            return Err(anyhow!(
-                "unexpected http method {:?} in request from zed web app",
-                http_method
-            ));
-        }
-        let path = parts.next().ok_or_else(|| {
-            anyhow!("failed to parse http request from zed login redirect - missing path")
-        })?;
-
-        // Parse the query parameters from the HTTP request.
-        let mut user_id = None;
-        let mut access_token = None;
-        let url = Url::parse(&format!("http://example.com{}", path))
-            .context("failed to parse login notification url")?;
-        for (key, value) in url.query_pairs() {
-            if key == "access_token" {
-                access_token = Some(value);
-            } else if key == "user_id" {
-                user_id = Some(value);
-            }
-        }
-
-        // Write an HTTP response to the user's browser, instructing it to close the tab.
-        // Then transfer focus back to the application.
-        stream
-            .write_all(LOGIN_RESPONSE.as_bytes())
-            .await
-            .context("failed to write login response")?;
-        stream.flush().await.context("failed to flush tcp stream")?;
-        platform.activate(true);
+        // Receive the HTTP request from the user's browser. Retrieve the user id and encrypted
+        // access token from the query params.
+        //
+        // TODO - Avoid ever starting more than one HTTP server. Maybe switch to using a
+        // custom URL scheme instead of this local HTTP server.
+        let (user_id, access_token) = executor
+            .spawn::<anyhow::Result<_>, _>(async move {
+                if let Some(req) = server.recv_timeout(Duration::from_secs(10 * 60))? {
+                    let path = req.url();
+                    let mut user_id = None;
+                    let mut access_token = None;
+                    let url = Url::parse(&format!("http://example.com{}", path))
+                        .context("failed to parse login notification url")?;
+                    for (key, value) in url.query_pairs() {
+                        if key == "access_token" {
+                            access_token = Some(value.to_string());
+                        } else if key == "user_id" {
+                            user_id = Some(value.to_string());
+                        }
+                    }
+                    req.respond(
+                        Response::from_string(LOGIN_RESPONSE)
+                            .with_header(Header::from_bytes("Content-Type", "text/html").unwrap()),
+                    )
+                    .context("failed to respond to login http request")?;
+                    Ok(user_id.zip(access_token))
+                } else {
+                    Ok(None)
+                }
+            })
+            .await?
+            .ok_or_else(|| anyhow!(""))?;
 
-        // If login succeeded, then store the credentials in the keychain.
-        let user_id = user_id.ok_or_else(|| anyhow!("missing user_id in login request"))?;
-        let access_token =
-            access_token.ok_or_else(|| anyhow!("missing access_token in login request"))?;
         let access_token = private_key
             .decrypt_string(&access_token)
             .context("failed to decrypt access token")?;
+        platform.activate(true);
         platform.write_credentials(&zed_url, &user_id, access_token.as_bytes());
-        log::info!("successfully signed in. user_id: {}", user_id);
-
         Ok((user_id.to_string(), access_token))
     })
 }
@@ -157,10 +145,6 @@ fn quit(_: &(), cx: &mut MutableAppContext) {
 }
 
 const LOGIN_RESPONSE: &'static str = "
-HTTP/1.1 200 OK\r
-Content-Length: 64\r
-Content-Type: text/html\r
-\r
 <!DOCTYPE html>
 <html>
 <script>window.close();</script>