Skip to content

Commit

Permalink
Verify add_snapshot is called with the right version
Browse files Browse the repository at this point in the history
  • Loading branch information
djmitche committed Dec 27, 2023
1 parent 4cb3580 commit 2e6521c
Showing 1 changed file with 23 additions and 0 deletions.
23 changes: 23 additions & 0 deletions taskchampion/taskchampion/src/server/cloud/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ pub(in crate::server) struct CloudServer<SVC: Service> {
/// The probability (0..255) that this run will perform cleanup.
cleanup_probability: u8,

/// The last version added locally with the `add_version` method.
last_version_added: Option<VersionId>,

/// For testing, a function that is called in the middle of `add_version` to simulate
/// a concurrent change in the service.
#[cfg(test)]
Expand All @@ -88,6 +91,7 @@ impl<SVC: Service> CloudServer<SVC> {
service,
cryptor,
cleanup_probability: DEFAULT_CLEANUP_PROBABILITY,
last_version_added: None,
#[cfg(test)]
add_version_intercept: None,
})
Expand Down Expand Up @@ -376,6 +380,9 @@ impl<SVC: Service> Server for CloudServer<SVC> {
// Attempt a cleanup, but ignore errors.
let _ = self.maybe_cleanup();

// Record the newly returned `version_id`, to verify that this is the snapshot we may get.
self.last_version_added = Some(version_id);

Ok((AddVersionResult::Ok(version_id), self.snapshot_urgency()?))
}

Expand Down Expand Up @@ -433,6 +440,12 @@ impl<SVC: Service> Server for CloudServer<SVC> {
}

fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> {
// The protocol says to verify that this is more recent than any other snapshot, and
// corresponds to an existing version. In practice, this will always be the version
// just returned from add_version, so just check that.
if Some(version_id) != self.last_version_added {
return Err(Error::Server("Snapshot is not for most recently added version".into()));
}
let name = Self::snapshot_name(&version_id);
let sealed = self.cryptor.seal(Unsealed {
version_id,
Expand Down Expand Up @@ -579,6 +592,7 @@ mod tests {
Self {
cryptor: self.cryptor.clone(),
cleanup_probability: 0,
last_version_added: None,
service: MockService::default(),
add_version_intercept: None,
}
Expand Down Expand Up @@ -1108,12 +1122,21 @@ mod tests {
fn add_snapshot() {
let mut server = make_server();
let v = Uuid::new_v4();
server.last_version_added = Some(v);
server.add_snapshot(v, b"SNAP".to_vec()).unwrap();
let mut expected = server.empty_copy();
expected.mock_add_snapshot(v, INSERTION_TIME, b"SNAP");
assert_eq!(server.unencrypted(), expected.unencrypted());
}

#[test]
fn add_snapshot_wrong_version() {
let mut server = make_server();
let v = Uuid::new_v4();
server.last_version_added = Some(Uuid::new_v4());
assert!(server.add_snapshot(v, b"SNAP".to_vec()).is_err());
}

#[test]
fn get_snapshot_missing() {
let mut server = make_server();
Expand Down

0 comments on commit 2e6521c

Please sign in to comment.