From c197df7df4045abdc2d4e8077781249dd83dbdd1 Mon Sep 17 00:00:00 2001 From: Matteo Bigoi Date: Sun, 22 Nov 2020 17:16:40 +0000 Subject: Handle more precise SFTP error codes (#203) * Handle more precise SFTP error codes * Allow set-env in macos github action * Bump to 0.9 since the Error interface has changed, hence this is a breaking change --- src/sftp.rs | 205 ++++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 143 insertions(+), 62 deletions(-) (limited to 'src/sftp.rs') diff --git a/src/sftp.rs b/src/sftp.rs index 2ef2af8..323bc6f 100644 --- a/src/sftp.rs +++ b/src/sftp.rs @@ -1,5 +1,6 @@ use libc::{c_int, c_long, c_uint, c_ulong, size_t}; use parking_lot::{Mutex, MutexGuard}; +use std::convert::TryFrom; use std::io::prelude::*; use std::io::{self, ErrorKind, SeekFrom}; use std::mem; @@ -7,7 +8,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use util; -use {raw, Error, SessionInner}; +use {raw, Error, ErrorCode, SessionInner}; /// A handle to a remote filesystem over SFTP. /// @@ -173,7 +174,8 @@ impl Sftp { open_type as c_int, ); if ret.is_null() { - Err(locked.sess.last_error().unwrap_or_else(Error::unknown)) + let rc = raw::libssh2_session_last_errno(locked.sess.raw); + Err(Self::error_code_into_error(locked.sess.raw, locked.raw, rc)) } else { Ok(File::from_raw(self, ret)) } @@ -216,7 +218,7 @@ impl Sftp { ret.push((dirname.join(&filename), stat)) } - Err(ref e) if e.code() == raw::LIBSSH2_ERROR_FILE => break, + Err(ref e) if e.code() == ErrorCode::Session(raw::LIBSSH2_ERROR_FILE) => break, Err(e) => return Err(e), } } @@ -227,7 +229,7 @@ impl Sftp { pub fn mkdir(&self, filename: &Path, mode: i32) -> Result<(), Error> { let filename = util::path2bytes(filename)?; let locked = self.lock()?; - locked.sess.rc(unsafe { + Self::rc(&locked, unsafe { raw::libssh2_sftp_mkdir_ex( locked.raw, filename.as_ptr() as *const _, @@ -256,15 +258,17 @@ impl Sftp { let locked = self.lock()?; unsafe { let mut ret = mem::zeroed(); - let rc = raw::libssh2_sftp_stat_ex( - locked.raw, - filename.as_ptr() as *const _, - filename.len() as c_uint, - raw::LIBSSH2_SFTP_STAT, - &mut ret, - ); - locked.sess.rc(rc)?; - Ok(FileStat::from_raw(&ret)) + Self::rc( + &locked, + raw::libssh2_sftp_stat_ex( + locked.raw, + filename.as_ptr() as *const _, + filename.len() as c_uint, + raw::LIBSSH2_SFTP_STAT, + &mut ret, + ), + ) + .map(|_| FileStat::from_raw(&ret)) } } @@ -274,15 +278,17 @@ impl Sftp { let locked = self.lock()?; unsafe { let mut ret = mem::zeroed(); - let rc = raw::libssh2_sftp_stat_ex( - locked.raw, - filename.as_ptr() as *const _, - filename.len() as c_uint, - raw::LIBSSH2_SFTP_LSTAT, - &mut ret, - ); - locked.sess.rc(rc)?; - Ok(FileStat::from_raw(&ret)) + Self::rc( + &locked, + raw::libssh2_sftp_stat_ex( + locked.raw, + filename.as_ptr() as *const _, + filename.len() as c_uint, + raw::LIBSSH2_SFTP_LSTAT, + &mut ret, + ), + ) + .map(|_| FileStat::from_raw(&ret)) } } @@ -290,7 +296,7 @@ impl Sftp { pub fn setstat(&self, filename: &Path, stat: FileStat) -> Result<(), Error> { let filename = util::path2bytes(filename)?; let locked = self.lock()?; - locked.sess.rc(unsafe { + Self::rc(&locked, unsafe { let mut raw = stat.raw(); raw::libssh2_sftp_stat_ex( locked.raw, @@ -352,12 +358,10 @@ impl Sftp { break; } } - if rc < 0 { - Err(Error::from_session_error_raw(locked.sess.raw, rc)) - } else { + Self::rc(&locked, rc).map(move |_| { unsafe { ret.set_len(rc as usize) } - Ok(mkpath(ret)) - } + mkpath(ret) + }) } /// Rename a filesystem object on the remote filesystem. @@ -378,7 +382,7 @@ impl Sftp { let src = util::path2bytes(src)?; let dst = util::path2bytes(dst)?; let locked = self.lock()?; - locked.sess.rc(unsafe { + Self::rc(&locked, unsafe { raw::libssh2_sftp_rename_ex( locked.raw, src.as_ptr() as *const _, @@ -394,7 +398,7 @@ impl Sftp { pub fn unlink(&self, file: &Path) -> Result<(), Error> { let file = util::path2bytes(file)?; let locked = self.lock()?; - locked.sess.rc(unsafe { + Self::rc(&locked, unsafe { raw::libssh2_sftp_unlink_ex(locked.raw, file.as_ptr() as *const _, file.len() as c_uint) }) } @@ -412,7 +416,9 @@ impl Sftp { raw: sftp_inner.raw, }) } - None => Err(Error::from_errno(raw::LIBSSH2_ERROR_BAD_USE)), + None => Err(Error::from_errno(ErrorCode::Session( + raw::LIBSSH2_ERROR_BAD_USE, + ))), } } @@ -440,16 +446,58 @@ impl Sftp { Err(sftp_inner_arc) => { // We are failing shutdown as there are files left open, keep this object usable self.inner = Some(sftp_inner_arc); - Err(Error::from_errno(raw::LIBSSH2_ERROR_BAD_USE)) + Err(Error::from_errno(ErrorCode::Session( + raw::LIBSSH2_ERROR_BAD_USE, + ))) } } } None => { // We have already shut this down. Shutting down twice is a mistake from the caller code - Err(Error::from_errno(raw::LIBSSH2_ERROR_BAD_USE)) + Err(Error::from_errno(ErrorCode::Session( + raw::LIBSSH2_ERROR_BAD_USE, + ))) + } + } + } + + fn error_code_into_error( + session_raw: *mut raw::LIBSSH2_SESSION, + sftp_raw: *mut raw::LIBSSH2_SFTP, + rc: libc::c_int, + ) -> Error { + if rc >= 0 { + Error::unknown() + } else if rc == raw::LIBSSH2_ERROR_SFTP_PROTOCOL { + let actual_rc = unsafe { raw::libssh2_sftp_last_error(sftp_raw) }; + // TODO: This conversion from `c_ulong` to `c_int` should not be + // necessary if the constants `LIBSSH2_FX_*` in the `-sys` crate + // are typed as `c_ulong`, as they should be. + if let Ok(actual_rc) = libc::c_int::try_from(actual_rc) { + Error::from_errno(ErrorCode::SFTP(actual_rc)) + } else { + Error::unknown() } + } else { + Error::from_session_error_raw(session_raw, rc) + } + } + + fn error_code_into_result( + session_raw: *mut raw::LIBSSH2_SESSION, + sftp_raw: *mut raw::LIBSSH2_SFTP, + rc: libc::c_int, + ) -> Result<(), Error> { + if rc >= 0 { + Ok(()) + } else { + Err(Self::error_code_into_error(session_raw, sftp_raw, rc)) } } + + fn rc(locked: &LockedSftp, rc: libc::c_int) -> Result<(), Error> { + Self::error_code_into_result(locked.sess.raw, locked.raw, rc) + } } impl Drop for SftpInnerDropWrapper { @@ -490,7 +538,7 @@ impl File { /// Set the metadata for this handle. pub fn setstat(&mut self, stat: FileStat) -> Result<(), Error> { let locked = self.lock()?; - locked.sess.rc(unsafe { + self.rc(&locked, unsafe { let mut raw = stat.raw(); raw::libssh2_sftp_fstat_ex(locked.raw, &mut raw, 1) }) @@ -501,10 +549,8 @@ impl File { let locked = self.lock()?; unsafe { let mut ret = mem::zeroed(); - locked - .sess - .rc(raw::libssh2_sftp_fstat_ex(locked.raw, &mut ret, 0))?; - Ok(FileStat::from_raw(&ret)) + self.rc(&locked, raw::libssh2_sftp_fstat_ex(locked.raw, &mut ret, 0)) + .map(|_| FileStat::from_raw(&ret)) } } @@ -513,10 +559,8 @@ impl File { let locked = self.lock()?; unsafe { let mut ret = mem::zeroed(); - locked - .sess - .rc(raw::libssh2_sftp_fstatvfs(locked.raw, &mut ret))?; - Ok(ret) + self.rc(&locked, raw::libssh2_sftp_fstatvfs(locked.raw, &mut ret)) + .map(move |_| ret) } } @@ -554,16 +598,19 @@ impl File { break; } } - if rc < 0 { - return Err(Error::from_session_error_raw(locked.sess.raw, rc)); - } else if rc == 0 { - return Err(Error::new(raw::LIBSSH2_ERROR_FILE, "no more files")); + if rc == 0 { + Err(Error::new( + ErrorCode::Session(raw::LIBSSH2_ERROR_FILE), + "no more files", + )) } else { - unsafe { - buf.set_len(rc as usize); - } + self.rc(&locked, rc).map(move |_| { + unsafe { + buf.set_len(rc as usize); + } + (mkpath(buf), FileStat::from_raw(&stat)) + }) } - Ok((mkpath(buf), FileStat::from_raw(&stat))) } /// This function causes the remote server to synchronize the file data and @@ -572,9 +619,7 @@ impl File { /// For this to work requires fsync@openssh.com support on the server. pub fn fsync(&mut self) -> Result<(), Error> { let locked = self.lock()?; - locked - .sess - .rc(unsafe { raw::libssh2_sftp_fsync(locked.raw) }) + self.rc(&locked, unsafe { raw::libssh2_sftp_fsync(locked.raw) }) } fn lock(&self) -> Result { @@ -590,7 +635,9 @@ impl File { raw: file_inner.raw, }) } - None => Err(Error::from_errno(raw::LIBSSH2_ERROR_BAD_USE)), + None => Err(Error::from_errno(ErrorCode::Session( + raw::LIBSSH2_ERROR_BAD_USE, + ))), } } @@ -598,24 +645,48 @@ impl File { pub fn close(&mut self) -> Result<(), Error> { { let locked = self.lock()?; - Error::rc(unsafe { raw::libssh2_sftp_close_handle(locked.raw) })?; + self.rc(&locked, unsafe { + raw::libssh2_sftp_close_handle(locked.raw) + })?; } self.inner = None; Ok(()) } + + fn rc(&self, locked: &LockedFile, rc: libc::c_int) -> Result<(), Error> { + if let Some(file_inner) = self.inner.as_ref() { + let sftp_inner = file_inner.sftp.0.as_ref().expect( + "We are holding an Arc, \ + so nobody could unset this (set on creation)", + ); + Sftp::error_code_into_result(locked.sess.raw, sftp_inner.raw, rc) + } else if rc < 0 { + Err(Error::from_errno(ErrorCode::Session(rc))) + } else { + Ok(()) + } + } } impl Read for File { fn read(&mut self, buf: &mut [u8]) -> io::Result { let locked = self.lock()?; - unsafe { - let rc = - raw::libssh2_sftp_read(locked.raw, buf.as_mut_ptr() as *mut _, buf.len() as size_t); - if rc < 0 { - Err(Error::from_session_error_raw(locked.sess.raw, rc as _).into()) + let rc = unsafe { + raw::libssh2_sftp_read(locked.raw, buf.as_mut_ptr() as *mut _, buf.len() as size_t) + }; + if rc < 0 { + let rc = rc as libc::c_int; + if let Some(file_inner) = self.inner.as_ref() { + let sftp_inner = file_inner.sftp.0.as_ref().expect( + "We are holding an Arc, \ + so nobody could unset this (set on creation)", + ); + Err(Sftp::error_code_into_error(locked.sess.raw, sftp_inner.raw, rc).into()) } else { - Ok(rc as usize) + Err(Error::from_errno(ErrorCode::Session(rc)).into()) } + } else { + Ok(rc as usize) } } } @@ -627,11 +698,21 @@ impl Write for File { raw::libssh2_sftp_write(locked.raw, buf.as_ptr() as *const _, buf.len() as size_t) }; if rc < 0 { - Err(Error::from_session_error_raw(locked.sess.raw, rc as _).into()) + let rc = rc as libc::c_int; + if let Some(file_inner) = self.inner.as_ref() { + let sftp_inner = file_inner.sftp.0.as_ref().expect( + "We are holding an Arc, \ + so nobody could unset this (set on creation)", + ); + Err(Sftp::error_code_into_error(locked.sess.raw, sftp_inner.raw, rc).into()) + } else { + Err(Error::from_errno(ErrorCode::Session(rc)).into()) + } } else { Ok(rc as usize) } } + fn flush(&mut self) -> io::Result<()> { Ok(()) } -- cgit v1.2.3