From 31010f3bcf0d9f57f45cfd691a67cb7da726b8b3 Mon Sep 17 00:00:00 2001 From: Marcin Anforowicz Date: Mon, 9 Dec 2024 18:15:25 -0800 Subject: [PATCH 1/6] Debugging macos CI test fail. --- .github/workflows/test.yml | 8 ++++---- gday_contact_exchange_protocol/src/lib.rs | 4 ++-- gday_file_transfer/src/offer.rs | 4 ++-- gday_server/src/connection_handler.rs | 4 +++- gday_server/tests/test_integration.rs | 8 ++++---- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 40da540..2697858 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,13 +17,13 @@ jobs: strategy: matrix: os: - - ubuntu-latest - - windows-latest + # - ubuntu-latest + # - windows-latest - macos-latest toolchain: - stable - - beta - - nightly + # - beta + # - nightly steps: - uses: actions/checkout@v3 diff --git a/gday_contact_exchange_protocol/src/lib.rs b/gday_contact_exchange_protocol/src/lib.rs index 63cf66b..e7fd55e 100644 --- a/gday_contact_exchange_protocol/src/lib.rs +++ b/gday_contact_exchange_protocol/src/lib.rs @@ -282,7 +282,7 @@ impl std::fmt::Display for FullContact { /// Writes `msg` to `writer` using [`serde_json`], and flushes. /// -/// Prefixes the message with 2 bytes holding the [`PROTOCOL_VERSION`] +/// Prefixes the message with 1 byte holding the [`PROTOCOL_VERSION`] /// and 2 bytes holding the length of the following message (all in big-endian). pub fn write_to(msg: impl Serialize, writer: &mut impl Write) -> Result<(), Error> { let vec = serde_json::to_vec(&msg)?; @@ -300,7 +300,7 @@ pub fn write_to(msg: impl Serialize, writer: &mut impl Write) -> Result<(), Erro /// Asynchronously writes `msg` to `writer` using [`serde_json`], and flushes. /// -/// Prefixes the message with 2 bytes holding the [`PROTOCOL_VERSION`] +/// Prefixes the message with 1 byte holding the [`PROTOCOL_VERSION`] /// and 2 bytes holding the length of the following message (all in big-endian). pub async fn write_to_async( msg: impl Serialize, diff --git a/gday_file_transfer/src/offer.rs b/gday_file_transfer/src/offer.rs index 841b291..839fcfd 100644 --- a/gday_file_transfer/src/offer.rs +++ b/gday_file_transfer/src/offer.rs @@ -163,7 +163,7 @@ impl FileResponseMsg { /// Writes `msg` to `writer` using [`serde_json`], and flushes. /// -/// Prefixes the message with 2 bytes holding the [`PROTOCOL_VERSION`] +/// Prefixes the message with 1 byte holding the [`PROTOCOL_VERSION`] /// and 4 bytes holding the length of the following message (all in big-endian). pub fn write_to(msg: impl Serialize, writer: &mut impl Write) -> Result<(), Error> { let vec = serde_json::to_vec(&msg)?; @@ -181,7 +181,7 @@ pub fn write_to(msg: impl Serialize, writer: &mut impl Write) -> Result<(), Erro /// Asynchronously writes `msg` to `writer` using [`serde_json`], and flushes. /// -/// Prefixes the message with 2 bytes holding the [`PROTOCOL_VERSION`] +/// Prefixes the message with 1 byte holding the [`PROTOCOL_VERSION`] /// and 4 bytes holding the length of the following message (all in big-endian). pub async fn write_to_async( msg: impl Serialize, diff --git a/gday_server/src/connection_handler.rs b/gday_server/src/connection_handler.rs index 0bc6592..8a4167d 100644 --- a/gday_server/src/connection_handler.rs +++ b/gday_server/src/connection_handler.rs @@ -3,7 +3,7 @@ use gday_contact_exchange_protocol::{read_from_async, write_to_async, ClientMsg, use log::{error, info, warn}; use std::net::SocketAddr; use tokio::{ - io::{AsyncRead, AsyncWrite}, + io::{AsyncRead, AsyncWrite, AsyncWriteExt}, net::TcpStream, }; use tokio_rustls::TlsAcceptor; @@ -27,8 +27,10 @@ pub async fn handle_connection( } }; let _ = handle_requests(&mut tls_stream, state, origin).await; + let _ = tls_stream.shutdown().await; } else { let _ = handle_requests(&mut tcp_stream, state, origin).await; + let _ = tcp_stream.shutdown().await; } } diff --git a/gday_server/tests/test_integration.rs b/gday_server/tests/test_integration.rs index 93dd798..8aa6e00 100644 --- a/gday_server/tests/test_integration.rs +++ b/gday_server/tests/test_integration.rs @@ -228,10 +228,10 @@ async fn test_request_limit() { }, &mut stream_v4, ); - assert!(matches!( - result, - Err(gday_contact_exchange_protocol::Error::IO(_)) - )); + assert!( + matches!(result, Err(gday_contact_exchange_protocol::Error::IO(_))), + "Expected Error::IO, got {result:?}." + ); // ensure other connections are unaffected write_to( From 18cbb6ea6bdea2c05cd613032bd617e15c9353f5 Mon Sep 17 00:00:00 2001 From: Marcin Anforowicz Date: Tue, 10 Dec 2024 11:10:48 -0800 Subject: [PATCH 2/6] Changed eof test. --- gday_encryption/src/helper_buf.rs | 10 +++++----- gday_server/tests/test_integration.rs | 13 +++---------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/gday_encryption/src/helper_buf.rs b/gday_encryption/src/helper_buf.rs index a304058..236349c 100644 --- a/gday_encryption/src/helper_buf.rs +++ b/gday_encryption/src/helper_buf.rs @@ -129,7 +129,7 @@ pub struct HelperBufPart<'a> { start_i: usize, } -impl<'a> aead::Buffer for HelperBufPart<'a> { +impl aead::Buffer for HelperBufPart<'_> { /// Extends the [`HelperBufPart`] with `other`. /// - Returns an [`aead::Error`] if there's not enough capacity. fn extend_from_slice(&mut self, other: &[u8]) -> aead::Result<()> { @@ -153,7 +153,7 @@ impl<'a> aead::Buffer for HelperBufPart<'a> { // The 4 following impls let the user treat this // struct as a slice with the data-containing portion -impl<'a> Deref for HelperBufPart<'a> { +impl Deref for HelperBufPart<'_> { type Target = [u8]; fn deref(&self) -> &Self::Target { @@ -161,19 +161,19 @@ impl<'a> Deref for HelperBufPart<'a> { } } -impl<'a> DerefMut for HelperBufPart<'a> { +impl DerefMut for HelperBufPart<'_> { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.parent.inner[self.start_i..self.parent.r_cursor] } } -impl<'a> AsRef<[u8]> for HelperBufPart<'a> { +impl AsRef<[u8]> for HelperBufPart<'_> { fn as_ref(&self) -> &[u8] { &self.parent.inner[self.start_i..self.parent.r_cursor] } } -impl<'a> AsMut<[u8]> for HelperBufPart<'a> { +impl AsMut<[u8]> for HelperBufPart<'_> { fn as_mut(&mut self) -> &mut [u8] { &mut self.parent.inner[self.start_i..self.parent.r_cursor] } diff --git a/gday_server/tests/test_integration.rs b/gday_server/tests/test_integration.rs index 8aa6e00..60e9e70 100644 --- a/gday_server/tests/test_integration.rs +++ b/gday_server/tests/test_integration.rs @@ -1,6 +1,8 @@ #![forbid(unsafe_code)] #![warn(clippy::all)] +use std::io::Read; + use gday_contact_exchange_protocol::{read_from, write_to, ClientMsg, Contact, ServerMsg}; #[tokio::test] @@ -222,16 +224,7 @@ async fn test_request_limit() { assert_eq!(response, ServerMsg::ErrorTooManyRequests); // ensure the server closed the connection - let result = write_to( - ClientMsg::CreateRoom { - room_code: [100; 32], - }, - &mut stream_v4, - ); - assert!( - matches!(result, Err(gday_contact_exchange_protocol::Error::IO(_))), - "Expected Error::IO, got {result:?}." - ); + assert_eq!(stream_v4.read(&mut [0, 0]).unwrap(), 0); // ensure other connections are unaffected write_to( From f7e3e6a939fd592a8aa84768a14f8b0a4af41783 Mon Sep 17 00:00:00 2001 From: Marcin Anforowicz Date: Tue, 10 Dec 2024 18:05:27 -0800 Subject: [PATCH 3/6] GitHub action fix. --- .github/workflows/check.yml | 45 +++++++++++++++++++++++++++++++++++++ .github/workflows/test.yml | 30 +++++-------------------- 2 files changed, 51 insertions(+), 24 deletions(-) create mode 100644 .github/workflows/check.yml diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml new file mode 100644 index 0000000..a0c560b --- /dev/null +++ b/.github/workflows/check.yml @@ -0,0 +1,45 @@ +# GitHub workflow which runs on pushes. +# Does multiple checks with nightly Rust on Linux. + +name: All Checks + +on: + push: + +env: + CARGO_TERM_COLOR: always + +jobs: + build_and_test: + name: All Checks + runs-on: ubuntu-latest + strategy: + matrix: + toolchain: + - nightly + steps: + - uses: actions/checkout@v3 + + - name: Update rustup + run: rustup update + + - name: Build + run: cargo build --verbose + + - name: Test + run: cargo test --verbose + + - name: Clippy + run: cargo clippy --verbose + + - name: Check documentation + run: cargo doc --no-deps --verbose + + - name: Check formatting + run: cargo fmt --check --verbose + + - name: Install cargo audit + run: cargo install --locked cargo-audit + + - name: Audit dependencies + run: cargo audit diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2697858..5a9a984 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,29 +1,26 @@ -# GitHub workflow which automatically -# tests code on pushes and pull requests. +# GitHub workflow which runs on pushes. +# Does a build and test with stable Rust on 3 different platforms. -name: Cargo Build & Test +name: Stable Build & Test on: push: - # pull_request: env: CARGO_TERM_COLOR: always jobs: build_and_test: - name: Rust Build & Test + name: Stable Build & Test runs-on: ${{ matrix.os }} strategy: matrix: os: - # - ubuntu-latest - # - windows-latest + - ubuntu-latest + - windows-latest - macos-latest toolchain: - stable - # - beta - # - nightly steps: - uses: actions/checkout@v3 @@ -35,18 +32,3 @@ jobs: - name: Test run: cargo test --verbose - - - name: Clippy - run: cargo clippy --verbose - - - name: Check documentation - run: cargo doc --no-deps --verbose - - - name: Check formatting - run: cargo fmt --check --verbose - - - name: Install cargo audit - run: cargo install --locked cargo-audit - - - name: Audit dependencies - run: cargo audit From 852934075e1f42c20cdcf2d9b062daf9e010b8b5 Mon Sep 17 00:00:00 2001 From: Marcin Anforowicz Date: Tue, 10 Dec 2024 18:18:06 -0800 Subject: [PATCH 4/6] Removed redundant shutdown calls in tests. --- gday/src/main.rs | 4 ++-- gday_encryption/tests/test_integration.rs | 3 --- gday_server/src/connection_handler.rs | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/gday/src/main.rs b/gday/src/main.rs index 8b8810a..5fd466c 100644 --- a/gday/src/main.rs +++ b/gday/src/main.rs @@ -195,7 +195,7 @@ async fn run(args: crate::Args) -> Result<(), Box> { .await .map_err(|_| gday_hole_punch::Error::HolePunchTimeout)??; - // Gracefully close the server connection + // Gracefully terminate TLS server_connection.shutdown().await?; let mut stream = EncryptedStream::encrypt_connection(stream, &shared_key).await?; @@ -264,7 +264,7 @@ async fn run(args: crate::Args) -> Result<(), Box> { .await .map_err(|_| gday_hole_punch::Error::HolePunchTimeout)??; - // Gracefully close the server connection + // Gracefully terminate TLS server_connection.shutdown().await?; let mut stream = EncryptedStream::encrypt_connection(stream, &shared_key).await?; diff --git a/gday_encryption/tests/test_integration.rs b/gday_encryption/tests/test_integration.rs index 1719bdf..c8fed4b 100644 --- a/gday_encryption/tests/test_integration.rs +++ b/gday_encryption/tests/test_integration.rs @@ -38,8 +38,6 @@ async fn test_transfers() { stream_a.write_all(chunk).await.unwrap(); stream_a.flush().await.unwrap(); } - // Ensure calling shutdown multiple times works - stream_a.shutdown().await.unwrap(); stream_a.shutdown().await.unwrap(); }); @@ -97,7 +95,6 @@ async fn test_bufread() { } stream_a.shutdown().await.unwrap(); - stream_a.shutdown().await.unwrap(); }); // Stream that will receive the test data sent to the loopback address. diff --git a/gday_server/src/connection_handler.rs b/gday_server/src/connection_handler.rs index 8a4167d..8024dad 100644 --- a/gday_server/src/connection_handler.rs +++ b/gday_server/src/connection_handler.rs @@ -27,10 +27,10 @@ pub async fn handle_connection( } }; let _ = handle_requests(&mut tls_stream, state, origin).await; + // Graceful TLS termination let _ = tls_stream.shutdown().await; } else { let _ = handle_requests(&mut tcp_stream, state, origin).await; - let _ = tcp_stream.shutdown().await; } } From 8078280e8874c09d3c369a5689ff6be6eec0db79 Mon Sep 17 00:00:00 2001 From: Marcin Anforowicz Date: Tue, 10 Dec 2024 18:34:53 -0800 Subject: [PATCH 5/6] Fixing loopback address for Windows test. --- gday_encryption/tests/test_integration.rs | 4 ++-- gday_hole_punch/tests/test_integration.rs | 2 +- gday_server/tests/test_integration.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gday_encryption/tests/test_integration.rs b/gday_encryption/tests/test_integration.rs index c8fed4b..564fc95 100644 --- a/gday_encryption/tests/test_integration.rs +++ b/gday_encryption/tests/test_integration.rs @@ -22,7 +22,7 @@ async fn test_transfers() { let chunk_size = 200_000; // Listens on the loopback address - let listener = tokio::net::TcpListener::bind("[::]:0").await.unwrap(); + let listener = tokio::net::TcpListener::bind("[::1]:0").await.unwrap(); let pipe_addr = listener.local_addr().unwrap(); // A thread that will send data to the loopback address @@ -77,7 +77,7 @@ async fn test_bufread() { let chunk_size = 200_000; // Listens on the loopback address - let listener = tokio::net::TcpListener::bind("[::]:0").await.unwrap(); + let listener = tokio::net::TcpListener::bind("[::1]:0").await.unwrap(); let pipe_addr = listener.local_addr().unwrap(); // A thread that will send data to the loopback address diff --git a/gday_hole_punch/tests/test_integration.rs b/gday_hole_punch/tests/test_integration.rs index be2941b..b0ecd97 100644 --- a/gday_hole_punch/tests/test_integration.rs +++ b/gday_hole_punch/tests/test_integration.rs @@ -12,7 +12,7 @@ async fn test_integration() { key: None, certificate: None, unencrypted: true, - addresses: vec!["0.0.0.0:0".parse().unwrap(), "[::]:0".parse().unwrap()], + addresses: vec!["127.0.0.1:0".parse().unwrap(), "[::1]:0".parse().unwrap()], timeout: 3600, request_limit: 10, verbosity: log::LevelFilter::Off, diff --git a/gday_server/tests/test_integration.rs b/gday_server/tests/test_integration.rs index 60e9e70..d95ac24 100644 --- a/gday_server/tests/test_integration.rs +++ b/gday_server/tests/test_integration.rs @@ -12,7 +12,7 @@ async fn test_integration() { key: None, certificate: None, unencrypted: true, - addresses: vec!["0.0.0.0:0".parse().unwrap(), "[::]:0".parse().unwrap()], + addresses: vec!["127.0.0.1:0".parse().unwrap(), "[::1]:0".parse().unwrap()], timeout: 3600, request_limit: 10, verbosity: log::LevelFilter::Off, @@ -185,7 +185,7 @@ async fn test_request_limit() { key: None, certificate: None, unencrypted: true, - addresses: vec!["0.0.0.0:0".parse().unwrap(), "[::]:0".parse().unwrap()], + addresses: vec!["127.0.0.1:0".parse().unwrap(), "[::1]:0".parse().unwrap()], timeout: 3600, request_limit: 10, verbosity: log::LevelFilter::Off, From a2c8aa3f5b91aecd9bfd3de128142cd1cdfc2950 Mon Sep 17 00:00:00 2001 From: Marcin Anforowicz Date: Tue, 10 Dec 2024 18:43:50 -0800 Subject: [PATCH 6/6] Changed test timing. --- gday_server/src/state.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gday_server/src/state.rs b/gday_server/src/state.rs index 8aee3c1..a6d6330 100644 --- a/gday_server/src/state.rs +++ b/gday_server/src/state.rs @@ -416,7 +416,7 @@ mod tests { #[tokio::test] async fn test_room_timeout() { - let mut state1 = State::new(100, Duration::from_millis(10)); + let mut state1 = State::new(100, Duration::from_millis(30)); let mut state2 = state1.clone(); let origin1 = IpAddr::V4(123.into()); @@ -440,7 +440,7 @@ mod tests { .unwrap(); // wait for the room to time out - tokio::time::sleep(Duration::from_millis(20)).await; + tokio::time::sleep(Duration::from_millis(300)).await; // confirm this room has been removed let result = state2.update_client(ROOM, false, example_endpoint, false, origin2);