Better (more correct) error handling

src/main.rs

@@ -48,11 +48,8 @@ async fn send_bad_request<S: AsyncSendTo>(
     socket: &mut S,
     source: SocketAddr,
     resp_buffer: &mut [u8],
-    message: Option<&IncomingMessage>,
+    message: &IncomingMessage,
 ) -> Result<(), Error> {
-    // Dummy message in case the message failed to parse at all
-    let dummy = IncomingMessage::invalid();
-    let message = message.unwrap_or(&dummy);
     let mut response = Response::new(false);
     response.add_attribute(Attribute::ErrorCode(ErrorCode::BadRequest));
     let size = response.wire_serialize(&message, resp_buffer)?;
@@ -70,10 +67,6 @@ async fn handle_message_parse<S: AsyncSendTo>(
 ) -> Result<IncomingMessage, Error> {
     match deserialize_message(message) {
         DeserializeResult::Ok(message) => Ok(message),
-        DeserializeResult::Error(err) => {
-            send_bad_request(socket, source, resp_buffer, None).await?;
-            Err(err)
-        }
         DeserializeResult::UnknownComprehensionAttributes(message, ids)
             if message.class == Class::Request =>
         {
@@ -84,6 +77,15 @@ async fn handle_message_parse<S: AsyncSendTo>(
             socket.send_to(&resp_buffer[..size], source).await?;
             Err(Error::UnrecognizedComprehensionAttributes)
         }
+        // Parsed the request, but failed when parsing attributes or something else
+        DeserializeResult::MalformedMessage(message, error) if message.class == Class::Request => {
+            send_bad_request(socket, source, resp_buffer, &message).await?;
+            Err(error)
+        }
+        // Couldn't parse the request at all
+        DeserializeResult::Error(err) => Err(err),
+        // Don't respond to non-requests
+        DeserializeResult::MalformedMessage(_, err) => Err(err),
         DeserializeResult::UnknownComprehensionAttributes(_, _) => Err(Error::MalformedMessage),
     }
 }
@@ -111,7 +113,7 @@ async fn handle_message<S: AsyncSendTo>(
         }
         // Unrecognized request
         (Class::Request, _) => {
-            send_bad_request(socket, source, &mut resp_buffer, Some(&message)).await?;
+            send_bad_request(socket, source, &mut resp_buffer, &message).await?;
         }
         // 6.3.2: No response is generated for indication
         (Class::Indication, _) => {}

src/protocol/convert.rs

@@ -22,6 +22,7 @@ use super::{
 pub enum DeserializeResult {
     Ok(IncomingMessage),
     UnknownComprehensionAttributes(IncomingMessage, Vec<u16>),
+    MalformedMessage(IncomingMessage, Error),
     Error(Error),
 }
 
@@ -173,7 +174,7 @@ pub fn deserialize_message(data: &[u8]) -> DeserializeResult {
         }
 
         if attr_data.len() < size_of::<RawAttributeHeader>() {
-            return DR::Error(Error::TruncatedMessage);
+            return DR::MalformedMessage(message, Error::TruncatedMessage);
         }
 
         let header: &RawAttributeHeader = get_struct(attr_data);
@@ -181,7 +182,7 @@ pub fn deserialize_message(data: &[u8]) -> DeserializeResult {
         let len = u16::from_be(header.len) as usize;
 
         if len + size_of::<RawAttributeHeader>() > attr_data.len() {
-            return DR::Error(Error::TruncatedMessage);
+            return DR::MalformedMessage(message, Error::TruncatedMessage);
         }
 
         let attr_data = &attr_data[size_of::<RawAttributeHeader>()..];
@@ -198,7 +199,9 @@ pub fn deserialize_message(data: &[u8]) -> DeserializeResult {
                         raw.to_sockaddr(&transaction_id),
                     ))
                 }
-                _ => return DR::Error(Error::MalformedMessage),
+                _ => {
+                    return DR::MalformedMessage(message, Error::TruncatedMessage);
+                }
             },
             ty if ty.is_comprehension_required() => {
                 unknown_attributes.push(ty.0);