diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index ebbece8..5081b8e 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -1,4 +1,4 @@ -name: Coverage +name: coverage on: [push, pull_request] @@ -6,27 +6,25 @@ env: CARGO_TERM_COLOR: always jobs: - build: + coverage: + name: Generate coverage runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - name: Setup nightly toolchain + - name: Checkout repository + uses: actions/checkout@v2 + - name: Setup rust toolchain uses: dtolnay/rust-toolchain@stable with: - toolchain: nightly - - name: Run tests (nightly) - run: cargo test --lib --package suppaftp --no-default-features --features native-tls,deprecated,async-native-tls --no-fail-fast - env: - RUST_LOG: trace - CARGO_INCREMENTAL: "0" - RUSTFLAGS: "-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Cpanic=abort -Zpanic_abort_tests" - RUSTDOCFLAGS: "-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Cpanic=abort -Zpanic_abort_tests" - - name: Coverage with grcov - id: coverage - uses: actions-rs/grcov@v0.1 + toolchain: stable + - uses: taiki-e/install-action@v2 + with: + tool: cargo-llvm-cov + - name: Run tests + run: cargo llvm-cov --no-fail-fast --no-default-features --features native-tls,deprecated,async-native-tls --workspace --lcov --output-path lcov.info - name: Coveralls - uses: coverallsapp/github-action@v1.1.1 + uses: coverallsapp/github-action@v2.3.6 with: github-token: ${{ secrets.GITHUB_TOKEN }} - path-to-lcov: ${{ steps.coverage.outputs.report }} + file: lcov.info + # currently we only run one coverage report per build + parallel: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c7e5f2..5323e12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Changelog - [Changelog](#changelog) + - [6.1.1](#611) + - [6.1.0](#610) - [6.0.7](#607) - [6.0.6](#606) - [6.0.5](#605) @@ -40,6 +42,19 @@ --- +## 6.1.1 + +Released on 17/03/2025 + +- added a couple of logs to debug streams. + +## 6.1.0 + +Released on 10/03/2025 + +- [Issue 100](https://github.com/veeso/suppaftp/issues/100): Migrated away from unmaintained `async-tls` to `futures-rustls` +- [Issue 98](https://github.com/veeso/suppaftp/issues/98): doc: fixed minor typos that referenced `termscp` + ## 6.0.7 - [Issue 88](https://github.com/veeso/suppaftp/issues/88): Removed `ip.is_private()` check on NAT workaround, which prevented public IPs to be used for Natting. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1342ab7..e80d1c2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,7 +37,7 @@ Check the issue is always assigned to `veeso`. ### Bug reports -If you want to report an issue or a bug you've encountered while using termscp, open an issue using the `Bug report` template. +If you want to report an issue or a bug you've encountered while using suppaftp, open an issue using the `Bug report` template. The `Bug` label should already be set and the issue should already be assigned to `veeso`. When you open a bug try to be the most precise as possible in describing your issue. I'm not saying you should always be that precise, since sometimes it's very easy for maintainers to understand what you're talking about. Just try to be reasonable to understand sometimes we might not know what you're talking about or we just don't have the technical knowledge you might think. @@ -53,7 +53,7 @@ Maintainers will may add additional labels to your issue: ### Feature requests Whenever you have a good idea which chould improve the project, it is a good idea to submit it to the project owner. -The first thing you should do though, is not starting to write the code, but is to become concern about how termscp works, what kind +The first thing you should do though, is not starting to write the code, but is to become concern about how suppaftp works, what kind of contribution I appreciate and what kind of contribution I won't consider. Said so, follow these steps: @@ -101,7 +101,7 @@ Let's make it simple and clear: In addition to the process described for the PRs, I've also decided to introduce a list of guidelines to follow when writing the code, that should be followed: -1. **Let's stop the NPM apocalypse**: personally I'm against the abuse of dependencies we make in software projects and I think that NodeJS has opened the way to this drama (and has already gone too far). Nowadays nobody cares about adding hundreds of dependencies to their projects. Don't misunderstand me: I think that package managers are cool, but I'm totally against the abuse we're making of them. I think when we work on a project, we should try to use the minor quantity of dependencies as possible, especially because it's not hard to see how many libraries are getting abandoned right now, causing compatibility issues after a while. So please, when working on termscp, try not to add useless dependencies. +1. **Let's stop the NPM apocalypse**: personally I'm against the abuse of dependencies we make in software projects and I think that NodeJS has opened the way to this drama (and has already gone too far). Nowadays nobody cares about adding hundreds of dependencies to their projects. Don't misunderstand me: I think that package managers are cool, but I'm totally against the abuse we're making of them. I think when we work on a project, we should try to use the minor quantity of dependencies as possible, especially because it's not hard to see how many libraries are getting abandoned right now, causing compatibility issues after a while. So please, when working on suppaftp, try not to add useless dependencies. 2. **No C-bindings**: personally I think that Rust still relies too much on C. And that's bad, really bad. Many libraries in Rust are just wrappers to C libraries, which is a huge problem, especially considering this is a multiplatform project. Everytime you add a C-binding to your project, you're forcing your users to install additional libraries to their systems. Sometimes these libraries are already installed on their systems (as happens for libssh2 or openssl in this case), but sometimes not. So if you really have to add a dependency to this project, please AVOID completely adding C-bounded libraries. 3. **Test units matter**: Whenever you implement something new to this project, always implement test units which cover the most cases as possible. 4. **Comments are useful**: Many people say that the code should be that simple to talk by itself about what it does, and comments should then be useless. I personally don't agree. I'm not saying they're wrong, but I'm just saying that this approach has, in my personal opinion, many aspects which are underrated: diff --git a/Cargo.toml b/Cargo.toml index fec9247..b50c712 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["suppaftp", "suppaftp-cli"] resolver = "2" [workspace.package] -version = "6.0.7" +version = "6.1.1" edition = "2021" rust-version = "1.71.1" authors = [ diff --git a/README.md b/README.md index bbdb581..d91ef00 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@

