From 9e78e4ded8a8b352c9fd02e507a25b02155e4087 Mon Sep 17 00:00:00 2001
From: anweiss <2326106+anweiss@users.noreply.github.com>
Date: Thu, 4 Aug 2022 15:15:23 -0400
Subject: [PATCH 1/3] implement validation error handling

---
 lifx-core/src/lib.rs | 86 +++++++++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 28 deletions(-)

diff --git a/lifx-core/src/lib.rs b/lifx-core/src/lib.rs
index dfe70f9..5594295 100644
--- a/lifx-core/src/lib.rs
+++ b/lifx-core/src/lib.rs
@@ -27,6 +27,7 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 use std::cmp::PartialEq;
 use std::convert::{TryFrom, TryInto};
 use std::ffi::{CStr, CString};
+use std::fmt::Write;
 use std::io;
 use std::io::Cursor;
 use thiserror::Error;
@@ -67,6 +68,9 @@ pub enum Error {
 
     #[error("i/o error")]
     Io(#[from] io::Error),
+
+    #[error("raw message error:\n`{0}`")]
+    RawMessage(String),
 }
 
 impl From<std::convert::Infallible> for Error {
@@ -1778,10 +1782,24 @@ impl Frame {
         8
     }
 
-    fn validate(&self) {
-        assert!(self.origin < 4);
-        assert!(self.addressable);
-        assert_eq!(self.protocol, 1024);
+    fn validate(&self) -> Result<(), Error> {
+        let mut errors = String::new();
+
+        if self.origin >= 4 {
+            let _ = writeln!(errors, "invalid origin value {}", self.origin);
+        }
+        if !self.addressable {
+            errors.push_str("missing target address\n");
+        }
+        if self.protocol != 1024 {
+            let _ = writeln!(errors, "invalid protocol number {}", self.protocol);
+        }
+
+        if !errors.is_empty() {
+            return Err(Error::RawMessage(errors));
+        }
+
+        Ok(())
     }
 
     fn pack(&self) -> Result<Vec<u8>, Error> {
@@ -1840,9 +1858,14 @@ impl FrameAddress {
     fn packed_size() -> usize {
         16
     }
-    fn validate(&self) {
-        //assert_eq!(self.reserved, [0;6]);
-        //assert_eq!(self.reserved2, 0);
+    fn validate(&self) -> Result<(), Error> {
+        if self.reserved != [0; 6] && self.reserved2 != 0 {
+            return Err(Error::RawMessage(
+                "reserved fields must be zeroed".to_string(),
+            ));
+        }
+
+        Ok(())
     }
     fn pack(&self) -> Result<Vec<u8>, Error> {
         let mut v = Vec::with_capacity(Self::packed_size());
@@ -1884,7 +1907,7 @@ impl FrameAddress {
             res_required,
             sequence,
         };
-        f.validate();
+        f.validate()?;
         Ok(f)
     }
 }
@@ -1893,9 +1916,14 @@ impl ProtocolHeader {
     fn packed_size() -> usize {
         12
     }
-    fn validate(&self) {
-        //assert_eq!(self.reserved, 0);
-        //assert_eq!(self.reserved2, 0);
+    fn validate(&self) -> Result<(), Error> {
+        if self.reserved != 0 && self.reserved2 != 0 {
+            return Err(Error::RawMessage(
+                "reserved fields must be zeroed".to_string(),
+            ));
+        }
+
+        Ok(())
     }
 
     /// Packs this part of the packet into some bytes
@@ -1918,7 +1946,7 @@ impl ProtocolHeader {
             typ,
             reserved2,
         };
-        f.validate();
+        f.validate()?;
         Ok(f)
     }
 }
@@ -2380,10 +2408,12 @@ impl RawMessage {
     }
 
     /// Validates that this object was constructed correctly.  Panics if not.
-    pub fn validate(&self) {
-        self.frame.validate();
-        self.frame_addr.validate();
-        self.protocol_header.validate();
+    pub fn validate(&self) -> Result<(), Error> {
+        self.frame.validate()?;
+        self.frame_addr.validate()?;
+        self.protocol_header.validate()?;
+
+        Ok(())
     }
 
     /// Packs this RawMessage into some bytes that can be send over the network.
@@ -2402,13 +2432,13 @@ impl RawMessage {
     pub fn unpack(v: &[u8]) -> Result<RawMessage, Error> {
         let mut start = 0;
         let frame = Frame::unpack(v)?;
-        frame.validate();
+        frame.validate()?;
         start += Frame::packed_size();
         let addr = FrameAddress::unpack(&v[start..])?;
-        addr.validate();
+        addr.validate()?;
         start += FrameAddress::packed_size();
         let proto = ProtocolHeader::unpack(&v[start..])?;
-        proto.validate();
+        proto.validate()?;
         start += ProtocolHeader::packed_size();
 
         let body = Vec::from(&v[start..(frame.size as usize)]);
@@ -2577,7 +2607,7 @@ mod tests {
             protocol: 1024,
             source: 1234567,
         };
-        frame.validate();
+        assert!(frame.validate().is_ok());
 
         let v = frame.pack().unwrap();
         println!("{:?}", v);
@@ -2643,7 +2673,7 @@ mod tests {
             res_required: false,
             sequence: 248,
         };
-        frame.validate();
+        assert!(frame.validate().is_ok());
 
         let v = frame.pack().unwrap();
         assert_eq!(v.len(), FrameAddress::packed_size());
@@ -2663,7 +2693,7 @@ mod tests {
         assert_eq!(v.len(), FrameAddress::packed_size());
 
         let frame = FrameAddress::unpack(&v).unwrap();
-        frame.validate();
+        assert!(frame.validate().is_ok());
         println!("FrameAddress: {:?}", frame);
     }
 
@@ -2674,7 +2704,7 @@ mod tests {
             reserved2: 0,
             typ: 0x4455,
         };
-        frame.validate();
+        assert!(frame.validate().is_ok());
 
         let v = frame.pack().unwrap();
         assert_eq!(v.len(), ProtocolHeader::packed_size());
@@ -2693,7 +2723,7 @@ mod tests {
         assert_eq!(v.len(), ProtocolHeader::packed_size());
 
         let frame = ProtocolHeader::unpack(&v).unwrap();
-        frame.validate();
+        assert!(frame.validate().is_ok());
         println!("ProtocolHeader: {:?}", frame);
     }
 
@@ -2706,7 +2736,7 @@ mod tests {
         ];
 
         let msg = RawMessage::unpack(&v).unwrap();
-        msg.validate();
+        assert!(msg.validate().is_ok());
         println!("{:#?}", msg);
     }
 
@@ -2723,7 +2753,7 @@ mod tests {
         ];
 
         let msg = RawMessage::unpack(&v).unwrap();
-        msg.validate();
+        assert!(msg.validate().is_ok());
         println!("{:#?}", msg);
     }
 
@@ -2839,7 +2869,7 @@ mod tests {
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         ];
         let rawmsg = RawMessage::unpack(&v).unwrap();
-        rawmsg.validate();
+        assert!(rawmsg.validate().is_ok());
 
         let msg = Message::from_raw(&rawmsg).unwrap();
 
@@ -2871,7 +2901,7 @@ mod tests {
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         ];
         let rawmsg = RawMessage::unpack(&v).unwrap();
-        rawmsg.validate();
+        assert!(rawmsg.validate().is_ok());
 
         let msg = Message::from_raw(&rawmsg).unwrap();
 

From a99901ab766adf1515480f4d9642b5b25ae0ee5b Mon Sep 17 00:00:00 2001
From: anweiss <2326106+anweiss@users.noreply.github.com>
Date: Mon, 15 Aug 2022 16:23:53 -0400
Subject: [PATCH 2/3] edits

---
 lifx-core/src/lib.rs | 46 +++++++++++++++++---------------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/lifx-core/src/lib.rs b/lifx-core/src/lib.rs
index 5594295..55e6e5b 100644
--- a/lifx-core/src/lib.rs
+++ b/lifx-core/src/lib.rs
@@ -27,7 +27,7 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 use std::cmp::PartialEq;
 use std::convert::{TryFrom, TryInto};
 use std::ffi::{CStr, CString};
-use std::fmt::Write;
+use std::fmt::{self, Write};
 use std::io;
 use std::io::Cursor;
 use thiserror::Error;
@@ -69,8 +69,8 @@ pub enum Error {
     #[error("i/o error")]
     Io(#[from] io::Error),
 
-    #[error("raw message error:\n`{0}`")]
-    RawMessage(String),
+    #[error("error unpacking raw message: `{0}`")]
+    RawMessage(&'static str),
 }
 
 impl From<std::convert::Infallible> for Error {
@@ -1783,20 +1783,14 @@ impl Frame {
     }
 
     fn validate(&self) -> Result<(), Error> {
-        let mut errors = String::new();
-
         if self.origin >= 4 {
-            let _ = writeln!(errors, "invalid origin value {}", self.origin);
+            return Err(Error::RawMessage("invalid origin value in frame"));
         }
         if !self.addressable {
-            errors.push_str("missing target address\n");
+            return Err(Error::RawMessage("missing target address in frame"));
         }
         if self.protocol != 1024 {
-            let _ = writeln!(errors, "invalid protocol number {}", self.protocol);
-        }
-
-        if !errors.is_empty() {
-            return Err(Error::RawMessage(errors));
+            return Err(Error::RawMessage("invalid protocol number in frame"));
         }
 
         Ok(())
@@ -1860,9 +1854,7 @@ impl FrameAddress {
     }
     fn validate(&self) -> Result<(), Error> {
         if self.reserved != [0; 6] && self.reserved2 != 0 {
-            return Err(Error::RawMessage(
-                "reserved fields must be zeroed".to_string(),
-            ));
+            return Err(Error::RawMessage("reserved fields must be zeroed"));
         }
 
         Ok(())
@@ -1918,9 +1910,7 @@ impl ProtocolHeader {
     }
     fn validate(&self) -> Result<(), Error> {
         if self.reserved != 0 && self.reserved2 != 0 {
-            return Err(Error::RawMessage(
-                "reserved fields must be zeroed".to_string(),
-            ));
+            return Err(Error::RawMessage("reserved fields must be zeroed"));
         }
 
         Ok(())
@@ -2407,7 +2397,7 @@ impl RawMessage {
             + self.payload.len()
     }
 
-    /// Validates that this object was constructed correctly.  Panics if not.
+    /// Validates that this object was constructed correctly.
     pub fn validate(&self) -> Result<(), Error> {
         self.frame.validate()?;
         self.frame_addr.validate()?;
@@ -2607,7 +2597,7 @@ mod tests {
             protocol: 1024,
             source: 1234567,
         };
-        assert!(frame.validate().is_ok());
+        frame.validate().unwrap();
 
         let v = frame.pack().unwrap();
         println!("{:?}", v);
@@ -2673,7 +2663,7 @@ mod tests {
             res_required: false,
             sequence: 248,
         };
-        assert!(frame.validate().is_ok());
+        frame.validate().unwrap();
 
         let v = frame.pack().unwrap();
         assert_eq!(v.len(), FrameAddress::packed_size());
@@ -2693,7 +2683,7 @@ mod tests {
         assert_eq!(v.len(), FrameAddress::packed_size());
 
         let frame = FrameAddress::unpack(&v).unwrap();
-        assert!(frame.validate().is_ok());
+        frame.validate().unwrap();
         println!("FrameAddress: {:?}", frame);
     }
 
@@ -2704,7 +2694,7 @@ mod tests {
             reserved2: 0,
             typ: 0x4455,
         };
-        assert!(frame.validate().is_ok());
+        frame.validate().unwrap();
 
         let v = frame.pack().unwrap();
         assert_eq!(v.len(), ProtocolHeader::packed_size());
@@ -2723,7 +2713,7 @@ mod tests {
         assert_eq!(v.len(), ProtocolHeader::packed_size());
 
         let frame = ProtocolHeader::unpack(&v).unwrap();
-        assert!(frame.validate().is_ok());
+        frame.validate().unwrap();
         println!("ProtocolHeader: {:?}", frame);
     }
 
@@ -2736,7 +2726,7 @@ mod tests {
         ];
 
         let msg = RawMessage::unpack(&v).unwrap();
-        assert!(msg.validate().is_ok());
+        msg.validate().unwrap();
         println!("{:#?}", msg);
     }
 
@@ -2753,7 +2743,7 @@ mod tests {
         ];
 
         let msg = RawMessage::unpack(&v).unwrap();
-        assert!(msg.validate().is_ok());
+        msg.validate().unwrap();
         println!("{:#?}", msg);
     }
 
@@ -2869,7 +2859,7 @@ mod tests {
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         ];
         let rawmsg = RawMessage::unpack(&v).unwrap();
-        assert!(rawmsg.validate().is_ok());
+        rawmsg.validate().unwrap();
 
         let msg = Message::from_raw(&rawmsg).unwrap();
 
@@ -2901,7 +2891,7 @@ mod tests {
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         ];
         let rawmsg = RawMessage::unpack(&v).unwrap();
-        assert!(rawmsg.validate().is_ok());
+        rawmsg.validate().unwrap();
 
         let msg = Message::from_raw(&rawmsg).unwrap();
 

From baa5fccc47d515876b6d96cb2c97b21069757c90 Mon Sep 17 00:00:00 2001
From: anweiss <2326106+anweiss@users.noreply.github.com>
Date: Tue, 16 Aug 2022 09:47:24 -0400
Subject: [PATCH 3/3] option for strict reserved field validation

---
 Cargo.lock                     |  7 ++++++
 Cargo.toml                     |  2 +-
 lifx-core/src/lib.rs           | 39 +++++++++++++++++-----------------
 utils/get_all_info/src/main.rs |  9 ++++----
 4 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 5309e0b..6f3c2c0 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -70,6 +70,13 @@ version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
 
+[[package]]
+name = "color_test"
+version = "0.1.0"
+dependencies = [
+ "lifx-core",
+]
+
 [[package]]
 name = "derive_arbitrary"
 version = "1.1.3"
diff --git a/Cargo.toml b/Cargo.toml
index c5e969d..d2fd94e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -9,7 +9,7 @@ readme = "README.md"
 edition = "2018"
 
 [workspace]
-members = ["lifx-core", "examples/multizone_test", "examples/waveform_test", "utils/get_all_info", "xtask"]
+members = ["lifx-core", "examples/color_test", "examples/multizone_test", "examples/waveform_test", "utils/get_all_info", "xtask"]
 
 [lib]
 
diff --git a/lifx-core/src/lib.rs b/lifx-core/src/lib.rs
index 55e6e5b..be1ee85 100644
--- a/lifx-core/src/lib.rs
+++ b/lifx-core/src/lib.rs
@@ -27,7 +27,6 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 use std::cmp::PartialEq;
 use std::convert::{TryFrom, TryInto};
 use std::ffi::{CStr, CString};
-use std::fmt::{self, Write};
 use std::io;
 use std::io::Cursor;
 use thiserror::Error;
@@ -1852,8 +1851,8 @@ impl FrameAddress {
     fn packed_size() -> usize {
         16
     }
-    fn validate(&self) -> Result<(), Error> {
-        if self.reserved != [0; 6] && self.reserved2 != 0 {
+    fn validate(&self, strict: bool) -> Result<(), Error> {
+        if strict && self.reserved != [0; 6] && self.reserved2 != 0 {
             return Err(Error::RawMessage("reserved fields must be zeroed"));
         }
 
@@ -1899,7 +1898,7 @@ impl FrameAddress {
             res_required,
             sequence,
         };
-        f.validate()?;
+        f.validate(false)?;
         Ok(f)
     }
 }
@@ -1908,8 +1907,8 @@ impl ProtocolHeader {
     fn packed_size() -> usize {
         12
     }
-    fn validate(&self) -> Result<(), Error> {
-        if self.reserved != 0 && self.reserved2 != 0 {
+    fn validate(&self, strict: bool) -> Result<(), Error> {
+        if strict && self.reserved != 0 && self.reserved2 != 0 {
             return Err(Error::RawMessage("reserved fields must be zeroed"));
         }
 
@@ -1936,7 +1935,7 @@ impl ProtocolHeader {
             typ,
             reserved2,
         };
-        f.validate()?;
+        f.validate(false)?;
         Ok(f)
     }
 }
@@ -2398,10 +2397,10 @@ impl RawMessage {
     }
 
     /// Validates that this object was constructed correctly.
-    pub fn validate(&self) -> Result<(), Error> {
+    pub fn validate(&self, strict: bool) -> Result<(), Error> {
         self.frame.validate()?;
-        self.frame_addr.validate()?;
-        self.protocol_header.validate()?;
+        self.frame_addr.validate(strict)?;
+        self.protocol_header.validate(strict)?;
 
         Ok(())
     }
@@ -2425,10 +2424,10 @@ impl RawMessage {
         frame.validate()?;
         start += Frame::packed_size();
         let addr = FrameAddress::unpack(&v[start..])?;
-        addr.validate()?;
+        addr.validate(false)?;
         start += FrameAddress::packed_size();
         let proto = ProtocolHeader::unpack(&v[start..])?;
-        proto.validate()?;
+        proto.validate(false)?;
         start += ProtocolHeader::packed_size();
 
         let body = Vec::from(&v[start..(frame.size as usize)]);
@@ -2663,7 +2662,7 @@ mod tests {
             res_required: false,
             sequence: 248,
         };
-        frame.validate().unwrap();
+        frame.validate(false).unwrap();
 
         let v = frame.pack().unwrap();
         assert_eq!(v.len(), FrameAddress::packed_size());
@@ -2683,7 +2682,7 @@ mod tests {
         assert_eq!(v.len(), FrameAddress::packed_size());
 
         let frame = FrameAddress::unpack(&v).unwrap();
-        frame.validate().unwrap();
+        frame.validate(false).unwrap();
         println!("FrameAddress: {:?}", frame);
     }
 
@@ -2694,7 +2693,7 @@ mod tests {
             reserved2: 0,
             typ: 0x4455,
         };
-        frame.validate().unwrap();
+        frame.validate(false).unwrap();
 
         let v = frame.pack().unwrap();
         assert_eq!(v.len(), ProtocolHeader::packed_size());
@@ -2713,7 +2712,7 @@ mod tests {
         assert_eq!(v.len(), ProtocolHeader::packed_size());
 
         let frame = ProtocolHeader::unpack(&v).unwrap();
-        frame.validate().unwrap();
+        frame.validate(false).unwrap();
         println!("ProtocolHeader: {:?}", frame);
     }
 
@@ -2726,7 +2725,7 @@ mod tests {
         ];
 
         let msg = RawMessage::unpack(&v).unwrap();
-        msg.validate().unwrap();
+        msg.validate(false).unwrap();
         println!("{:#?}", msg);
     }
 
@@ -2743,7 +2742,7 @@ mod tests {
         ];
 
         let msg = RawMessage::unpack(&v).unwrap();
-        msg.validate().unwrap();
+        msg.validate(false).unwrap();
         println!("{:#?}", msg);
     }
 
@@ -2859,7 +2858,7 @@ mod tests {
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         ];
         let rawmsg = RawMessage::unpack(&v).unwrap();
-        rawmsg.validate().unwrap();
+        rawmsg.validate(false).unwrap();
 
         let msg = Message::from_raw(&rawmsg).unwrap();
 
@@ -2891,7 +2890,7 @@ mod tests {
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
         ];
         let rawmsg = RawMessage::unpack(&v).unwrap();
-        rawmsg.validate().unwrap();
+        rawmsg.validate(false).unwrap();
 
         let msg = Message::from_raw(&rawmsg).unwrap();
 
diff --git a/utils/get_all_info/src/main.rs b/utils/get_all_info/src/main.rs
index 43c297a..e71358a 100644
--- a/utils/get_all_info/src/main.rs
+++ b/utils/get_all_info/src/main.rs
@@ -359,9 +359,10 @@ impl Manager {
 
         for addr in get_if_addrs().unwrap() {
             if let IfAddr::V4(Ifv4Addr {
-                                broadcast: Some(bcast),
-                                ..
-                            }) = addr.addr {
+                broadcast: Some(bcast),
+                ..
+            }) = addr.addr
+            {
                 if addr.ip().is_loopback() {
                     continue;
                 }
@@ -395,7 +396,7 @@ fn main() {
         }
         mgr.refresh();
 
-        println!("\n\n\n\n");
+        // println!("\n\n\n\n");
         if let Ok(bulbs) = mgr.bulbs.lock() {
             let bulbs = bulbs.values();
             for bulb in bulbs {