Update Actix crates to latest versions

This avoids a vulnerability in tokio (#3085). The major version updates
of both actix-web and actix-rt required some signficant changes. Chief
among those, it turns out we were relying on actix-rt to run the
HttpServer in a different thread from the rest of the test, so that we
could talk to it from sync code in the test thread. This no longer
works, so the sync code is now run in a dedicated thread with
`actix_rt::task::spawn_blocking`.

taskchampion/integration-tests/Cargo.toml

@@ -13,8 +13,8 @@ taskchampion-sync-server = { path = "../sync-server" }
 
 [dev-dependencies]
 anyhow = "1.0"
-actix-web = "^3.3.2"
-actix-rt = "^1.1.1"
+actix-web = "^4.3.1"
+actix-rt = "2"
 tempfile = "3"
 pretty_assertions = "1"
 log = "^0.4.17"

taskchampion/integration-tests/tests/cross-sync.rs

@@ -5,20 +5,23 @@ use taskchampion_sync_server::{storage::InMemoryStorage, Server};
 
 #[actix_rt::test]
 async fn cross_sync() -> anyhow::Result<()> {
+    async fn server() -> anyhow::Result<u16> {
         let _ = env_logger::builder()
             .is_test(true)
             .filter_level(log::LevelFilter::Trace)
             .try_init();
 
         let server = Server::new(Default::default(), Box::new(InMemoryStorage::new()));
-    let httpserver =
-        HttpServer::new(move || App::new().configure(|sc| server.config(sc))).bind("0.0.0.0:0")?;
+        let httpserver = HttpServer::new(move || App::new().configure(|sc| server.config(sc)))
+            .bind("0.0.0.0:0")?;
 
         // bind was to :0, so the kernel will have selected an unused port
         let port = httpserver.addrs()[0].port();
+        actix_rt::spawn(httpserver.run());
+        Ok(port)
+    }
 
-    httpserver.run();
-
+    fn client(port: u16) -> anyhow::Result<()> {
         // set up two replicas, and demonstrate replication between them
         let mut rep1 = Replica::new(StorageConfig::InMemory.into_storage()?);
         let mut rep2 = Replica::new(StorageConfig::InMemory.into_storage()?);
@@ -85,6 +88,10 @@ async fn cross_sync() -> anyhow::Result<()> {
             .expect("expected task 2 on rep2");
         assert_eq!(t22.get_status(), Status::Completed);
 
-    // note that we just drop the server here..
+        Ok(())
+    }
+
+    let port = server().await?;
+    actix_rt::task::spawn_blocking(move || client(port)).await??;
     Ok(())
 }

taskchampion/integration-tests/tests/snapshots.rs

@@ -14,19 +14,22 @@ async fn sync_with_snapshots() -> anyhow::Result<()> {
         .filter_level(log::LevelFilter::Trace)
         .try_init();
 
+    async fn server() -> anyhow::Result<u16> {
         let sync_server_config = SyncServerConfig {
             snapshot_days: 100,
             snapshot_versions: 3,
         };
         let server = Server::new(sync_server_config, Box::new(InMemoryStorage::new()));
-    let httpserver =
-        HttpServer::new(move || App::new().configure(|sc| server.config(sc))).bind("0.0.0.0:0")?;
+        let httpserver = HttpServer::new(move || App::new().configure(|sc| server.config(sc)))
+            .bind("0.0.0.0:0")?;
 
         // bind was to :0, so the kernel will have selected an unused port
         let port = httpserver.addrs()[0].port();
+        actix_rt::spawn(httpserver.run());
+        Ok(port)
+    }
 
-    httpserver.run();
-
+    fn client(port: u16) -> anyhow::Result<()> {
         let client_key = Uuid::new_v4();
         let encryption_secret = b"abc123".to_vec();
         let make_server = || {
@@ -55,10 +58,10 @@ async fn sync_with_snapshots() -> anyhow::Result<()> {
         }
 
         // now set up a second replica and sync it; it should catch up on that history, using a
-    // snapshot.  Note that we can't verify that it used a snapshot, because the server currently
-    // keeps all versions (so rep2 could sync from the beginning of the version history).  You can
-    // manually verify that it is applying a snapshot by adding `assert!(false)` below and skimming
-    // the logs.
+        // snapshot.  Note that we can't verify that it used a snapshot, because the server
+        // currently keeps all versions (so rep2 could sync from the beginning of the version
+        // history).  You can manually verify that it is applying a snapshot by adding
+        // `assert!(false)` below and skimming the logs.
 
         let mut rep2 = Replica::new(StorageConfig::InMemory.into_storage()?);
         let mut serv2 = make_server()?;
@@ -86,9 +89,10 @@ async fn sync_with_snapshots() -> anyhow::Result<()> {
 
         assert_eq!(t11.get_description(), "sync-back");
 
-    // uncomment this to force a failure and see the logs
-    // assert!(false);
-
-    // note that we just drop the server here..
+        Ok(())
+    }
+
+    let port = server().await?;
+    actix_rt::task::spawn_blocking(move || client(port)).await??;
     Ok(())
 }

taskchampion/sync-server/Cargo.toml

@@ -9,7 +9,7 @@ publish = false
 
 [dependencies]
 uuid = { version = "^1.3.0", features = ["serde", "v4"] }
-actix-web = "^3.3.2"
+actix-web = "^4.3.1"
 anyhow = "1.0"
 thiserror = "1.0"
 futures = "^0.3.25"
@@ -22,6 +22,6 @@ rusqlite = { version = "0.29", features = ["bundled"] }
 chrono = { version = "^0.4.22", features = ["serde"] }
 
 [dev-dependencies]
-actix-rt = "^1.1.1"
+actix-rt = "2"
 tempfile = "3"
 pretty_assertions = "1"

taskchampion/sync-server/src/api/add_snapshot.rs

@@ -19,9 +19,11 @@ const MAX_SIZE: usize = 100 * 1024 * 1024;
 pub(crate) async fn service(
     req: HttpRequest,
     server_state: web::Data<Arc<ServerState>>,
-    web::Path((version_id,)): web::Path<(VersionId,)>,
+    path: web::Path<VersionId>,
     mut payload: web::Payload,
 ) -> Result<HttpResponse> {
+    let version_id = path.into_inner();
+
     // check content-type
     if req.content_type() != SNAPSHOT_CONTENT_TYPE {
         return Err(error::ErrorBadRequest("Bad content-type"));
@@ -100,8 +102,8 @@ mod test {
         let uri = format!("/v1/client/add-snapshot/{}", version_id);
         let req = test::TestRequest::post()
             .uri(&uri)
-            .header("Content-Type", "application/vnd.taskchampion.snapshot")
-            .header("X-Client-Key", client_key.to_string())
+            .insert_header(("Content-Type", "application/vnd.taskchampion.snapshot"))
+            .insert_header(("X-Client-Key", client_key.to_string()))
             .set_payload(b"abcd".to_vec())
             .to_request();
         let resp = test::call_service(&mut app, req).await;
@@ -111,14 +113,14 @@ mod test {
         let uri = "/v1/client/snapshot";
         let req = test::TestRequest::get()
             .uri(uri)
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
-        let mut resp = test::call_service(&mut app, req).await;
+        let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::OK);
 
-        use futures::StreamExt;
-        let (bytes, _) = resp.take_body().into_future().await;
-        assert_eq!(bytes.unwrap().unwrap().as_ref(), b"abcd");
+        use actix_web::body::MessageBody;
+        let bytes = resp.into_body().try_into_bytes().unwrap();
+        assert_eq!(bytes.as_ref(), b"abcd");
 
         Ok(())
     }
@@ -143,8 +145,8 @@ mod test {
         let uri = format!("/v1/client/add-snapshot/{}", version_id);
         let req = test::TestRequest::post()
             .uri(&uri)
-            .header("Content-Type", "application/vnd.taskchampion.snapshot")
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("Content-Type", "application/vnd.taskchampion.snapshot"))
+            .append_header(("X-Client-Key", client_key.to_string()))
             .set_payload(b"abcd".to_vec())
             .to_request();
         let resp = test::call_service(&mut app, req).await;
@@ -154,7 +156,7 @@ mod test {
         let uri = "/v1/client/snapshot";
         let req = test::TestRequest::get()
             .uri(uri)
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
         let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::NOT_FOUND);
@@ -172,8 +174,8 @@ mod test {
         let uri = format!("/v1/client/add-snapshot/{}", version_id);
         let req = test::TestRequest::post()
             .uri(&uri)
-            .header("Content-Type", "not/correct")
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("Content-Type", "not/correct"))
+            .append_header(("X-Client-Key", client_key.to_string()))
             .set_payload(b"abcd".to_vec())
             .to_request();
         let resp = test::call_service(&mut app, req).await;
@@ -192,11 +194,11 @@ mod test {
         let uri = format!("/v1/client/add-snapshot/{}", version_id);
         let req = test::TestRequest::post()
             .uri(&uri)
-            .header(
+            .append_header((
                 "Content-Type",
                 "application/vnd.taskchampion.history-segment",
-            )
-            .header("X-Client-Key", client_key.to_string())
+            ))
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
         let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::BAD_REQUEST);

taskchampion/sync-server/src/api/add_version.rs

@@ -27,9 +27,11 @@ const MAX_SIZE: usize = 100 * 1024 * 1024;
 pub(crate) async fn service(
     req: HttpRequest,
     server_state: web::Data<Arc<ServerState>>,
-    web::Path((parent_version_id,)): web::Path<(VersionId,)>,
+    path: web::Path<VersionId>,
     mut payload: web::Payload,
 ) -> Result<HttpResponse> {
+    let parent_version_id = path.into_inner();
+
     // check content-type
     if req.content_type() != HISTORY_SEGMENT_CONTENT_TYPE {
         return Err(error::ErrorBadRequest("Bad content-type"));
@@ -80,21 +82,21 @@ pub(crate) async fn service(
     Ok(match result {
         AddVersionResult::Ok(version_id) => {
             let mut rb = HttpResponse::Ok();
-            rb.header(VERSION_ID_HEADER, version_id.to_string());
+            rb.append_header((VERSION_ID_HEADER, version_id.to_string()));
             match snap_urgency {
                 SnapshotUrgency::None => {}
                 SnapshotUrgency::Low => {
-                    rb.header(SNAPSHOT_REQUEST_HEADER, "urgency=low");
+                    rb.append_header((SNAPSHOT_REQUEST_HEADER, "urgency=low"));
                 }
                 SnapshotUrgency::High => {
-                    rb.header(SNAPSHOT_REQUEST_HEADER, "urgency=high");
+                    rb.append_header((SNAPSHOT_REQUEST_HEADER, "urgency=high"));
                 }
             };
             rb.finish()
         }
         AddVersionResult::ExpectedParentVersion(parent_version_id) => {
             let mut rb = HttpResponse::Conflict();
-            rb.header(PARENT_VERSION_ID_HEADER, parent_version_id.to_string());
+            rb.append_header((PARENT_VERSION_ID_HEADER, parent_version_id.to_string()));
             rb.finish()
         }
     })
@@ -128,11 +130,11 @@ mod test {
         let uri = format!("/v1/client/add-version/{}", parent_version_id);
         let req = test::TestRequest::post()
             .uri(&uri)
-            .header(
+            .append_header((
                 "Content-Type",
                 "application/vnd.taskchampion.history-segment",
-            )
-            .header("X-Client-Key", client_key.to_string())
+            ))
+            .append_header(("X-Client-Key", client_key.to_string()))
             .set_payload(b"abcd".to_vec())
             .to_request();
         let resp = test::call_service(&mut app, req).await;
@@ -170,11 +172,11 @@ mod test {
         let uri = format!("/v1/client/add-version/{}", parent_version_id);
         let req = test::TestRequest::post()
             .uri(&uri)
-            .header(
+            .append_header((
                 "Content-Type",
                 "application/vnd.taskchampion.history-segment",
-            )
-            .header("X-Client-Key", client_key.to_string())
+            ))
+            .append_header(("X-Client-Key", client_key.to_string()))
             .set_payload(b"abcd".to_vec())
             .to_request();
         let resp = test::call_service(&mut app, req).await;
@@ -198,8 +200,8 @@ mod test {
         let uri = format!("/v1/client/add-version/{}", parent_version_id);
         let req = test::TestRequest::post()
             .uri(&uri)
-            .header("Content-Type", "not/correct")
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("Content-Type", "not/correct"))
+            .append_header(("X-Client-Key", client_key.to_string()))
             .set_payload(b"abcd".to_vec())
             .to_request();
         let resp = test::call_service(&mut app, req).await;
@@ -218,11 +220,11 @@ mod test {
         let uri = format!("/v1/client/add-version/{}", parent_version_id);
         let req = test::TestRequest::post()
             .uri(&uri)
-            .header(
+            .append_header((
                 "Content-Type",
                 "application/vnd.taskchampion.history-segment",
-            )
-            .header("X-Client-Key", client_key.to_string())
+            ))
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
         let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::BAD_REQUEST);

taskchampion/sync-server/src/api/get_child_version.rs

@@ -18,8 +18,10 @@ use std::sync::Arc;
 pub(crate) async fn service(
     req: HttpRequest,
     server_state: web::Data<Arc<ServerState>>,
-    web::Path((parent_version_id,)): web::Path<(VersionId,)>,
+    path: web::Path<VersionId>,
 ) -> Result<HttpResponse> {
+    let parent_version_id = path.into_inner();
+
     let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
 
     let client_key = client_key_header(&req)?;
@@ -44,8 +46,8 @@ pub(crate) async fn service(
             history_segment,
         } => Ok(HttpResponse::Ok()
             .content_type(HISTORY_SEGMENT_CONTENT_TYPE)
-            .header(VERSION_ID_HEADER, version_id.to_string())
-            .header(PARENT_VERSION_ID_HEADER, parent_version_id.to_string())
+            .append_header((VERSION_ID_HEADER, version_id.to_string()))
+            .append_header((PARENT_VERSION_ID_HEADER, parent_version_id.to_string()))
             .body(history_segment)),
         GetVersionResult::NotFound => Err(error::ErrorNotFound("no such version")),
         GetVersionResult::Gone => Err(error::ErrorGone("version has been deleted")),
@@ -83,9 +85,9 @@ mod test {
         let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
         let req = test::TestRequest::get()
             .uri(&uri)
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
-        let mut resp = test::call_service(&mut app, req).await;
+        let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::OK);
         assert_eq!(
             resp.headers().get("X-Version-Id").unwrap(),
@@ -100,9 +102,9 @@ mod test {
             &"application/vnd.taskchampion.history-segment".to_string()
         );
 
-        use futures::StreamExt;
-        let (bytes, _) = resp.take_body().into_future().await;
-        assert_eq!(bytes.unwrap().unwrap().as_ref(), b"abcd");
+        use actix_web::body::MessageBody;
+        let bytes = resp.into_body().try_into_bytes().unwrap();
+        assert_eq!(bytes.as_ref(), b"abcd");
     }
 
     #[actix_rt::test]
@@ -117,7 +119,7 @@ mod test {
         let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
         let req = test::TestRequest::get()
             .uri(&uri)
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
         let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::NOT_FOUND);
@@ -144,7 +146,7 @@ mod test {
         let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
         let req = test::TestRequest::get()
             .uri(&uri)
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
         let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::GONE);
@@ -157,7 +159,7 @@ mod test {
         let uri = format!("/v1/client/get-child-version/{}", NIL_VERSION_ID);
         let req = test::TestRequest::get()
             .uri(&uri)
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
         let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::NOT_FOUND);

taskchampion/sync-server/src/api/get_snapshot.rs

@@ -32,7 +32,7 @@ pub(crate) async fn service(
     {
         Ok(HttpResponse::Ok()
             .content_type(SNAPSHOT_CONTENT_TYPE)
-            .header(VERSION_ID_HEADER, version_id.to_string())
+            .append_header((VERSION_ID_HEADER, version_id.to_string()))
             .body(data))
     } else {
         Err(error::ErrorNotFound("no snapshot"))
@@ -66,7 +66,7 @@ mod test {
         let uri = "/v1/client/snapshot";
         let req = test::TestRequest::get()
             .uri(uri)
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
         let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::NOT_FOUND);
@@ -102,13 +102,13 @@ mod test {
         let uri = "/v1/client/snapshot";
         let req = test::TestRequest::get()
             .uri(uri)
-            .header("X-Client-Key", client_key.to_string())
+            .append_header(("X-Client-Key", client_key.to_string()))
             .to_request();
-        let mut resp = test::call_service(&mut app, req).await;
+        let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), StatusCode::OK);
 
-        use futures::StreamExt;
-        let (bytes, _) = resp.take_body().into_future().await;
-        assert_eq!(bytes.unwrap().unwrap().as_ref(), snapshot_data);
+        use actix_web::body::MessageBody;
+        let bytes = resp.into_body().try_into_bytes().unwrap();
+        assert_eq!(bytes.as_ref(), snapshot_data);
     }
 }

taskchampion/sync-server/src/lib.rs

@@ -34,10 +34,9 @@ impl Server {
     pub fn config(&self, cfg: &mut web::ServiceConfig) {
         cfg.service(
             web::scope("")
-                .data(self.server_state.clone())
+                .app_data(web::Data::new(self.server_state.clone()))
                 .wrap(
-                    middleware::DefaultHeaders::new()
-                        .header("Cache-Control", "no-store, max-age=0"),
+                    middleware::DefaultHeaders::new().add(("Cache-Control", "no-store, max-age=0")),
                 )
                 .service(index)
                 .service(api_scope()),