From c9ffdc53922061c86e34099f994407498f235644 Mon Sep 17 00:00:00 2001 From: Titouan Rigoudy Date: Sun, 14 Jun 2020 17:35:07 +0000 Subject: [PATCH] Introduce ProtoDecodeError and use it in ProtoDecoder. --- Cargo.lock | 56 ++++++++ Cargo.toml | 1 + src/proto/base_codec.rs | 261 +++++++++++++++++++---------------- src/proto/mod.rs | 4 +- src/proto/peer/message.rs | 31 +++-- src/proto/server/request.rs | 47 ++++--- src/proto/server/response.rs | 69 ++++----- src/proto/user.rs | 16 ++- 8 files changed, 293 insertions(+), 192 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6d64bfb..411dd72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -482,6 +482,22 @@ name = "percent-encoding" version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "proc-macro2" +version = "1.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "unicode-xid 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "quote" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "proc-macro2 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "rand" version = "0.3.23" @@ -729,6 +745,7 @@ dependencies = [ "rust-crypto 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)", "slab 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "thiserror 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)", "threadpool 1.8.1 (registry+https://github.com/rust-lang/crates.io-index)", "tokio-codec 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "tokio-core 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)", @@ -736,6 +753,34 @@ dependencies = [ "ws 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "syn" +version = "1.0.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "proc-macro2 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)", + "unicode-xid 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "thiserror" +version = "1.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "thiserror-impl 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "proc-macro2 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 1.0.31 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "thread-id" version = "2.0.0" @@ -973,6 +1018,11 @@ dependencies = [ "smallvec 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "unicode-xid" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "url" version = "1.7.2" @@ -1099,6 +1149,8 @@ dependencies = [ "checksum parking_lot_core 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "cb88cb1cb3790baa6776844f968fea3be44956cf184fa1be5a03341f5491278c" "checksum parking_lot_core 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "b876b1b9e7ac6e1a74a6da34d25c42e17e8862aa409cbbbdcfc8d86c6f3bc62b" "checksum percent-encoding 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "31010dd2e1ac33d5b46a5b413495239882813e0369f8ed8a5e266f173602f831" +"checksum proc-macro2 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)" = "beae6331a816b1f65d04c45b078fd8e6c93e8071771f41b8163255bbd8d7c8fa" +"checksum quote 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)" = "aa563d17ecb180e500da1cfd2b028310ac758de548efdd203e18f283af693f37" "checksum rand 0.3.23 (registry+https://github.com/rust-lang/crates.io-index)" = "64ac302d8f83c0c1974bf758f6b041c6c8ada916fbb44a609158ca8b064cc76c" "checksum rand 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)" = "552840b97013b1a26992c11eac34bdd778e464601a4c2054b5f0bff7c6761293" "checksum rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)" = "6d71dacdc3c88c1fde3885a3be3fbab9f35724e6ce99467f7d9c5026132184ca" @@ -1128,6 +1180,9 @@ dependencies = [ "checksum slab 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" "checksum smallvec 0.6.13 (registry+https://github.com/rust-lang/crates.io-index)" = "f7b0758c52e15a8b5e3691eae6cc559f08eee9406e548a4477ba4e67770a82b6" "checksum smallvec 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c7cb5678e1615754284ec264d9bb5b4c27d2018577fd90ac0ceb578591ed5ee4" +"checksum syn 1.0.31 (registry+https://github.com/rust-lang/crates.io-index)" = "b5304cfdf27365b7585c25d4af91b35016ed21ef88f17ced89c7093b43dba8b6" +"checksum thiserror 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)" = "b13f926965ad00595dd129fa12823b04bbf866e9085ab0a5f2b05b850fbfc344" +"checksum thiserror-impl 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)" = "893582086c2f98cde18f906265a65b5030a074b1046c674ae898be6519a7f479" "checksum thread-id 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "a9539db560102d1cef46b8b78ce737ff0bb64e7e18d35b2a5688f7d097d0ff03" "checksum thread_local 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "8576dbbfcaef9641452d5cf0df9b0e7eeab7694956dd33bb61515fb8f18cfdd5" "checksum threadpool 1.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "d050e60b33d41c19108b32cea32164033a9013fe3b46cbd4457559bfbf77afaa" @@ -1148,6 +1203,7 @@ dependencies = [ "checksum tokio-uds 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "5076db410d6fdc6523df7595447629099a1fdc47b3d9f896220780fa48faf798" "checksum unicode-bidi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "49f2bd0c6468a8230e1db229cff8029217cf623c767ea5d60bfbd42729ea54d5" "checksum unicode-normalization 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)" = "5479532badd04e128284890390c1e876ef7a993d0570b3597ae43dfa1d59afa4" +"checksum unicode-xid 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "826e7639553986605ec5979c7dd957c7895e93eabed50ab2ffa7f6128a75097c" "checksum url 1.7.2 (registry+https://github.com/rust-lang/crates.io-index)" = "dd4e7c0d531266369519a4aa4f399d748bd37043b00bde1e4ff1f60a120b355a" "checksum utf8-ranges 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "a1ca13c08c41c9c3e04224ed9ff80461d97e121589ff27c753a16cb10830ae0f" "checksum winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a" diff --git a/Cargo.toml b/Cargo.toml index 0d02ba1..b4c1822 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ parking_lot = "^0.8" rust-crypto = "^0.2.34" rustc-serialize = "^0.3.17" slab = "^0.2" +thiserror = "^1.0" threadpool = "^1.0" tokio-core = "^0.1" tokio-io = "^0.1" diff --git a/src/proto/base_codec.rs b/src/proto/base_codec.rs index d150d15..dc5994a 100644 --- a/src/proto/base_codec.rs +++ b/src/proto/base_codec.rs @@ -13,7 +13,6 @@ //! * Pairs are serialized as two consecutive values. //! * Vectors are serialized as length-prefixed arrays of serialized values. -use std::fmt; use std::io; use std::net; @@ -21,6 +20,7 @@ use bytes::{BufMut, BytesMut}; use encoding::all::WINDOWS_1252; use encoding::{DecoderTrap, EncoderTrap, Encoding}; use std::convert::{TryFrom, TryInto}; +use thiserror::Error; // Constants // --------- @@ -38,8 +38,70 @@ pub trait Encode { fn encode(&mut self, value: T) -> io::Result<()>; } -// TODO: Define a real DecodeError type using the thiserror crate for better -// error messages without all this io::Error hackery. +// TODO: Add backtrace fields to each enum variant once std::backtrace is +// stabilized. +#[derive(PartialEq, Error, Debug)] +pub enum ProtoDecodeError { + #[error("at position {position}: not enough bytes to decode: expected {expected}, found {remaining}")] + NotEnoughData { + /// The number of bytes the decoder expected to read. + /// + /// Invariant: `remaining < expected`. + expected: usize, + + /// The number of bytes remaining in the input buffer. + /// + /// Invariant: `remaining < expected`. + remaining: usize, + + /// The decoder's position in the input buffer. + position: usize, + }, + #[error("at position {position}: invalid boolean value: {value}")] + InvalidBool { + /// The invalid value. Never equal to 0 nor 1. + value: u8, + + /// The decoder's position in the input buffer. + position: usize, + }, + #[error("at position {position}: invalid u16 value: {value}")] + InvalidU16 { + /// The invalid value. Always greater than u16::max_value(). + value: u32, + + /// The decoder's position in the input buffer. + position: usize, + }, + #[error("at position {position}: failed to decode string: {cause}")] + InvalidString { + /// The cause of the error, as reported by the encoding library. + cause: String, + + /// The decoder's position in the input buffer. + position: usize, + }, + #[error("at position {position}: invalid {value_name}: {cause}")] + InvalidData { + /// The name of the value the decoder failed to decode. + value_name: String, + + /// The cause of the error. + cause: String, + + /// The decoder's position in the input buffer. + position: usize, + }, +} + +impl From for io::Error { + fn from(error: ProtoDecodeError) -> Self { + match &error { + &ProtoDecodeError::NotEnoughData { .. } => unexpected_eof_error(format!("{}", error)), + _ => invalid_data_error(format!("{}", error)), + } + } +} /// Builds an UnexpectedEof error with the given message. fn unexpected_eof_error(message: String) -> io::Error { @@ -78,7 +140,7 @@ pub struct ProtoDecoder<'a> { /// a `ProtoDecoder`. pub trait ProtoDecode: Sized { /// Attempts to decode a value of this type with the given decoder. - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result; + fn decode_from(decoder: &mut ProtoDecoder) -> Result; } impl<'a> ProtoDecoder<'a> { @@ -90,6 +152,11 @@ impl<'a> ProtoDecoder<'a> { } } + /// The current position of this decoder in the input buffer. + pub fn position(&self) -> usize { + self.position + } + /// Returns the number of bytes remaining to decode. pub fn remaining(&self) -> usize { self.buffer.len() - self.position @@ -113,13 +180,13 @@ impl<'a> ProtoDecoder<'a> { /// /// Returns a slice of size `n` if successful, in which case this decoder /// advances its internal position by `n`. - fn consume(&mut self, n: usize) -> io::Result<&[u8]> { + fn consume(&mut self, n: usize) -> Result<&[u8], ProtoDecodeError> { if self.remaining() < n { - return Err(unexpected_eof_error(format!( - "expected {} bytes remaining, found {}", - n, - self.remaining() - ))); + return Err(ProtoDecodeError::NotEnoughData { + expected: n, + remaining: self.remaining(), + position: self.position, + }); } // Cannot use bytes() here as it borrows self immutably, which @@ -131,11 +198,7 @@ impl<'a> ProtoDecoder<'a> { } /// Attempts to decode a u32 value. - /// - /// Note that this method returns a less descriptive error than - /// `self.decode::()`. It is intended to be a low-level building block - /// for decoding other types. - fn decode_u32(&mut self) -> io::Result { + fn decode_u32(&mut self) -> Result { let bytes = self.consume(U32_BYTE_LEN)?; // The conversion from slice to fixed-size array cannot fail, because // consume() guarantees that its return value is of size n. @@ -143,33 +206,46 @@ impl<'a> ProtoDecoder<'a> { Ok(u32::from_le_bytes(array)) } + fn decode_u16(&mut self) -> Result { + let position = self.position; + let n = self.decode_u32()?; + match u16::try_from(n) { + Ok(value) => Ok(value), + Err(_) => Err(ProtoDecodeError::InvalidU16 { + value: n, + position: position, + }), + } + } + /// Attempts to decode a boolean value. - /// - /// Note that this method returns a less descriptive error than - /// `self.decode::()`. It is intended to be a low-level building block - /// for decoding other types. - fn decode_bool(&mut self) -> io::Result { + fn decode_bool(&mut self) -> Result { + let position = self.position; let bytes = self.consume(1)?; match bytes[0] { 0 => Ok(false), 1 => Ok(true), - n => Err(invalid_data_error(format!("invalid bool value {}", n))), + n => Err(ProtoDecodeError::InvalidBool { + value: n, + position: position, + }), } } /// Attempts to decode a string value. - /// - /// Note that this method returns a less descriptive error than - /// `self.decode::()`. It is intended to be a low-level building - /// block for decoding other types. - fn decode_string(&mut self) -> io::Result { + fn decode_string(&mut self) -> Result { let length = self.decode_u32()? as usize; + + let position = self.position; let bytes = self.consume(length)?; let result = WINDOWS_1252.decode(bytes, DecoderTrap::Strict); match result { Ok(string) => Ok(string), - Err(error) => Err(invalid_data_error(format!("invalid string: {:?}", error))), + Err(error) => Err(ProtoDecodeError::InvalidString { + cause: error.to_string(), + position: position, + }), } } @@ -180,55 +256,44 @@ impl<'a> ProtoDecoder<'a> { /// ``` /// let val: Foo = decoder.decode()?; /// ``` - pub fn decode(&mut self) -> io::Result { - let position = self.position; - match T::decode_from(self) { - Ok(value) => Ok(value), - Err(ref error) => Err(annotate_error( - error, - &format!("decoding value at position {}", position), - )), - } + pub fn decode(&mut self) -> Result { + T::decode_from(self) } } impl ProtoDecode for u32 { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { decoder.decode_u32() } } impl ProtoDecode for u16 { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { - let n = decoder.decode_u32()?; - match u16::try_from(n) { - Ok(value) => Ok(value), - Err(_) => Err(invalid_data_error(format!("invalid u16 value {}", n))), - } + fn decode_from(decoder: &mut ProtoDecoder) -> Result { + decoder.decode_u16() } } impl ProtoDecode for bool { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { decoder.decode_bool() } } impl ProtoDecode for net::Ipv4Addr { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let ip = decoder.decode_u32()?; Ok(net::Ipv4Addr::from(ip)) } } impl ProtoDecode for String { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { decoder.decode_string() } } impl ProtoDecode for (T, U) { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let first = decoder.decode()?; let second = decoder.decode()?; Ok((first, second)) @@ -236,7 +301,7 @@ impl ProtoDecode for (T, U) { } impl ProtoDecode for Vec { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let len = decoder.decode_u32()? as usize; let mut vec = Vec::with_capacity(len); for _ in 0..len { @@ -404,7 +469,7 @@ pub mod tests { use bytes::BytesMut; - use super::{ProtoDecode, ProtoDecoder, ProtoEncode, ProtoEncoder}; + use super::{ProtoDecode, ProtoDecodeError, ProtoDecoder, ProtoEncode, ProtoEncoder}; // Declared here because assert_eq!(bytes, &[]) fails to infer types. const EMPTY_BYTES: &'static [u8] = &[]; @@ -421,28 +486,6 @@ pub mod tests { assert_eq!(output, input); } - pub fn expect_io_error(result: io::Result, kind: io::ErrorKind, message: &str) - where - T: fmt::Debug + Send + 'static, - { - match result { - Err(e) => { - assert_eq!(e.kind(), kind); - let ok = match e.get_ref() { - Some(e) => { - assert_eq!(e.description(), message); - true - } - None => false, - }; - if !ok { - panic!(e) - } - } - Ok(message) => panic!(message), - } - } - // A few integers and their corresponding byte encodings. const U32_ENCODINGS: [(u32, [u8; 4]); 8] = [ (0, [0, 0, 0, 0]), @@ -455,36 +498,6 @@ pub mod tests { (u32::MAX, [255, 255, 255, 255]), ]; - #[test] - fn expect_io_error_success() { - let kind = io::ErrorKind::InvalidInput; - let message = "some message"; - let result: io::Result<()> = Err(io::Error::new(kind, message)); - expect_io_error(result, kind, message); - } - - #[test] - #[should_panic] - fn expect_io_error_not_err() { - expect_io_error(Ok(()), io::ErrorKind::InvalidInput, "some message"); - } - - #[test] - #[should_panic] - fn expect_io_error_wrong_kind() { - let result: io::Result<()> = - Err(io::Error::new(io::ErrorKind::UnexpectedEof, "some message")); - expect_io_error(result, io::ErrorKind::InvalidInput, "some message"); - } - - #[test] - #[should_panic] - fn expect_io_error_wrong_message() { - let result: io::Result<()> = - Err(io::Error::new(io::ErrorKind::InvalidInput, "some message")); - expect_io_error(result, io::ErrorKind::InvalidInput, "other message"); - } - #[test] fn encode_u32() { for &(val, ref encoded_bytes) in &U32_ENCODINGS { @@ -524,10 +537,13 @@ pub mod tests { let result = decoder.decode::(); - expect_io_error( + assert_eq!( result, - io::ErrorKind::UnexpectedEof, - "decoding value at position 0: expected 4 bytes remaining, found 1", + Err(ProtoDecodeError::NotEnoughData { + expected: 4, + remaining: 1, + position: 0, + }) ); assert_eq!(decoder.bytes(), &[13]); } @@ -571,10 +587,12 @@ pub mod tests { let result = ProtoDecoder::new(&buffer).decode::(); - expect_io_error( + assert_eq!( result, - io::ErrorKind::InvalidData, - "decoding value at position 0: invalid bool value 42", + Err(ProtoDecodeError::InvalidBool { + value: 42, + position: 0, + }) ); } @@ -584,10 +602,13 @@ pub mod tests { let result = ProtoDecoder::new(&buffer).decode::(); - expect_io_error( + assert_eq!( result, - io::ErrorKind::UnexpectedEof, - "decoding value at position 0: expected 1 bytes remaining, found 0", + Err(ProtoDecodeError::NotEnoughData { + expected: 1, + remaining: 0, + position: 0, + }) ); } @@ -623,13 +644,12 @@ pub mod tests { assert_eq!(val, expected_val as u16); assert_eq!(decoder.bytes(), EMPTY_BYTES); } else { - expect_io_error( + assert_eq!( decoder.decode::(), - io::ErrorKind::InvalidData, - &format!( - "decoding value at position 0: invalid u16 value {}", - expected_val - ), + Err(ProtoDecodeError::InvalidU16 { + value: expected_val, + position: 0, + }) ); } } @@ -642,10 +662,13 @@ pub mod tests { let result = decoder.decode::(); - expect_io_error( + assert_eq!( result, - io::ErrorKind::UnexpectedEof, - "decoding value at position 0: expected 4 bytes remaining, found 0", + Err(ProtoDecodeError::NotEnoughData { + expected: 4, + remaining: 0, + position: 0, + }) ); } diff --git a/src/proto/mod.rs b/src/proto/mod.rs index 23340b8..4a28178 100644 --- a/src/proto/mod.rs +++ b/src/proto/mod.rs @@ -8,7 +8,9 @@ pub mod server; mod stream; mod user; -pub use self::base_codec::{Decode, ProtoDecode, ProtoDecoder, ProtoEncode, ProtoEncoder}; +pub use self::base_codec::{ + Decode, ProtoDecode, ProtoDecodeError, ProtoDecoder, ProtoEncode, ProtoEncoder, +}; pub use self::codec::*; pub use self::handler::*; pub use self::packet::*; diff --git a/src/proto/peer/message.rs b/src/proto/peer/message.rs index 7d595b2..3ed5e35 100644 --- a/src/proto/peer/message.rs +++ b/src/proto/peer/message.rs @@ -2,8 +2,8 @@ use std::io; use crate::proto::peer::constants::*; use crate::proto::{ - MutPacket, Packet, PacketReadError, ProtoDecode, ProtoDecoder, ProtoEncode, ProtoEncoder, - ReadFromPacket, WriteToPacket, + MutPacket, Packet, PacketReadError, ProtoDecode, ProtoDecodeError, ProtoDecoder, ProtoEncode, + ProtoEncoder, ReadFromPacket, WriteToPacket, }; /*=========* @@ -42,7 +42,8 @@ impl ReadFromPacket for Message { } impl ProtoDecode for Message { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { + let position = decoder.position(); let code: u32 = decoder.decode()?; let message = match code { CODE_PIERCE_FIREWALL => { @@ -54,10 +55,11 @@ impl ProtoDecode for Message { Message::PeerInit(peer_init) } _ => { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - format!("unknown peer message code: {}", code), - )); + return Err(ProtoDecodeError::InvalidData { + value_name: "peer message code".to_string(), + cause: format!("unknown value {}", code), + position: position, + }) } }; Ok(message) @@ -139,7 +141,7 @@ impl ProtoEncode for PeerInit { } impl ProtoDecode for PeerInit { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let user_name = decoder.decode()?; let connection_type = decoder.decode()?; let token = decoder.decode()?; @@ -157,8 +159,8 @@ mod tests { use bytes::BytesMut; - use crate::proto::base_codec::tests::{expect_io_error, roundtrip}; - use crate::proto::ProtoDecoder; + use crate::proto::base_codec::tests::roundtrip; + use crate::proto::{ProtoDecodeError, ProtoDecoder}; use super::*; @@ -168,10 +170,13 @@ mod tests { let result = ProtoDecoder::new(&bytes).decode::(); - expect_io_error( + assert_eq!( result, - io::ErrorKind::InvalidData, - "decoding value at position 0: unknown peer message code: 1337", + Err(ProtoDecodeError::InvalidData { + value_name: "peer message code".to_string(), + cause: "unknown value 1337".to_string(), + position: 0, + }) ); } diff --git a/src/proto/server/request.rs b/src/proto/server/request.rs index 5545966..ea58e5a 100644 --- a/src/proto/server/request.rs +++ b/src/proto/server/request.rs @@ -5,7 +5,7 @@ use crypto::md5::Md5; use crate::proto::packet::{MutPacket, WriteToPacket}; use crate::proto::server::constants::*; -use crate::proto::{ProtoDecode, ProtoDecoder, ProtoEncode, ProtoEncoder}; +use crate::proto::{ProtoDecode, ProtoDecodeError, ProtoDecoder, ProtoEncode, ProtoEncoder}; /* ------- * * Helpers * @@ -149,7 +149,8 @@ impl ProtoEncode for ServerRequest { } impl ProtoDecode for ServerRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { + let position = decoder.position(); let code: u32 = decoder.decode()?; let request = match code { CODE_CANNOT_CONNECT => { @@ -194,10 +195,11 @@ impl ProtoDecode for ServerRequest { ServerRequest::UserStatusRequest(request) } _ => { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - format!("unknown server request code: {}", code), - )); + return Err(ProtoDecodeError::InvalidData { + value_name: "server request code".to_string(), + cause: format!("unknown value {}", code), + position: position, + }) } }; Ok(request) @@ -230,7 +232,7 @@ impl ProtoEncode for CannotConnectRequest { } impl ProtoDecode for CannotConnectRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let token = decoder.decode()?; let user_name = decoder.decode()?; Ok(CannotConnectRequest { token, user_name }) @@ -266,7 +268,7 @@ impl ProtoEncode for ConnectToPeerRequest { } impl ProtoDecode for ConnectToPeerRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let token = decoder.decode()?; let user_name = decoder.decode()?; let connection_type = decoder.decode()?; @@ -304,7 +306,7 @@ impl ProtoEncode for FileSearchRequest { } impl ProtoDecode for FileSearchRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let ticket = decoder.decode()?; let query = decoder.decode()?; Ok(FileSearchRequest { ticket, query }) @@ -376,7 +378,7 @@ impl ProtoEncode for LoginRequest { } impl ProtoDecode for LoginRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let username = decoder.decode()?; let password = decoder.decode()?; let major = decoder.decode()?; @@ -415,7 +417,7 @@ impl ProtoEncode for PeerAddressRequest { } impl ProtoDecode for PeerAddressRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let username = decoder.decode()?; Ok(PeerAddressRequest { username: username }) } @@ -444,7 +446,7 @@ impl ProtoEncode for RoomJoinRequest { } impl ProtoDecode for RoomJoinRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; Ok(RoomJoinRequest { room_name: room_name, @@ -475,7 +477,7 @@ impl ProtoEncode for RoomLeaveRequest { } impl ProtoDecode for RoomLeaveRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; Ok(RoomLeaveRequest { room_name: room_name, @@ -509,7 +511,7 @@ impl ProtoEncode for RoomMessageRequest { } impl ProtoDecode for RoomMessageRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; let message = decoder.decode()?; Ok(RoomMessageRequest { room_name, message }) @@ -539,7 +541,7 @@ impl ProtoEncode for SetListenPortRequest { } impl ProtoDecode for SetListenPortRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let port = decoder.decode()?; Ok(SetListenPortRequest { port: port }) } @@ -568,7 +570,7 @@ impl ProtoEncode for UserStatusRequest { } impl ProtoDecode for UserStatusRequest { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let user_name = decoder.decode()?; Ok(UserStatusRequest { user_name: user_name, @@ -586,8 +588,8 @@ mod tests { use bytes::BytesMut; - use crate::proto::base_codec::tests::{expect_io_error, roundtrip}; - use crate::proto::ProtoDecoder; + use crate::proto::base_codec::tests::roundtrip; + use crate::proto::{ProtoDecodeError, ProtoDecoder}; use super::*; @@ -597,10 +599,13 @@ mod tests { let result = ProtoDecoder::new(&bytes).decode::(); - expect_io_error( + assert_eq!( result, - io::ErrorKind::InvalidData, - "decoding value at position 0: unknown server request code: 1337", + Err(ProtoDecodeError::InvalidData { + value_name: "server request code".to_string(), + cause: "unknown value 1337".to_string(), + position: 0, + }) ); } diff --git a/src/proto/server/response.rs b/src/proto/server/response.rs index dec6db0..7755a30 100644 --- a/src/proto/server/response.rs +++ b/src/proto/server/response.rs @@ -3,7 +3,9 @@ use std::net; use crate::proto::packet::{Packet, PacketReadError, ReadFromPacket}; use crate::proto::server::constants::*; -use crate::proto::{ProtoDecode, ProtoDecoder, ProtoEncode, ProtoEncoder, User, UserStatus}; +use crate::proto::{ + ProtoDecode, ProtoDecodeError, ProtoDecoder, ProtoEncode, ProtoEncoder, User, UserStatus, +}; /*=================* * SERVER RESPONSE * @@ -169,7 +171,8 @@ impl ProtoEncode for ServerResponse { } impl ProtoDecode for ServerResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { + let position = decoder.position(); let code: u32 = decoder.decode()?; let response = match code { CODE_CONNECT_TO_PEER => { @@ -241,10 +244,11 @@ impl ProtoDecode for ServerResponse { ServerResponse::WishlistIntervalResponse(response) } _ => { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - format!("unknown server response code: {}", code), - )); + return Err(ProtoDecodeError::InvalidData { + value_name: "server response code".to_string(), + cause: format!("unknown value {}", code), + position: position, + }); } }; Ok(response) @@ -297,7 +301,7 @@ impl ProtoEncode for ConnectToPeerResponse { } impl ProtoDecode for ConnectToPeerResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let user_name = decoder.decode()?; let connection_type = decoder.decode()?; let ip = decoder.decode()?; @@ -350,7 +354,7 @@ impl ProtoEncode for FileSearchResponse { } impl ProtoDecode for FileSearchResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let user_name = decoder.decode()?; let ticket = decoder.decode()?; let query = decoder.decode()?; @@ -426,7 +430,7 @@ impl ProtoEncode for LoginResponse { } impl ProtoDecode for LoginResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let ok: bool = decoder.decode()?; if !ok { let reason = decoder.decode()?; @@ -436,7 +440,7 @@ impl ProtoDecode for LoginResponse { let motd = decoder.decode()?; let ip = decoder.decode()?; - let result: io::Result = decoder.decode(); + let result = decoder.decode::(); match result { Ok(value) => debug!("LoginResponse last field: {}", value), Err(e) => debug!("Error reading LoginResponse field: {:?}", e), @@ -473,7 +477,7 @@ impl ProtoEncode for ParentMinSpeedResponse { } impl ProtoDecode for ParentMinSpeedResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let value = decoder.decode()?; Ok(ParentMinSpeedResponse { value }) } @@ -502,7 +506,7 @@ impl ProtoEncode for ParentSpeedRatioResponse { } impl ProtoDecode for ParentSpeedRatioResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let value = decoder.decode()?; Ok(ParentSpeedRatioResponse { value }) } @@ -538,7 +542,7 @@ impl ProtoEncode for PeerAddressResponse { } impl ProtoDecode for PeerAddressResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let username = decoder.decode()?; let ip = decoder.decode()?; let port = decoder.decode()?; @@ -569,7 +573,7 @@ impl ProtoEncode for PrivilegedUsersResponse { } impl ProtoDecode for PrivilegedUsersResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let users = decoder.decode()?; Ok(PrivilegedUsersResponse { users }) } @@ -733,7 +737,7 @@ impl ProtoEncode for UserInfo { } impl ProtoDecode for UserInfo { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let average_speed = decoder.decode()?; let num_downloads = decoder.decode()?; let unknown = decoder.decode()?; @@ -809,7 +813,7 @@ fn build_users( } impl ProtoDecode for RoomJoinResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; let user_names = decoder.decode()?; let user_statuses = decoder.decode()?; @@ -865,7 +869,7 @@ impl ProtoEncode for RoomLeaveResponse { } impl ProtoDecode for RoomLeaveResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; Ok(RoomLeaveResponse { room_name }) } @@ -954,7 +958,7 @@ impl RoomListResponse { rooms } - fn decode_rooms(decoder: &mut ProtoDecoder) -> io::Result> { + fn decode_rooms(decoder: &mut ProtoDecoder) -> Result, ProtoDecodeError> { let room_names = decoder.decode()?; let user_counts = decoder.decode()?; Ok(Self::build_rooms(room_names, user_counts)) @@ -984,7 +988,7 @@ impl ProtoEncode for RoomListResponse { } impl ProtoDecode for RoomListResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let rooms = RoomListResponse::decode_rooms(decoder)?; let owned_private_rooms = RoomListResponse::decode_rooms(decoder)?; let other_private_rooms = RoomListResponse::decode_rooms(decoder)?; @@ -1031,7 +1035,7 @@ impl ProtoEncode for RoomMessageResponse { } impl ProtoDecode for RoomMessageResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; let user_name = decoder.decode()?; let message = decoder.decode()?; @@ -1077,7 +1081,7 @@ impl ProtoEncode for RoomTickersResponse { } impl ProtoDecode for RoomTickersResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; let tickers = decoder.decode()?; Ok(RoomTickersResponse { room_name, tickers }) @@ -1139,7 +1143,7 @@ impl ProtoEncode for RoomUserJoinedResponse { } impl ProtoDecode for RoomUserJoinedResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; let user_name = decoder.decode()?; let status = decoder.decode()?; @@ -1182,7 +1186,7 @@ impl ProtoEncode for RoomUserLeftResponse { } impl ProtoDecode for RoomUserLeftResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let room_name = decoder.decode()?; let user_name = decoder.decode()?; Ok(RoomUserLeftResponse { @@ -1233,7 +1237,7 @@ impl ProtoEncode for UserInfoResponse { } impl ProtoDecode for UserInfoResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let user_name = decoder.decode()?; let average_speed: u32 = decoder.decode()?; let num_downloads: u32 = decoder.decode()?; @@ -1282,7 +1286,7 @@ impl ProtoEncode for UserStatusResponse { } impl ProtoDecode for UserStatusResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let user_name = decoder.decode()?; let status = decoder.decode()?; let is_privileged = decoder.decode()?; @@ -1317,7 +1321,7 @@ impl ProtoEncode for WishlistIntervalResponse { } impl ProtoDecode for WishlistIntervalResponse { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { let seconds = decoder.decode()?; Ok(WishlistIntervalResponse { seconds }) } @@ -1334,8 +1338,8 @@ mod tests { use bytes::BytesMut; - use crate::proto::base_codec::tests::{expect_io_error, roundtrip}; - use crate::proto::ProtoDecoder; + use crate::proto::base_codec::tests::roundtrip; + use crate::proto::{ProtoDecodeError, ProtoDecoder}; use super::*; @@ -1345,10 +1349,13 @@ mod tests { let result = ProtoDecoder::new(&bytes).decode::(); - expect_io_error( + assert_eq!( result, - io::ErrorKind::InvalidData, - "decoding value at position 0: unknown server response code: 1337", + Err(ProtoDecodeError::InvalidData { + value_name: "server response code".to_string(), + cause: "unknown value 1337".to_string(), + position: 0, + }) ); } diff --git a/src/proto/user.rs b/src/proto/user.rs index 19bb0ed..c0c6154 100644 --- a/src/proto/user.rs +++ b/src/proto/user.rs @@ -1,8 +1,8 @@ use std::io; use crate::proto::{ - MutPacket, Packet, PacketReadError, ProtoDecode, ProtoDecoder, ProtoEncode, ProtoEncoder, - ReadFromPacket, WriteToPacket, + MutPacket, Packet, PacketReadError, ProtoDecode, ProtoDecodeError, ProtoDecoder, ProtoEncode, + ProtoEncoder, ReadFromPacket, WriteToPacket, }; const STATUS_OFFLINE: u32 = 1; @@ -56,16 +56,18 @@ impl ProtoEncode for UserStatus { } impl ProtoDecode for UserStatus { - fn decode_from(decoder: &mut ProtoDecoder) -> io::Result { + fn decode_from(decoder: &mut ProtoDecoder) -> Result { + let position = decoder.position(); let value: u32 = decoder.decode()?; match value { STATUS_OFFLINE => Ok(UserStatus::Offline), STATUS_AWAY => Ok(UserStatus::Away), STATUS_ONLINE => Ok(UserStatus::Online), - _ => Err(io::Error::new( - io::ErrorKind::InvalidData, - format!("invalid user status: {}", value), - )), + _ => Err(ProtoDecodeError::InvalidData { + value_name: "user status".to_string(), + cause: format!("unknown value {}", value), + position: position, + }), } } }