Merge pull request #179 from zed-industries/rpc-protocol-version

Antonio Scandurra created

Forbid connections from outdated clients

Change summary

server/src/rpc.rs            |   5 +
zed/assets/themes/_base.toml |   1 
zed/src/file_finder.rs       |  59 ++++++++++++-------
zed/src/fs.rs                |  12 ++++
zed/src/rpc.rs               |  49 ++++++++++------
zed/src/test.rs              |   4 
zed/src/theme.rs             |   3 
zed/src/workspace.rs         | 114 +++++++++++++++++++++++---------------
zrpc/src/lib.rs              |   2 
9 files changed, 159 insertions(+), 90 deletions(-)

Detailed changes

server/src/rpc.rs 🔗

@@ -894,8 +894,11 @@ pub fn add_routes(app: &mut tide::Server<Arc<AppState>>, rpc: &Arc<Peer>) {
             let connection_upgrade = header_contains_ignore_case(&request, CONNECTION, "upgrade");
             let upgrade_to_websocket = header_contains_ignore_case(&request, UPGRADE, "websocket");
             let upgrade_requested = connection_upgrade && upgrade_to_websocket;
+            let client_protocol_version: Option<u32> = request
+                .header("X-Zed-Protocol-Version")
+                .and_then(|v| v.as_str().parse().ok());
 
-            if !upgrade_requested {
+            if !upgrade_requested || client_protocol_version != Some(zrpc::PROTOCOL_VERSION) {
                 return Ok(Response::new(StatusCode::UpgradeRequired));
             }
 

zed/assets/themes/_base.toml 🔗

@@ -11,6 +11,7 @@ title = "$text.0"
 avatar_width = 20
 icon_color = "$text.2.color"
 avatar = { corner_radius = 10, border = { width = 1, color = "#00000088" } }
+outdated_warning = { extends = "$text.2", size = 13 }
 
 [workspace.titlebar.offline_icon]
 padding = { right = 4 }

zed/src/file_finder.rs 🔗

@@ -438,29 +438,37 @@ mod tests {
     use crate::{
         editor::{self, Insert},
         fs::FakeFs,
-        test::{temp_tree, test_app_state},
+        test::test_app_state,
         workspace::Workspace,
     };
     use serde_json::json;
-    use std::fs;
-    use tempdir::TempDir;
+    use std::path::PathBuf;
 
     #[gpui::test]
     async fn test_matching_paths(mut cx: gpui::TestAppContext) {
-        let tmp_dir = TempDir::new("example").unwrap();
-        fs::create_dir(tmp_dir.path().join("a")).unwrap();
-        fs::write(tmp_dir.path().join("a/banana"), "banana").unwrap();
-        fs::write(tmp_dir.path().join("a/bandana"), "bandana").unwrap();
+        let app_state = cx.update(test_app_state);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    "a": {
+                        "banana": "",
+                        "bandana": "",
+                    }
+                }),
+            )
+            .await;
         cx.update(|cx| {
             super::init(cx);
             editor::init(cx);
         });
 
-        let app_state = cx.update(test_app_state);
         let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx));
         workspace
             .update(&mut cx, |workspace, cx| {
-                workspace.add_worktree(tmp_dir.path(), cx)
+                workspace.add_worktree(Path::new("/root"), cx)
             })
             .await
             .unwrap();
@@ -572,17 +580,17 @@ mod tests {
 
     #[gpui::test]
     async fn test_single_file_worktrees(mut cx: gpui::TestAppContext) {
-        let temp_dir = TempDir::new("test-single-file-worktrees").unwrap();
-        let dir_path = temp_dir.path().join("the-parent-dir");
-        let file_path = dir_path.join("the-file");
-        fs::create_dir(&dir_path).unwrap();
-        fs::write(&file_path, "").unwrap();
-
         let app_state = cx.update(test_app_state);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree("/root", json!({ "the-parent-dir": { "the-file": "" } }))
+            .await;
+
         let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx));
         workspace
             .update(&mut cx, |workspace, cx| {
-                workspace.add_worktree(&file_path, cx)
+                workspace.add_worktree(Path::new("/root/the-parent-dir/the-file"), cx)
             })
             .await
             .unwrap();
@@ -620,18 +628,25 @@ mod tests {
 
     #[gpui::test(retries = 5)]
     async fn test_multiple_matches_with_same_relative_path(mut cx: gpui::TestAppContext) {
-        let tmp_dir = temp_tree(json!({
-            "dir1": { "a.txt": "" },
-            "dir2": { "a.txt": "" }
-        }));
-
         let app_state = cx.update(test_app_state);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    "dir1": { "a.txt": "" },
+                    "dir2": { "a.txt": "" }
+                }),
+            )
+            .await;
+
         let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx));
 
         workspace
             .update(&mut cx, |workspace, cx| {
                 workspace.open_paths(
-                    &[tmp_dir.path().join("dir1"), tmp_dir.path().join("dir2")],
+                    &[PathBuf::from("/root/dir1"), PathBuf::from("/root/dir2")],
                     cx,
                 )
             })

