Escape and truncate error messages

zebra-network/src/protocol/external/message.rs

@@ -6,7 +6,6 @@ use chrono::{DateTime, Utc};
 
 use zebra_chain::{
     block::{self, Block},
-    serialization::ZcashSerialize,
     transaction::UnminedTx,
 };
 
@@ -351,40 +350,37 @@ const MAX_REJECT_MESSAGE_LENGTH: usize = 12;
 /// This is equivalent to `MAX_REJECT_MESSAGE_LENGTH` in zcashd.
 const MAX_REJECT_REASON_LENGTH: usize = 111;
 
+// TODO: add tests for Error conversion and Reject message serialization (#4633)
+// (Zebra does not currently send reject messages, and it ignores received reject messages.)
 impl<E> From<E> for Message
 where
     E: Error,
 {
     fn from(e: E) -> Self {
-        let message_bytes = e.to_string().zcash_serialize_to_vec().unwrap_or_default();
+        let message = e
+            .to_string()
+            .escape_default()
+            .take(MAX_REJECT_MESSAGE_LENGTH)
+            .collect();
+        let reason = e
+            .source()
+            .map(ToString::to_string)
+            .unwrap_or_default()
+            .escape_default()
+            .take(MAX_REJECT_REASON_LENGTH)
+            .collect();
 
         Message::Reject {
-            message: String::from_utf8(
-                message_bytes[0..std::cmp::min(MAX_REJECT_MESSAGE_LENGTH, message_bytes.len())]
-                    .to_vec(),
-            )
-            .unwrap_or_else(|_| String::from("")),
+            message,
 
             // The generic case, impls for specific error types should
             // use specific varieties of `RejectReason`.
             ccode: RejectReason::Other,
 
-            reason: if let Some(reason) = e.source() {
-                let reason_bytes = reason
-                    .to_string()
-                    .zcash_serialize_to_vec()
-                    .unwrap_or_default();
-
-                String::from_utf8(
-                    reason_bytes[0..std::cmp::min(MAX_REJECT_REASON_LENGTH, reason_bytes.len())]
-                        .to_vec(),
-                )
-                .unwrap_or_else(|_| String::from(""))
-            } else {
-                String::from("")
-            },
-
-            // Allow this to be overridden but not populated by default, methinks.
+            reason,
+
+            // The hash of the rejected block or transaction.
+            // We don't have that data here, so the caller needs to fill it in later.
             data: None,
         }
     }