Skip to content

Commit

Permalink
[ENG-1310, ENG-1300] Spacedrop better testing + fix zero-sized files (s…
Browse files Browse the repository at this point in the history
…pacedriveapp#1606)

* unit tests save lives

* Support for zero-sized files

* fix: clippy

---------

Co-authored-by: jake <77554505+brxken128@users.noreply.github.com>
  • Loading branch information
oscartbeaumont and brxken128 committed Oct 17, 2023
1 parent a7ad24c commit 1b85684
Show file tree
Hide file tree
Showing 6 changed files with 419 additions and 147 deletions.
5 changes: 2 additions & 3 deletions core/src/p2p/p2p_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl P2PManager {
debug!("Received ping from peer '{}'", event.peer_id);
}
Header::Spacedrop(req) => {
let id = Uuid::new_v4(); // TODO: Get ID from the remote
let id = req.id;
let (tx, rx) = oneshot::channel();

info!(
Expand All @@ -254,7 +254,6 @@ impl P2PManager {
.find(|p| p.peer_id == event.peer_id)
.map(|p| p.metadata.name)
.unwrap_or_else(|| "Unknown".to_string()),
// TODO: If multiple files in request ask user to select a whole directory instead!
files: req
.requests
.iter()
Expand All @@ -270,7 +269,7 @@ impl P2PManager {

tokio::select! {
_ = sleep(SPACEDROP_TIMEOUT) => {
info!("spacedrop({id}): timeout, rejecting!");
info!("({id}): timeout, rejecting!");

stream.write_all(&[0]).await.unwrap();
stream.flush().await.unwrap();
Expand Down
1 change: 1 addition & 0 deletions crates/p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,4 @@ hex = "0.4.3"
[dev-dependencies]
tokio = { workspace = true, features = ["rt-multi-thread"] }
tracing-subscriber = { workspace = true, features = ["env-filter"] }
uuid = { version = "1.4.1", features = ["v4"] }
49 changes: 48 additions & 1 deletion crates/p2p/src/spaceblock/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ impl<'a> Block<'a> {
pub fn to_bytes(&self) -> Vec<u8> {
let mut buf = Vec::new();
buf.extend_from_slice(&self.offset.to_le_bytes());
debug_assert_eq!(self.data.len(), self.size as usize); // TODO: Should `self.size` be inferred instead?
buf.extend_from_slice(&self.size.to_le_bytes());
buf.extend_from_slice(self.data);
buf
Expand All @@ -33,6 +34,10 @@ impl<'a> Block<'a> {

// TODO: Ensure `size` is `block_size` or smaller else buffer overflow

if size as usize > data_buf.len() {
return Err(());
}

stream
.read_exact(&mut data_buf[..size as usize])
.await
Expand All @@ -46,4 +51,46 @@ impl<'a> Block<'a> {
}
}

// TODO: Unit test `Block`
#[cfg(test)]
mod tests {
use std::io::Cursor;

use crate::spaceblock::BlockSize;

use super::*;

#[tokio::test]
async fn test_block() {
let mut req = Block {
offset: 420,
size: 10, // Matches length of string on next line
data: b"Spacedrive".as_ref(),
};
let bytes = req.to_bytes();
let mut data2 = vec![0; req.data.len()];
let req2 = Block::from_stream(&mut Cursor::new(bytes), &mut data2)
.await
.unwrap();
let data = std::mem::take(&mut req.data);
assert_eq!(req, req2);
assert_eq!(data, data2);
}

#[tokio::test]
#[should_panic] // TODO: This currently panics but long term it should have proper error handling
async fn test_block_data_buf_overflow() {
let mut req = Block {
offset: 420,
size: 10, // Matches length of string on next line
data: b"Spacedrive".as_ref(),
};
let bytes = req.to_bytes();
let mut data2 = vec![0; 5]; // Length smaller than `req.data.len()`
let req2 = Block::from_stream(&mut Cursor::new(bytes), &mut data2)
.await
.unwrap();
let data = std::mem::take(&mut req.data);
assert_eq!(req, req2);
assert_eq!(data, data2);
}
}
17 changes: 17 additions & 0 deletions crates/p2p/src/spaceblock/block_size.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,20 @@ impl BlockSize {
self.0
}
}

#[cfg(test)]
mod tests {
use std::io::Cursor;

use super::*;

#[tokio::test]
async fn test_block_size() {
let req = BlockSize::dangerously_new(5);
let bytes = req.to_bytes();
let req2 = BlockSize::from_stream(&mut Cursor::new(bytes))
.await
.unwrap();
assert_eq!(req, req2);
}
}
Loading

0 comments on commit 1b85684

Please sign in to comment.