fix: minimize assertions in non-test code (#56)

Initial effort to reduce or eliminate the use of assertions in the
production code (i.e. not test code). See issue 43 for an example of how
this can cause an application (thread) to panic when reading an unusual
file. Not all of the changes were as simple as returning an Err, since some
functions did not return a Result. Also, the error type used (InvalidData)
is just a catch-all with a message, and in some cases a more refined error
type may be in order.

cargo test passes

src/mp4box/dinf.rs

@@ -260,7 +260,9 @@ impl<R: Read + Seek> ReadBox<&mut R> for UrlBox {
             reader.read_exact(&mut buf)?;
             match String::from_utf8(buf) {
                 Ok(t) => {
-                    assert_eq!(t.len(), buf_size as usize);
+                    if t.len() != buf_size as usize {
+                        return Err(Error::InvalidData("string too small"))
+                    }
                     t
                 }
                 _ => String::default(),

src/mp4box/elst.rs

@@ -30,8 +30,7 @@ impl ElstBox {
         let mut size = HEADER_SIZE + HEADER_EXT_SIZE + 4;
         if self.version == 1 {
             size += self.entries.len() as u64 * 20;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             size += self.entries.len() as u64 * 12;
         }
         size

src/mp4box/hdlr.rs

@@ -58,7 +58,9 @@ impl<R: Read + Seek> ReadBox<&mut R> for HdlrBox {
 
         let handler_string = match String::from_utf8(buf) {
             Ok(t) => {
-                assert_eq!(t.len(), buf_size as usize);
+                if t.len() != buf_size as usize {
+                    return Err(Error::InvalidData("string too small"))
+                }
                 t
             }
             _ => String::from("null"),

src/mp4box/mdhd.rs

@@ -26,8 +26,7 @@ impl MdhdBox {
 
         if self.version == 1 {
             size += 28;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             size += 16;
         }
         size += 4;
@@ -82,14 +81,15 @@ impl<R: Read + Seek> ReadBox<&mut R> for MdhdBox {
                 reader.read_u32::<BigEndian>()?,
                 reader.read_u64::<BigEndian>()?,
             )
-        } else {
-            assert_eq!(version, 0);
+        } else if version == 0 {
             (
                 reader.read_u32::<BigEndian>()? as u64,
                 reader.read_u32::<BigEndian>()? as u64,
                 reader.read_u32::<BigEndian>()?,
                 reader.read_u32::<BigEndian>()? as u64,
             )
+        } else {
+            return Err(Error::InvalidData("version must be 0 or 1"));
         };
         let language_code = reader.read_u16::<BigEndian>()?;
         let language = language_string(language_code);
@@ -120,12 +120,13 @@ impl<W: Write> WriteBox<&mut W> for MdhdBox {
             writer.write_u64::<BigEndian>(self.modification_time)?;
             writer.write_u32::<BigEndian>(self.timescale)?;
             writer.write_u64::<BigEndian>(self.duration)?;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             writer.write_u32::<BigEndian>(self.creation_time as u32)?;
             writer.write_u32::<BigEndian>(self.modification_time as u32)?;
             writer.write_u32::<BigEndian>(self.timescale)?;
             writer.write_u32::<BigEndian>(self.duration as u32)?;
+        } else {
+            return Err(Error::InvalidData("version must be 0 or 1"));
         }
 
         let language_code = language_code(&self.language);

src/mp4box/mehd.rs

@@ -21,8 +21,7 @@ impl MehdBox {
 
         if self.version == 1 {
             size += 8;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             size += 4;
         }
         size
@@ -66,9 +65,10 @@ impl<R: Read + Seek> ReadBox<&mut R> for MehdBox {
 
         let fragment_duration = if version == 1 {
             reader.read_u64::<BigEndian>()?
-        } else {
-            assert_eq!(version, 0);
+        } else if version == 0 {
             reader.read_u32::<BigEndian>()? as u64
+        } else {
+            return Err(Error::InvalidData("version must be 0 or 1"));
         };
         skip_bytes_to(reader, start + size)?;
 
@@ -89,9 +89,10 @@ impl<W: Write> WriteBox<&mut W> for MehdBox {
 
         if self.version == 1 {
             writer.write_u64::<BigEndian>(self.fragment_duration)?;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             writer.write_u32::<BigEndian>(self.fragment_duration as u32)?;
+        } else {
+            return Err(Error::InvalidData("version must be 0 or 1"));
         }
 
         Ok(size)

src/mp4box/mvhd.rs

@@ -26,8 +26,7 @@ impl MvhdBox {
         let mut size = HEADER_SIZE + HEADER_EXT_SIZE;
         if self.version == 1 {
             size += 28;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             size += 16;
         }
         size += 80;
@@ -82,14 +81,15 @@ impl<R: Read + Seek> ReadBox<&mut R> for MvhdBox {
                 reader.read_u32::<BigEndian>()?,
                 reader.read_u64::<BigEndian>()?,
             )
-        } else {
-            assert_eq!(version, 0);
+        } else if version == 0 {
             (
                 reader.read_u32::<BigEndian>()? as u64,
                 reader.read_u32::<BigEndian>()? as u64,
                 reader.read_u32::<BigEndian>()?,
                 reader.read_u32::<BigEndian>()? as u64,
             )
+        } else {
+            return Err(Error::InvalidData("version must be 0 or 1"));
         };
         let rate = FixedPointU16::new_raw(reader.read_u32::<BigEndian>()?);
 
@@ -119,12 +119,13 @@ impl<W: Write> WriteBox<&mut W> for MvhdBox {
             writer.write_u64::<BigEndian>(self.modification_time)?;
             writer.write_u32::<BigEndian>(self.timescale)?;
             writer.write_u64::<BigEndian>(self.duration)?;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             writer.write_u32::<BigEndian>(self.creation_time as u32)?;
             writer.write_u32::<BigEndian>(self.modification_time as u32)?;
             writer.write_u32::<BigEndian>(self.timescale)?;
             writer.write_u32::<BigEndian>(self.duration as u32)?;
+        } else {
+            return Err(Error::InvalidData("version must be 0 or 1"));
         }
         writer.write_u32::<BigEndian>(self.rate.raw_value())?;
 

src/mp4box/stsz.rs

@@ -83,7 +83,9 @@ impl<W: Write> WriteBox<&mut W> for StszBox {
         writer.write_u32::<BigEndian>(self.sample_size)?;
         writer.write_u32::<BigEndian>(self.sample_count)?;
         if self.sample_size == 0 {
-            assert_eq!(self.sample_count, self.sample_sizes.len() as u32);
+            if self.sample_count != self.sample_sizes.len() as u32 {
+                return Err(Error::InvalidData("sample count out of sync"));
+            }
             for sample_number in self.sample_sizes.iter() {
                 writer.write_u32::<BigEndian>(*sample_number)?;
             }

src/mp4box/tkhd.rs

@@ -73,8 +73,7 @@ impl TkhdBox {
         let mut size = HEADER_SIZE + HEADER_EXT_SIZE;
         if self.version == 1 {
             size += 32;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             size += 20;
         }
         size += 60;
@@ -125,8 +124,7 @@ impl<R: Read + Seek> ReadBox<&mut R> for TkhdBox {
                 reader.read_u32::<BigEndian>()?,
                 reader.read_u64::<BigEndian>()?,
             )
-        } else {
-            assert_eq!(version, 0);
+        } else if version == 0 {
             (
                 reader.read_u32::<BigEndian>()? as u64,
                 reader.read_u32::<BigEndian>()? as u64,
@@ -134,6 +132,8 @@ impl<R: Read + Seek> ReadBox<&mut R> for TkhdBox {
                 reader.read_u32::<BigEndian>()?,
                 reader.read_u32::<BigEndian>()? as u64,
             )
+        } else {
+            return Err(Error::InvalidData("version must be 0 or 1"));
         };
         reader.read_u64::<BigEndian>()?; // reserved
         let layer = reader.read_u16::<BigEndian>()?;
@@ -188,13 +188,14 @@ impl<W: Write> WriteBox<&mut W> for TkhdBox {
             writer.write_u32::<BigEndian>(self.track_id)?;
             writer.write_u32::<BigEndian>(0)?; // reserved
             writer.write_u64::<BigEndian>(self.duration)?;
-        } else {
-            assert_eq!(self.version, 0);
+        } else if self.version == 0 {
             writer.write_u32::<BigEndian>(self.creation_time as u32)?;
             writer.write_u32::<BigEndian>(self.modification_time as u32)?;
             writer.write_u32::<BigEndian>(self.track_id)?;
             writer.write_u32::<BigEndian>(0)?; // reserved
             writer.write_u32::<BigEndian>(self.duration as u32)?;
+        } else {
+            return Err(Error::InvalidData("version must be 0 or 1"));
         }
 
         writer.write_u64::<BigEndian>(0)?; // reserved

src/mp4box/trun.rs

@@ -154,7 +154,9 @@ impl<W: Write> WriteBox<&mut W> for TrunBox {
         if let Some(v) = self.first_sample_flags {
             writer.write_u32::<BigEndian>(v)?;
         }
-        assert_eq!(self.sample_count, self.sample_sizes.len() as u32);
+        if self.sample_count != self.sample_sizes.len() as u32 {
+            return Err(Error::InvalidData("sample count out of sync"));
+        }
         for i in 0..self.sample_count as usize {
             if TrunBox::FLAG_SAMPLE_DURATION & self.flags > 0 {
                 writer.write_u32::<BigEndian>(self.sample_durations[i])?;

src/reader.rs

@@ -66,7 +66,9 @@ impl<R: Read + Seek> Mp4Reader<R> {
         let mut tracks = if let Some(ref moov) = moov {
             let mut tracks = Vec::with_capacity(moov.traks.len());
             for (i, trak) in moov.traks.iter().enumerate() {
-                assert_eq!(trak.tkhd.track_id, i as u32 + 1);
+                if trak.tkhd.track_id != i as u32 + 1 {
+                    return Err(Error::InvalidData("tracks out of order"));
+                }
                 tracks.push(Mp4Track::from(trak));
             }
             tracks

src/track.rs

@@ -303,19 +303,26 @@ impl Mp4Track {
         }
     }
 
-    fn stsc_index(&self, sample_id: u32) -> usize {
+    fn stsc_index(&self, sample_id: u32) -> Result<usize> {
+        if self.trak.mdia.minf.stbl.stsc.entries.is_empty() {
+            return Err(Error::InvalidData("no stsc entries"));
+        }
         for (i, entry) in self.trak.mdia.minf.stbl.stsc.entries.iter().enumerate() {
             if sample_id < entry.first_sample {
-                assert_ne!(i, 0);
-                return i - 1;
+                return if i == 0 {
+                    Err(Error::InvalidData("sample not found"))
+                } else {
+                    Ok(i - 1)
+                };
             }
         }
-
-        assert_ne!(self.trak.mdia.minf.stbl.stsc.entries.len(), 0);
-        self.trak.mdia.minf.stbl.stsc.entries.len() - 1
+        Ok(self.trak.mdia.minf.stbl.stsc.entries.len() - 1)
     }
 
     fn chunk_offset(&self, chunk_id: u32) -> Result<u64> {
+        if self.trak.mdia.minf.stbl.stco.is_none() && self.trak.mdia.minf.stbl.co64.is_none() {
+            return Err(Error::InvalidData("must have either stco or co64 boxes"));
+        }
         if let Some(ref stco) = self.trak.mdia.minf.stbl.stco {
             if let Some(offset) = stco.entries.get(chunk_id as usize - 1) {
                 return Ok(*offset as u64);
@@ -326,21 +333,17 @@ impl Mp4Track {
                     chunk_id,
                 ));
             }
-        } else {
-            if let Some(ref co64) = self.trak.mdia.minf.stbl.co64 {
-                if let Some(offset) = co64.entries.get(chunk_id as usize - 1) {
-                    return Ok(*offset);
-                } else {
-                    return Err(Error::EntryInStblNotFound(
-                        self.track_id(),
-                        BoxType::Co64Box,
-                        chunk_id,
-                    ));
-                }
+        } else if let Some(ref co64) = self.trak.mdia.minf.stbl.co64 {
+            if let Some(offset) = co64.entries.get(chunk_id as usize - 1) {
+                return Ok(*offset);
+            } else {
+                return Err(Error::EntryInStblNotFound(
+                    self.track_id(),
+                    BoxType::Co64Box,
+                    chunk_id,
+                ));
             }
         }
-
-        assert!(self.trak.mdia.minf.stbl.stco.is_some() || self.trak.mdia.minf.stbl.co64.is_some());
         return Err(Error::Box2NotFound(BoxType::StcoBox, BoxType::Co64Box));
     }
 
@@ -436,7 +439,7 @@ impl Mp4Track {
                 ))
             }
         } else {
-            let stsc_index = self.stsc_index(sample_id);
+            let stsc_index = self.stsc_index(sample_id)?;
 
             let stsc = &self.trak.mdia.minf.stbl.stsc;
             let stsc_entry = stsc.entries.get(stsc_index).unwrap();
@@ -631,7 +634,6 @@ impl Mp4TrackWriter {
                 self.is_fixed_sample_size = true;
             }
         } else {
-            assert!(self.trak.mdia.minf.stbl.stsz.sample_count > 0);
             if self.is_fixed_sample_size {
                 if self.fixed_sample_size != size {
                     self.is_fixed_sample_size = false;

src/writer.rs

@@ -116,7 +116,9 @@ impl<W: Write + Seek> Mp4Writer<W> {
     fn update_mdat_size(&mut self) -> Result<()> {
         let mdat_end = self.writer.seek(SeekFrom::Current(0))?;
         let mdat_size = mdat_end - self.mdat_pos;
-        assert!(mdat_size < std::u32::MAX as u64);
+        if mdat_size > std::u32::MAX as u64 {
+            return Err(Error::InvalidData("mdat size too large"));
+        }
         self.writer.seek(SeekFrom::Start(self.mdat_pos))?;
         self.writer.write_u32::<BigEndian>(mdat_size as u32)?;
         self.writer.seek(SeekFrom::Start(mdat_end))?;