zed/src/fs.rs 🔗

@@ -29,6 +29,8 @@ pub trait Fs: Send + Sync {
         latency: Duration,
     ) -> Pin<Box<dyn Send + Stream<Item = Vec<fsevent::Event>>>>;
     fn is_fake(&self) -> bool;
+    #[cfg(any(test, feature = "test-support"))]
+    fn as_fake(&self) -> &FakeFs;
 }
 
 #[derive(Clone, Debug)]
@@ -125,6 +127,11 @@ impl Fs for RealFs {
     fn is_fake(&self) -> bool {
         false
     }
+
+    #[cfg(any(test, feature = "test-support"))]
+    fn as_fake(&self) -> &FakeFs {
+        panic!("called `RealFs::as_fake`")
+    }
 }
 
 #[derive(Clone, Debug)]
@@ -413,4 +420,9 @@ impl Fs for FakeFs {
     fn is_fake(&self) -> bool {
         true
     }
+
+    #[cfg(any(test, feature = "test-support"))]
+    fn as_fake(&self) -> &FakeFs {
+        self
+    }
 }

zed/src/rpc.rs 🔗

@@ -55,6 +55,8 @@ pub struct Client {
 
 #[derive(Error, Debug)]
 pub enum EstablishConnectionError {
+    #[error("upgrade required")]
+    UpgradeRequired,
     #[error("unauthorized")]
     Unauthorized,
     #[error("{0}")]
@@ -68,8 +70,10 @@ pub enum EstablishConnectionError {
 impl From<WebsocketError> for EstablishConnectionError {
     fn from(error: WebsocketError) -> Self {
         if let WebsocketError::Http(response) = &error {
-            if response.status() == StatusCode::UNAUTHORIZED {
-                return EstablishConnectionError::Unauthorized;
+            match response.status() {
+                StatusCode::UNAUTHORIZED => return EstablishConnectionError::Unauthorized,
+                StatusCode::UPGRADE_REQUIRED => return EstablishConnectionError::UpgradeRequired,
+                _ => {}
             }
         }
         EstablishConnectionError::Other(error.into())
@@ -85,6 +89,7 @@ impl EstablishConnectionError {
 #[derive(Copy, Clone, Debug)]
 pub enum Status {
     SignedOut,
+    UpgradeRequired,
     Authenticating,
     Connecting,
     ConnectionError,
@@ -227,7 +232,7 @@ impl Client {
                     }
                 }));
             }
-            Status::SignedOut => {
+            Status::SignedOut | Status::UpgradeRequired => {
                 state._maintain_connection.take();
             }
             _ => {}
@@ -346,6 +351,7 @@ impl Client {
             | Status::Reconnecting { .. }
             | Status::Authenticating
             | Status::Reauthenticating => return Ok(()),
+            Status::UpgradeRequired => return Err(EstablishConnectionError::UpgradeRequired)?,
         };
 
         if was_disconnected {
@@ -388,22 +394,25 @@ impl Client {
                 self.set_connection(conn, cx).await;
                 Ok(())
             }
-            Err(err) => {
-                if matches!(err, EstablishConnectionError::Unauthorized) {
-                    self.state.write().credentials.take();
-                    if used_keychain {
-                        cx.platform().delete_credentials(&ZED_SERVER_URL).log_err();
-                        self.set_status(Status::SignedOut, cx);
-                        self.authenticate_and_connect(cx).await
-                    } else {
-                        self.set_status(Status::ConnectionError, cx);
-                        Err(err)?
-                    }
+            Err(EstablishConnectionError::Unauthorized) => {
+                self.state.write().credentials.take();
+                if used_keychain {
+                    cx.platform().delete_credentials(&ZED_SERVER_URL).log_err();
+                    self.set_status(Status::SignedOut, cx);
+                    self.authenticate_and_connect(cx).await
                 } else {
                     self.set_status(Status::ConnectionError, cx);
-                    Err(err)?
+                    Err(EstablishConnectionError::Unauthorized)?
                 }
             }
+            Err(EstablishConnectionError::UpgradeRequired) => {
+                self.set_status(Status::UpgradeRequired, cx);
+                Err(EstablishConnectionError::UpgradeRequired)?
+            }
+            Err(error) => {
+                self.set_status(Status::ConnectionError, cx);
+                Err(error)?
+            }
         }
     }
 
@@ -489,10 +498,12 @@ impl Client {
         credentials: &Credentials,
         cx: &AsyncAppContext,
     ) -> Task<Result<Connection, EstablishConnectionError>> {
-        let request = Request::builder().header(
-            "Authorization",
-            format!("{} {}", credentials.user_id, credentials.access_token),
-        );
+        let request = Request::builder()
+            .header(
+                "Authorization",
+                format!("{} {}", credentials.user_id, credentials.access_token),
+            )
+            .header("X-Zed-Protocol-Version", zrpc::PROTOCOL_VERSION);
         cx.background().spawn(async move {
             if let Some(host) = ZED_SERVER_URL.strip_prefix("https://") {
                 let stream = smol::net::TcpStream::connect(host).await?;

zed/src/test.rs 🔗

@@ -1,7 +1,7 @@
 use crate::{
     assets::Assets,
     channel::ChannelList,
-    fs::RealFs,
+    fs::FakeFs,
     http::{HttpClient, Request, Response, ServerResponse},
     language::LanguageRegistry,
     rpc::{self, Client, Credentials, EstablishConnectionError},
@@ -177,7 +177,7 @@ pub fn test_app_state(cx: &mut MutableAppContext) -> Arc<AppState> {
         channel_list: cx.add_model(|cx| ChannelList::new(user_store.clone(), rpc.clone(), cx)),
         rpc,
         user_store,
-        fs: Arc::new(RealFs),
+        fs: Arc::new(FakeFs::new()),
     })
 }
 

zed/src/theme.rs 🔗

@@ -54,6 +54,7 @@ pub struct Titlebar {
     pub offline_icon: OfflineIcon,
     pub icon_color: Color,
     pub avatar: ImageStyle,
+    pub outdated_warning: ContainedText,
 }
 
 #[derive(Clone, Deserialize)]
@@ -169,7 +170,7 @@ pub struct Selector {
     pub active_item: ContainedLabel,
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 pub struct ContainedText {
     #[serde(flatten)]
     pub container: ContainerStyle,

zed/src/workspace.rs 🔗

@@ -1041,6 +1041,16 @@ impl Workspace {
                 .with_style(theme.workspace.titlebar.offline_icon.container)
                 .boxed(),
             ),
+            rpc::Status::UpgradeRequired => Some(
+                Label::new(
+                    "Please update Zed to collaborate".to_string(),
+                    theme.workspace.titlebar.outdated_warning.text.clone(),
+                )
+                .contained()
+                .with_style(theme.workspace.titlebar.outdated_warning.container)
+                .aligned()
+                .boxed(),
+            ),
             _ => None,
         }
     }
@@ -1195,11 +1205,9 @@ mod tests {
         editor::{Editor, Insert},
         fs::FakeFs,
         test::{temp_tree, test_app_state},
-        worktree::WorktreeHandle,
     };
     use serde_json::json;
-    use std::{collections::HashSet, fs};
-    use tempdir::TempDir;
+    use std::collections::HashSet;
 
     #[gpui::test]
     async fn test_open_paths_action(mut cx: gpui::TestAppContext) {
@@ -1268,20 +1276,26 @@ mod tests {
 
     #[gpui::test]
     async fn test_open_entry(mut cx: gpui::TestAppContext) {
-        let dir = temp_tree(json!({
-            "a": {
-                "file1": "contents 1",
-                "file2": "contents 2",
-                "file3": "contents 3",
-            },
-        }));
-
         let app_state = cx.update(test_app_state);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    "a": {
+                        "file1": "contents 1",
+                        "file2": "contents 2",
+                        "file3": "contents 3",
+                    },
+                }),
+            )
+            .await;
 
         let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx));
         workspace
             .update(&mut cx, |workspace, cx| {
-                workspace.add_worktree(dir.path(), cx)
+                workspace.add_worktree(Path::new("/root"), cx)
             })
             .await
             .unwrap();
@@ -1445,28 +1459,30 @@ mod tests {
 
     #[gpui::test]
     async fn test_save_conflicting_item(mut cx: gpui::TestAppContext) {
-        let dir = temp_tree(json!({
-            "a.txt": "",
-        }));
-
         let app_state = cx.update(test_app_state);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    "a.txt": "",
+                }),
+            )
+            .await;
+
         let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx));
         workspace
             .update(&mut cx, |workspace, cx| {
-                workspace.add_worktree(dir.path(), cx)
+                workspace.add_worktree(Path::new("/root"), cx)
             })
             .await
             .unwrap();
-        let tree = cx.read(|cx| {
-            let mut trees = workspace.read(cx).worktrees().iter();
-            trees.next().unwrap().clone()
-        });
-        tree.flush_fs_events(&cx).await;
 
         // Open a file within an existing worktree.
         cx.update(|cx| {
             workspace.update(cx, |view, cx| {
-                view.open_paths(&[dir.path().join("a.txt")], cx)
+                view.open_paths(&[PathBuf::from("/root/a.txt")], cx)
             })
         })
         .await;
@@ -1477,7 +1493,12 @@ mod tests {
         });
 
         cx.update(|cx| editor.update(cx, |editor, cx| editor.insert(&Insert("x".into()), cx)));
-        fs::write(dir.path().join("a.txt"), "changed").unwrap();
+        app_state
+            .fs
+            .as_fake()
+            .insert_file("/root/a.txt", "changed".to_string())
+            .await
+            .unwrap();
         editor
             .condition(&cx, |editor, cx| editor.has_conflict(cx))
             .await;
@@ -1493,12 +1514,12 @@ mod tests {
 
     #[gpui::test]
     async fn test_open_and_save_new_file(mut cx: gpui::TestAppContext) {
-        let dir = TempDir::new("test-new-file").unwrap();
         let app_state = cx.update(test_app_state);
+        app_state.fs.as_fake().insert_dir("/root").await.unwrap();
         let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx));
         workspace
             .update(&mut cx, |workspace, cx| {
-                workspace.add_worktree(dir.path(), cx)
+                workspace.add_worktree(Path::new("/root"), cx)
             })
             .await
             .unwrap();
@@ -1511,7 +1532,6 @@ mod tests {
                 .unwrap()
                 .clone()
         });
-        tree.flush_fs_events(&cx).await;
 
         // Create a new untitled buffer
         let editor = workspace.update(&mut cx, |workspace, cx| {
@@ -1537,7 +1557,7 @@ mod tests {
             workspace.save_active_item(&Save, cx)
         });
         cx.simulate_new_path_selection(|parent_dir| {
-            assert_eq!(parent_dir, dir.path());
+            assert_eq!(parent_dir, Path::new("/root"));
             Some(parent_dir.join("the-new-name.rs"))
         });
         cx.read(|cx| {
@@ -1598,8 +1618,8 @@ mod tests {
     async fn test_setting_language_when_saving_as_single_file_worktree(
         mut cx: gpui::TestAppContext,
     ) {
-        let dir = TempDir::new("test-new-file").unwrap();
         let app_state = cx.update(test_app_state);
+        app_state.fs.as_fake().insert_dir("/root").await.unwrap();
         let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx));
 
         // Create a new untitled buffer
@@ -1623,7 +1643,7 @@ mod tests {
         workspace.update(&mut cx, |workspace, cx| {
             workspace.save_active_item(&Save, cx)
         });
-        cx.simulate_new_path_selection(|_| Some(dir.path().join("the-new-name.rs")));
+        cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name.rs")));
 
         editor
             .condition(&cx, |editor, cx| !editor.is_dirty(cx))
@@ -1640,7 +1660,7 @@ mod tests {
         cx.update(init);
 
         let app_state = cx.update(test_app_state);
-        cx.dispatch_global_action(OpenNew(app_state));
+        cx.dispatch_global_action(OpenNew(app_state.clone()));
         let window_id = *cx.window_ids().first().unwrap();
         let workspace = cx.root_view::<Workspace>(window_id).unwrap();
         let editor = workspace.update(&mut cx, |workspace, cx| {
@@ -1660,10 +1680,8 @@ mod tests {
             workspace.save_active_item(&Save, cx)
         });
 
-        let dir = TempDir::new("test-new-empty-workspace").unwrap();
-        cx.simulate_new_path_selection(|_| {
-            Some(dir.path().canonicalize().unwrap().join("the-new-name"))
-        });
+        app_state.fs.as_fake().insert_dir("/root").await.unwrap();
+        cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name")));
 
         editor
             .condition(&cx, |editor, cx| editor.title(cx) == "the-new-name")
@@ -1676,20 +1694,26 @@ mod tests {
     #[gpui::test]
     async fn test_pane_actions(mut cx: gpui::TestAppContext) {
         cx.update(|cx| pane::init(cx));
-
-        let dir = temp_tree(json!({
-            "a": {
-                "file1": "contents 1",
-                "file2": "contents 2",
-                "file3": "contents 3",
-            },
-        }));
-
         let app_state = cx.update(test_app_state);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    "a": {
+                        "file1": "contents 1",
+                        "file2": "contents 2",
+                        "file3": "contents 3",
+                    },
+                }),
+            )
+            .await;
+
         let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx));
         workspace
             .update(&mut cx, |workspace, cx| {
-                workspace.add_worktree(dir.path(), cx)
+                workspace.add_worktree(Path::new("/root"), cx)
             })
             .await
             .unwrap();

zrpc/src/lib.rs 🔗

@@ -4,3 +4,5 @@ mod peer;
 pub mod proto;
 pub use conn::Connection;
 pub use peer::*;
+
+pub const PROTOCOL_VERSION: u32 = 0;