Parse received errors and other minor error logging improvements

3 files changed, 104 insertions, 57 deletions

diff --git a/src/lib.rs b/src/lib.rs
index f9fc57e..2d5523a 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -69,21 +69,60 @@ impl Tftp {
         buf.push(0x00);
     }
 
+    pub fn parse_error(&self, buf: &[u8]) -> std::io::Error {
+        let mut kind = std::io::ErrorKind::InvalidData;
+        let mut error = String::from("Invalid packet received");
+
+        if buf.len() < 5 {
+            return std::io::Error::new(kind, error);
+        }
+
+        let opcode = u16::from_be_bytes([buf[0], buf[1]]);
+        if opcode != Opcodes::ERROR as u16 {
+            return std::io::Error::new(kind, error);
+        }
+
+        let errorcode = u16::from_be_bytes([buf[2], buf[3]]);
+        error = match String::from_utf8(buf[4 ..].to_vec()) {
+            Ok(e) => e,
+            Err(_) => return std::io::Error::new(kind, error),
+        };
+
+        kind = match errorcode {
+            1 => std::io::ErrorKind::NotFound,
+            2 => std::io::ErrorKind::PermissionDenied,
+            3 => std::io::ErrorKind::UnexpectedEof,
+            4 => std::io::ErrorKind::InvalidData,
+            5 => std::io::ErrorKind::InvalidInput,
+            6 => std::io::ErrorKind::AlreadyExists,
+            7 => std::io::ErrorKind::NotFound,
+            _ => std::io::ErrorKind::InvalidData,
+        };
+
+        return std::io::Error::new(kind, error);
+    }
+
     fn wait_for_ack(&self, sock: &UdpSocket, expected_block: u16) -> Result<bool, io::Error> {
-        let mut buf = [0; 4];
-        match sock.recv(&mut buf) {
-            Ok(_) => (),
+        let mut buf = [0; 512];
+        let len = match sock.recv(&mut buf) {
+            Ok(l) => l,
             Err(ref error) if [io::ErrorKind::WouldBlock, io::ErrorKind::TimedOut].contains(&error.kind()) => {
                 return Ok(false);
             }
             Err(err) => return Err(err),
         };
 
+        if len < 4 {
+            return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "invalid data received"));
+        }
+
         let opcode = u16::from_be_bytes([buf[0], buf[1]]);
         let block_nr = u16::from_be_bytes([buf[2], buf[3]]);
 
         if opcode == Opcodes::ACK as u16 && block_nr == expected_block {
             return Ok(true)
+        } else if opcode == Opcodes::ERROR as u16 {
+            return Err(self.parse_error(&buf[4 ..]));
         }
 
         Ok(false)