Developed by veeso and Matt McCoy

-

Current version: 6.0.7 (18/01/2025)

+

Current version: 6.1.1 (17/03/2025)

💡 If you don't know what to choose, `native-tls` should be preferred for compatibility reasons. +> [!NOTE] +> 💡 If you don't know what to choose, `native-tls` should be preferred for compatibility reasons. > ❗ If you want to link libssl statically, enable feature `native-tls-vendored` #### Async support @@ -148,8 +149,9 @@ If you want to enable **async** support, you must enable `async` feature in your suppaftp = { version = "^6", features = ["async"] } ``` -> ⚠️ If you want to enable both **native-tls** and **async** you must use the **async-native-tls** feature ⚠️ -> ⚠️ If you want to enable both **rustls** and **async** you must use the **async-rustls** feature ⚠️ +> [!CAUTION] +> ⚠️ If you want to enable both **native-tls** and **async** you must use the **async-native-tls** feature ⚠️ +> ⚠️ If you want to enable both **rustls** and **async** you must use the **async-rustls** feature ⚠️ > ❗ If you want to link libssl statically, enable feature `async-native-tls-vendored` #### Deprecated methods diff --git a/suppaftp/Cargo.toml b/suppaftp/Cargo.toml index d8f09ba..130859c 100644 --- a/suppaftp/Cargo.toml +++ b/suppaftp/Cargo.toml @@ -33,8 +33,9 @@ thiserror = "^2" async-std = { version = "^1.10", optional = true } async-native-tls-crate = { package = "async-native-tls", version = "^0.5", optional = true } async-trait = { version = "0.1.64", optional = true } -async-tls = { version = "^0.13", optional = true } +futures-rustls = { version = "^0.26", optional = true } pin-project = { version = "^1", optional = true } +rustls-pki-types = { version = "1", optional = true, features = ["alloc"] } # secure native-tls-crate = { package = "native-tls", version = "^0.2", optional = true } rustls-crate = { package = "rustls", version = "^0.23", default-features = false, features = [ @@ -43,7 +44,7 @@ rustls-crate = { package = "rustls", version = "^0.23", default-features = false "std", "tls12", ], optional = true } -futures-lite = "2.0.0" +futures-lite = "2" [dev-dependencies] async-attributes = "1.1.2" @@ -57,10 +58,10 @@ webpki-roots = "0.26" [features] default = [] # Enable async support for suppaftp -async = ["async-std", "async-trait", "pin-project"] +async = ["dep:async-std", "dep:async-trait", "dep:pin-project"] async-default-tls = ["async-native-tls"] -async-native-tls = ["async-native-tls-crate", "async-secure"] -async-rustls = ["async-tls", "async-secure"] +async-native-tls = ["dep:async-native-tls-crate", "async-secure"] +async-rustls = ["dep:futures-rustls", "dep:rustls-pki-types", "async-secure"] async-secure = ["async"] # Enable deprecated FTP/FTPS methods diff --git a/suppaftp/src/async_ftp/mod.rs b/suppaftp/src/async_ftp/mod.rs index 8f7a7e5..07bbadd 100644 --- a/suppaftp/src/async_ftp/mod.rs +++ b/suppaftp/src/async_ftp/mod.rs @@ -8,7 +8,7 @@ mod tls; use std::future::Future; #[cfg(not(feature = "async-secure"))] use std::marker::PhantomData; -use std::net::{Ipv4Addr, SocketAddr}; +use std::net::SocketAddr; use std::pin::Pin; use std::string::String; use std::time::Duration; @@ -29,13 +29,14 @@ pub use tls::{AsyncNativeTlsConnector, AsyncNativeTlsStream}; #[cfg(feature = "async-rustls")] pub use tls::{AsyncRustlsConnector, AsyncRustlsStream}; -use super::regex::{EPSV_PORT_RE, MDTM_RE, PASV_PORT_RE, SIZE_RE}; +use super::regex::{EPSV_PORT_RE, MDTM_RE, SIZE_RE}; use super::types::{FileType, FtpError, FtpResult, Mode, Response}; use super::Status; use crate::command::Command; #[cfg(feature = "async-secure")] use crate::command::ProtectionLevel; use crate::types::Features; +use crate::FtpStream; /// A function that creates a new stream for the data connection in passive mode. /// @@ -850,24 +851,7 @@ where self.perform(Command::Pasv).await?; // PASV response format : 227 Entering Passive Mode (h1,h2,h3,h4,p1,p2). let response: Response = self.read_response(Status::PassiveMode).await?; - let response_str = response.as_string().map_err(|_| FtpError::BadResponse)?; - let caps = PASV_PORT_RE - .captures(&response_str) - .ok_or_else(|| FtpError::UnexpectedResponse(response.clone()))?; - // If the regex matches we can be sure groups contains numbers - let (oct1, oct2, oct3, oct4) = ( - caps[1].parse::().unwrap(), - caps[2].parse::().unwrap(), - caps[3].parse::().unwrap(), - caps[4].parse::().unwrap(), - ); - let (msb, lsb) = ( - caps[5].parse::().unwrap(), - caps[6].parse::().unwrap(), - ); - let ip = Ipv4Addr::new(oct1, oct2, oct3, oct4); - let port = (u16::from(msb) << 8) | u16::from(lsb); - let addr = SocketAddr::new(ip.into(), port); + let addr = FtpStream::parse_passive_address_from_response(response)?; trace!("Passive address: {}", addr); if self.nat_workaround { let mut remote = self @@ -876,7 +860,7 @@ where .get_ref() .peer_addr() .map_err(FtpError::ConnectionError)?; - remote.set_port(port); + remote.set_port(addr.port()); trace!("Replacing site local address {} with {}", addr, remote); Ok(remote) } else { @@ -922,6 +906,7 @@ where match data_stream.read_line(&mut line).await { Ok(0) => break, Ok(_) => { + trace!("STREAM IN: {:?}", line); if line.ends_with('\n') { line.pop(); if line.ends_with('\r') { @@ -933,7 +918,10 @@ where } lines.push(line); } - Err(_) => return Err(FtpError::BadResponse), + Err(err) => { + error!("failed to get lines from stream: {err}"); + return Err(FtpError::BadResponse); + } } } trace!("Lines from stream {:?}", lines); diff --git a/suppaftp/src/async_ftp/tls/rustls.rs b/suppaftp/src/async_ftp/tls/rustls.rs index 78e9f7a..5c82e7f 100644 --- a/suppaftp/src/async_ftp/tls/rustls.rs +++ b/suppaftp/src/async_ftp/tls/rustls.rs @@ -6,10 +6,11 @@ use std::pin::Pin; use async_std::io::{Read, Write}; use async_std::net::TcpStream; -use async_tls::client::TlsStream; -use async_tls::TlsConnector as RustlsTlsConnector; use async_trait::async_trait; +use futures_rustls::client::TlsStream; +use futures_rustls::TlsConnector as RustlsTlsConnector; use pin_project::pin_project; +use rustls_pki_types::{DnsName, ServerName}; use super::{AsyncTlsConnector, AsyncTlsStream}; use crate::{FtpError, FtpResult}; @@ -36,8 +37,13 @@ impl AsyncTlsConnector for AsyncRustlsConnector { type Stream = AsyncRustlsStream; async fn connect(&self, domain: &str, stream: TcpStream) -> FtpResult { + let server_name = ServerName::DnsName( + DnsName::try_from(domain.to_string()) + .map_err(|e| FtpError::SecureError(e.to_string()))?, + ); + self.connector - .connect(domain, stream) + .connect(server_name, stream) .await .map(AsyncRustlsStream::from) .map_err(|e| FtpError::SecureError(e.to_string())) @@ -95,7 +101,7 @@ impl AsyncTlsStream for AsyncRustlsStream { type InnerStream = TlsStream; fn get_ref(&self) -> &TcpStream { - self.stream.get_ref() + self.stream.get_ref().0 } fn mut_ref(&mut self) -> &mut Self::InnerStream { @@ -103,6 +109,6 @@ impl AsyncTlsStream for AsyncRustlsStream { } fn tcp_stream(self) -> TcpStream { - self.stream.get_ref().clone() + self.stream.get_ref().0.clone() } } diff --git a/suppaftp/src/lib.rs b/suppaftp/src/lib.rs index 2b2121c..23ff698 100644 --- a/suppaftp/src/lib.rs +++ b/suppaftp/src/lib.rs @@ -224,8 +224,14 @@ pub type AsyncRustlsFtpStream = ImplAsyncFtpStream; // -- test logging #[cfg(test)] pub fn log_init() { - let _ = env_logger::builder() - .is_test(true) - .filter_level(log::LevelFilter::Debug) - .try_init(); + use std::sync::Once; + + static INIT: Once = Once::new(); + + INIT.call_once(|| { + let _ = env_logger::builder() + .filter_level(log::LevelFilter::Trace) + .is_test(true) + .try_init(); + }); } diff --git a/suppaftp/src/list.rs b/suppaftp/src/list.rs index 3afe8cd..ece6596 100644 --- a/suppaftp/src/list.rs +++ b/suppaftp/src/list.rs @@ -146,7 +146,7 @@ impl File { self.size } - //// Returns the last time the file was modified + /// Returns the last time the file was modified pub fn modified(&self) -> SystemTime { self.modified } diff --git a/suppaftp/src/sync_ftp/mod.rs b/suppaftp/src/sync_ftp/mod.rs index 72da85a..f8de664 100644 --- a/suppaftp/src/sync_ftp/mod.rs +++ b/suppaftp/src/sync_ftp/mod.rs @@ -770,6 +770,7 @@ where match data_stream.read_line(&mut line) { Ok(0) => break, Ok(_) => { + trace!("STREAM IN: {:?}", line); if line.ends_with('\n') { line.pop(); if line.ends_with('\r') { @@ -781,7 +782,10 @@ where } lines.push(line); } - Err(_) => return Err(FtpError::BadResponse), + Err(err) => { + error!("failed to get lines from stream: {err}"); + return Err(FtpError::BadResponse); + } } } trace!("Lines from stream {:?}", lines); @@ -969,8 +973,28 @@ where debug!("PASV command"); self.perform(Command::Pasv)?; // PASV response format : 227 Entering Passive Mode (h1,h2,h3,h4,p1,p2). - let response: Response = self.read_response(Status::PassiveMode)?; + let response = self.read_response(Status::PassiveMode)?; + let addr = Self::parse_passive_address_from_response(response)?; + trace!("Passive address: {addr}",); + if self.nat_workaround { + let mut remote = self + .reader + .get_ref() + .get_ref() + .peer_addr() + .map_err(FtpError::ConnectionError)?; + remote.set_port(addr.port()); + trace!("Replacing site local address {} with {}", addr, remote); + Ok(remote) + } else { + Ok(addr) + } + } + + /// Parse passive address from response + pub(crate) fn parse_passive_address_from_response(response: Response) -> FtpResult { let response_str = response.as_string().map_err(|_| FtpError::BadResponse)?; + trace!("PASV response: {response_str}",); let caps = PASV_PORT_RE .captures(&response_str) .ok_or_else(|| FtpError::UnexpectedResponse(response.clone()))?; @@ -988,20 +1012,8 @@ where let ip = Ipv4Addr::new(oct1, oct2, oct3, oct4); let port = (u16::from(msb) << 8) | u16::from(lsb); let addr = SocketAddr::new(ip.into(), port); - trace!("Passive address: {}", addr); - if self.nat_workaround { - let mut remote = self - .reader - .get_ref() - .get_ref() - .peer_addr() - .map_err(FtpError::ConnectionError)?; - remote.set_port(port); - trace!("Replacing site local address {} with {}", addr, remote); - Ok(remote) - } else { - Ok(addr) - } + + Ok(addr) } /// Execute a command which returns list of strings in a separate stream @@ -1022,6 +1034,7 @@ where #[cfg(test)] mod test { + use std::net::IpAddr; use std::sync::Arc; #[cfg(feature = "secure")] @@ -1041,6 +1054,41 @@ mod test { with_test_ftp_stream(|_stream| {}); } + #[test] + fn test_should_parse_passive_address_from_response() { + let response = vec![ + 50, 50, 55, 32, 69, 110, 116, 101, 114, 105, 110, 103, 32, 80, 97, 115, 115, 105, 118, + 101, 32, 77, 111, 100, 101, 32, 40, 49, 50, 55, 44, 48, 44, 48, 44, 49, 44, 49, 49, 55, + 44, 53, 54, 41, 13, 10, + ]; + let response = Response::new(Status::PassiveMode, response); + + let address = FtpStream::parse_passive_address_from_response(response) + .expect("Failed to parse passive address"); + assert_eq!( + address.ip(), + IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), + "IP address is not correct" + ); + assert_eq!(address.port(), 30008, "Port is not correct"); + + let response = vec![ + 50, 50, 55, 32, 69, 110, 116, 101, 114, 105, 110, 103, 32, 80, 97, 115, 115, 105, 118, + 101, 32, 77, 111, 100, 101, 32, 40, 53, 56, 44, 50, 52, 55, 44, 57, 50, 44, 49, 50, 50, + 44, 49, 52, 54, 44, 50, 51, 57, 41, 46, 13, 10, + ]; + let response = Response::new(Status::PassiveMode, response); + + let address = FtpStream::parse_passive_address_from_response(response) + .expect("Failed to parse passive address"); + assert_eq!( + address.ip(), + IpAddr::V4(Ipv4Addr::new(58, 247, 92, 122)), + "IP address is not correct" + ); + assert_eq!(address.port(), 37615, "Port is not correct"); + } + #[test] #[serial] fn should_change_mode() { @@ -1163,7 +1211,6 @@ mod test { } #[test] - fn should_transfer_file() { with_test_ftp_stream(|stream| { // Set transfer type to Binary