@@ -312,6 +351,7 @@ impl Tftp {
 
             let _opcode = match u16::from_be_bytes([buf[0], buf[1]]) {
                 opc if opc == Opcodes::DATA as u16 => (),
+                opc if opc == Opcodes::ERROR as u16 => return Err(self.parse_error(&buf[.. len])),
                 _ => return Err(io::Error::new(io::ErrorKind::Other, "unexpected opcode")),
             };
             if u16::from_be_bytes([buf[2], buf[3]]) != block_nr {
diff --git a/src/tftpc.rs b/src/tftpc.rs
index a006962..6aa61c9 100644
--- a/src/tftpc.rs
+++ b/src/tftpc.rs
@@ -65,30 +65,36 @@ impl Tftpc {
         Some(remote)
     }
 
-    fn wait_for_response(&self, sock: &UdpSocket, expected_opcode: rtftp::Opcodes, expected_block: u16, expected_remote: Option<SocketAddr>) -> Option<SocketAddr> {
+    fn wait_for_response(&self, sock: &UdpSocket, expected_opcode: rtftp::Opcodes, expected_block: u16, expected_remote: Option<SocketAddr>) -> Result<Option<SocketAddr>, std::io::Error> {
         let mut buf = [0; 4];
         let (len, remote) = match sock.peek_from(&mut buf) {
             Ok(args) => args,
-            Err(_) => return None,
+            Err(err) => return Err(err),
         };
 
         if let Some(rem) = expected_remote {
             /* verify we got a response from the same client that sent
                an optional previous option ack */
             if rem != remote {
-                return None;
+                return Ok(None);
             }
         }
 
         let opcode = u16::from_be_bytes([buf[0], buf[1]]);
         let block_nr = u16::from_be_bytes([buf[2], buf[3]]);
 
+        if opcode == rtftp::Opcodes::ERROR as u16 {
+            let mut buf = [0; 512];
+            let len = sock.recv(&mut buf)?;
+            return Err(self.tftp.parse_error(&buf[ ..len]));
+        }
+
         /* first data packet is expected to be block 1 */
         if len != 4 || opcode != expected_opcode as u16 || block_nr != expected_block {
-            return None;
+            return Ok(None);
         }
 
-        Some(remote)
+        Ok(Some(remote))
     }
 
     fn append_option_req(&self, buf: &mut Vec<u8>, fsize: u64) {
@@ -97,24 +103,25 @@ impl Tftpc {
         self.tftp.append_option(buf, "tsize", &format!("{}", fsize));
     }
 
-    fn handle_wrq(&mut self, sock: &UdpSocket) -> Result<(), io::Error> {
+    fn handle_wrq(&mut self, sock: &UdpSocket) -> Result<String, io::Error> {
         let mut file = match File::open(self.conf.filename.as_path()) {
             Ok(f) => f,
             Err(err) => return Err(err),
         };
+        let err_invalidpath = io::Error::new(io::ErrorKind::InvalidInput, "Invalid path/filename");
         let filename = match self.conf.filename.file_name() {
             Some(f) => match f.to_str() {
                 Some(s) => s,
-                None => return Err(io::Error::new(io::ErrorKind::InvalidInput, "Invalid path/filename")),
+                None => return Err(err_invalidpath),
             }
-            None => return Err(io::Error::new(io::ErrorKind::InvalidInput, "Invalid path/filename")),
+            None => return Err(err_invalidpath),
         };
         let metadata = match file.metadata() {
             Ok(m) => m,
-            Err(_) => return Err(io::Error::new(io::ErrorKind::InvalidInput, "Invalid path/filename")),
+            Err(_) => return Err(err_invalidpath),
         };
         if !metadata.is_file() {
-            return Err(io::Error::new(io::ErrorKind::InvalidInput, "Invalid path/filename"));
+            return Err(err_invalidpath);
         }
 
         let mut buf = Vec::with_capacity(512);
@@ -128,7 +135,7 @@ impl Tftpc {
             remote = self.wait_for_option_ack(&sock);
             if remote.is_none() {
                 /* for WRQ either OACK or ACK is replied */
-                remote = self.wait_for_response(&sock, rtftp::Opcodes::ACK, 0, None);
+                remote = self.wait_for_response(&sock, rtftp::Opcodes::ACK, 0, None)?;
             }
             if remote.is_some() {
                 break;
@@ -141,17 +148,16 @@ impl Tftpc {
         }
 
         match self.tftp.send_file(&sock, &mut file) {
-            Ok(_) => println!("Sent {} to {}.", self.conf.filename.display(), self.conf.remote),
+            Ok(_) => return Ok(format!("Sent {} to {}.", self.conf.filename.display(), self.conf.remote)),
             Err(err) => {
+                let error = format!("Sending {} to {} failed ({}).", self.conf.filename.display(), self.conf.remote, err);
                 self.tftp.send_error(&sock, 0, "Sending error")?;
-                println!("Sending {} to {} failed ({}).", self.conf.filename.display(), self.conf.remote, err);
+                return Err(io::Error::new(err.kind(), error));
             },
         }
-
-        Ok(())
     }
 
-    fn handle_rrq(&mut self, sock: &UdpSocket) -> Result<(), io::Error> {
+    fn handle_rrq(&mut self, sock: &UdpSocket) -> Result<String, io::Error> {
         let filename = match self.conf.filename.file_name() {
             Some(f) => f,
             None => return Err(io::Error::new(io::ErrorKind::InvalidInput, "Invalid path/filename")),
@@ -179,7 +185,7 @@ impl Tftpc {
                 /* for RRQ the received OACKs need to be acked */
                 self.tftp.send_ack_to(&sock, r, 0)?;
             }
-            remote = self.wait_for_response(&sock, rtftp::Opcodes::DATA, 1, oack_remote);
+            remote = self.wait_for_response(&sock, rtftp::Opcodes::DATA, 1, oack_remote)?;
             if remote.is_some() {
                 break;
             }
@@ -191,14 +197,13 @@ impl Tftpc {
         }
 
         match self.tftp.recv_file(&sock, &mut file) {
-            Ok(_) => println!("Received {} from {}.", self.conf.filename.display(), self.conf.remote),
+            Ok(_) => return Ok(format!("Received {} from {}.", self.conf.filename.display(), self.conf.remote)),
             Err(err) => {
+                let error = format!("Receiving {} from {} failed ({}).", self.conf.filename.display(), self.conf.remote, err);
                 self.tftp.send_error(&sock, 0, "Receiving error")?;
-                println!("Receiving {} from {} failed ({}).", self.conf.filename.display(), self.conf.remote, err);
+                return Err(std::io::Error::new(err.kind(), error));
             },
         }
-
-        Ok(())
     }
 
     pub fn start(&mut self) {
@@ -210,7 +215,7 @@ impl Tftpc {
             Mode::WRQ => self.handle_wrq(&socket),
         };
         match err {
-            Ok(_) => {},
+            Ok(msg) => println!("{}", msg),
             Err(err) => {
                 println!("Error: {}", err);
                 return;
diff --git a/src/tftpd.rs b/src/tftpd.rs
index c85603d..86d190a 100644
--- a/src/tftpd.rs
+++ b/src/tftpd.rs
@@ -73,7 +73,7 @@ impl Tftpd {
     }
 
 
-    fn handle_wrq(&mut self, socket: &UdpSocket, cl: &SocketAddr, buf: &[u8]) -> Result<(), io::Error> {
+    fn handle_wrq(&mut self, socket: &UdpSocket, cl: &SocketAddr, buf: &[u8]) -> Result<(String), io::Error> {
         let (filename, mode, mut options) = self.tftp.parse_file_mode_options(buf)?;
         self.tftp.init_tftp_options(&socket, &mut options)?;
 
@@ -88,39 +88,38 @@ impl Tftpd {
         let path = match self.file_allowed(&filename) {
             Some(p) => p,
             None => {
-                println!("Sending {} to {} failed (permission check failed).", filename.display(), cl);
+                let err = format!("Sending {} to {} failed (permission check failed).", filename.display(), cl);
                 self.tftp.send_error(&socket, 2, "Permission denied")?;
-                return Err(io::Error::new(io::ErrorKind::PermissionDenied, "permission denied"));
+                return Err(io::Error::new(io::ErrorKind::PermissionDenied, err));
             }
         };
 
         let mut file = match OpenOptions::new().write(true).create_new(true).open(&path) {
             Ok(f) => f,
             Err(ref err) if err.kind() == io::ErrorKind::AlreadyExists => {
-                println!("Receiving {} from {} failed ({}).", path.display(), cl, err);
+                let error = format!("Receiving {} from {} failed ({}).", path.display(), cl, err);
                 self.tftp.send_error(&socket, 6, "File already exists")?;
-                return Err(io::Error::new(err.kind(), "already exists"));
+                return Err(io::Error::new(err.kind(), error));
             },
             Err(err) => {
-                println!("Receiving {} from {} failed ({}).", path.display(), cl, err);
+                let error = format!("Receiving {} from {} failed ({}).", path.display(), cl, err);
                 self.tftp.send_error(&socket, 6, "Permission denied")?;
-                return Err(io::Error::new(io::ErrorKind::PermissionDenied, "permission denied"));
+                return Err(io::Error::new(io::ErrorKind::PermissionDenied, error));
             }
         };
 
         self.tftp.ack_options(&socket, &options, false)?;
         match self.tftp.recv_file(&socket, &mut file) {
-            Ok(_) => println!("Received {} from {}.", path.display(), cl),
+            Ok(_) => return Ok(format!("Received {} from {}.", path.display(), cl)),
             Err(ref err) => {
-                println!("Receiving {} from {} failed ({}).", path.display(), cl, err);
+                let error = format!("Receiving {} from {} failed ({}).", path.display(), cl, err);
                 self.tftp.send_error(&socket, 0, "Receiving error")?;
-                return Err(io::Error::new(err.kind(), err.to_string()));
+                return Err(io::Error::new(err.kind(), error));
             }
         }
-        Ok(())
     }
 
-    fn handle_rrq(&mut self, socket: &UdpSocket, cl: &SocketAddr, buf: &[u8]) -> Result<(), io::Error> {
+    fn handle_rrq(&mut self, socket: &UdpSocket, cl: &SocketAddr, buf: &[u8]) -> Result<(String), io::Error> {
         let (filename, mode, mut options) = self.tftp.parse_file_mode_options(buf)?;
         self.tftp.init_tftp_options(&socket, &mut options)?;
 
@@ -135,21 +134,23 @@ impl Tftpd {
         let path = match self.file_allowed(&filename) {
             Some(p) => p,
             None => {
-                println!("Sending {} to {} failed (permission check failed).", filename.display(), cl);
+                let err = format!("Sending {} to {} failed (permission check failed).", filename.display(), cl);
                 self.tftp.send_error(&socket, 2, "Permission denied")?;
-                return Err(io::Error::new(io::ErrorKind::PermissionDenied, "permission denied"));
+                return Err(io::Error::new(io::ErrorKind::PermissionDenied, err));
             }
         };
 
         let mut file = match File::open(&path) {
             Ok(f) => f,
             Err(ref error) if error.kind() == io::ErrorKind::NotFound => {
+                let err = format!("Sending {} to {} failed ({}).", path.display(), cl, error.to_string());
                 self.tftp.send_error(&socket, 1, "File not found")?;
-                return Err(io::Error::new(io::ErrorKind::NotFound, "file not found"));
+                return Err(io::Error::new(io::ErrorKind::NotFound, err));
             },
-            Err(_) => {
+            Err(error) => {
+                let err = format!("Sending {} to {} failed ({}).", path.display(), cl, error.to_string());
                 self.tftp.send_error(&socket, 2, "Permission denied")?;
-                return Err(io::Error::new(io::ErrorKind::PermissionDenied, "permission denied"));
+                return Err(io::Error::new(io::ErrorKind::PermissionDenied, err));
             }
         };
         if !file.metadata()?.is_file() {
@@ -162,24 +163,26 @@ impl Tftpd {
         }
         self.tftp.ack_options(&socket, &options, true)?;
         match self.tftp.send_file(&socket, &mut file) {
-            Ok(_) => println!("Sent {} to {}.", path.display(), cl),
-            Err(err) => println!("Sending {} to {} failed ({}).", path.display(), cl, err.to_string()),
+            Ok(_) => return Ok(format!("Sent {} to {}.", path.display(), cl)),
+            Err(err) => {
+                let error = format!("Sending {} to {} failed ({}).", path.display(), cl, err.to_string());
+                return Err(std::io::Error::new(err.kind(), error));
+            }
         }
-        Ok(())
     }
 
-    pub fn handle_client(&mut self, cl: &SocketAddr, buf: &[u8]) -> Result<(), io::Error> {
+    pub fn handle_client(&mut self, cl: &SocketAddr, buf: &[u8]) -> Result<String, io::Error> {
         let socket = UdpSocket::bind("[::]:0")?;
         socket.set_read_timeout(Some(Duration::from_secs(5)))?;
         socket.connect(cl)?;
 
         let _opcode = match u16::from_be_bytes([buf[0], buf[1]]) {
-            o if o == rtftp::Opcodes::RRQ  as u16 => {
+            o if o == rtftp::Opcodes::RRQ as u16 => {
                 if self.conf.wo {
                     self.tftp.send_error(&socket, 4, "reading not allowed")?;
                     return Err(io::Error::new(io::ErrorKind::Other, "unallowed mode"));
                 } else {
-                    self.handle_rrq(&socket, &cl, &buf[2..])?;
+                    return self.handle_rrq(&socket, &cl, &buf[2..]);
                 }
             },
             o if o == rtftp::Opcodes::WRQ as u16 => {
@@ -187,16 +190,15 @@ impl Tftpd {
                     self.tftp.send_error(&socket, 4, "writing not allowed")?;
                     return Err(io::Error::new(io::ErrorKind::Other, "unallowed mode"));
                 } else {
-                    self.handle_wrq(&socket, &cl, &buf[2..])?;
+                   return  self.handle_wrq(&socket, &cl, &buf[2..]);
                 }
             },
-            o if o == rtftp::Opcodes::ERROR as u16 => println!("Received ERROR from {}", cl),
+            o if o == rtftp::Opcodes::ERROR as u16 => return Ok(format!("Received ERROR from {}", cl)),
             _ => {
                 self.tftp.send_error(&socket, 4, "Unexpected opcode")?;
                 return Err(io::Error::new(io::ErrorKind::Other, "unexpected opcode"));
             }
         };
-        Ok(())
     }
 
     fn drop_privs(&self, uid: u32, gid: u32) -> Result<(), Box<Error>> {
@@ -226,14 +228,14 @@ impl Tftpd {
         let socket = match UdpSocket::bind(format!("[::]:{}", self.conf.port)) {
             Ok(s) => s,
             Err(err) => {
-                println!("Binding a socket failed: {}", err);
+                eprintln!("Binding a socket failed: {}", err);
                 return;
             }
         };
         match self.drop_privs(self.conf.uid, self.conf.gid) {
             Ok(_) => (),
             Err(err) => {
-                println!("Dropping privileges failed: {}", err);
+                eprintln!("Dropping privileges failed: {}", err);
                 return;
             }
         };
@@ -241,7 +243,7 @@ impl Tftpd {
         match env::set_current_dir(&self.conf.dir) {
             Ok(_) => (),
             Err(err) => {
-                println!("Changing directory to {} failed ({}).", &self.conf.dir.display(), err);
+                eprintln!("Changing directory to {} failed ({}).", &self.conf.dir.display(), err);
                 return;
             }
         }
@@ -251,14 +253,14 @@ impl Tftpd {
             let (n, src) = match socket.recv_from(&mut buf) {
                 Ok(args) => args,
                 Err(err) => {
-                    println!("Receiving data from socket failed: {}", err);
+                    eprintln!("Receiving data from socket failed: {}", err);
                     break;
                 }
             };
 
             match self.handle_client(&src, &buf[0..n]) {
-                /* errors intentionally ignored */
-                _ => (),
+                Ok(msg) => println!("{}", msg),
+                Err(err) => println!("{}", err),
             }
         }