From 9041eeb907c36cff7ee7fb6a116d52d6f3bcb16d Mon Sep 17 00:00:00 2001
From: Scott Lamb <slamb@slamb.org>
Date: Tue, 17 Oct 2017 08:55:21 -0700
Subject: [PATCH] fix panic when requesting zero segment duration

The recording::Segment was constructing a segment with no frames in it, which
was causing a panic when appending a zero-length stts to the Slices. Fix this
in a couple ways:

* Slices::append should return Err rather than panic. No reason to crash the
  whole program when we have trouble serving a single .mp4 request.
* recording::Segment shouldn't produce zero-frame segments
---
 src/mp4.rs       |  3 +--
 src/recording.rs | 17 ++++++++++++++++-
 src/slices.rs    | 23 ++++++++++++++---------
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/src/mp4.rs b/src/mp4.rs
index 955db41..eb53b74 100644
--- a/src/mp4.rs
+++ b/src/mp4.rs
@@ -1405,8 +1405,7 @@ impl BodyState {
 
     fn append_slice(&mut self, len: u64, t: SliceType, p: usize) -> Result<(), Error> {
         let l = self.slices.len();
-        self.slices.append(Slice::new(l + len, t, p)?);
-        Ok(())
+        self.slices.append(Slice::new(l + len, t, p)?)
     }
 
     /// Appends a static bytestring, flushing the buffer if necessary.
diff --git a/src/recording.rs b/src/recording.rs
index 6ae3b81..cb2c49e 100644
--- a/src/recording.rs
+++ b/src/recording.rs
@@ -406,10 +406,13 @@ impl Segment {
         if self_.desired_range_90k.start == 0 &&
            self_.desired_range_90k.end == recording.duration_90k {
             // Fast path. Existing entry is fine.
+            trace!("recording::Segment::new fast path, recording={:#?}", recording);
             return Ok(self_)
         }
 
         // Slow path. Need to iterate through the index.
+        trace!("recording::Segment::new slow path, desired_range_90k={:?}, recording={:#?}",
+               self_.desired_range_90k, recording);
         db.with_recording_playback(self_.camera_id, self_.recording_id, |playback| {
             let mut begin = Box::new(SampleIndexIterator::new());
             let data = &(&playback).video_index;
@@ -441,7 +444,7 @@ impl Segment {
                     self_.frames = 0;
                     self_.key_frames = 0;
                 }
-                if it.start_90k >= end_90k {
+                if it.start_90k >= end_90k && self_.frames > 0 {
                     break;
                 }
                 self_.frames += 1;
@@ -709,6 +712,18 @@ mod tests {
         assert_eq!(&get_frames(&db.db, &segment, |it| it.bytes), &[2, 3]);
     }
 
+    /// Even if the desired duration is 0, there should still be a frame.
+    #[test]
+    fn test_segment_zero_desired_duration() {
+        testutil::init();
+        let mut encoder = SampleIndexEncoder::new();
+        encoder.add_sample(1, 1, true);
+        let db = TestDb::new();
+        let row = db.create_recording_from_encoder(encoder);
+        let segment = Segment::new(&db.db.lock(), &row, 0 .. 0).unwrap();
+        assert_eq!(&get_frames(&db.db, &segment, |it| it.bytes), &[1]);
+    }
+
     /// Test a `Segment` which uses the whole recording.
     /// This takes a fast path which skips scanning the index in `new()`.
     #[test]
diff --git a/src/slices.rs b/src/slices.rs
index b3d3235..74e8880 100644
--- a/src/slices.rs
+++ b/src/slices.rs
@@ -30,6 +30,7 @@
 
 //! Tools for implementing a `http_entity::Entity` body composed from many "slices".
 
+use error::Error;
 use reffers::ARefs;
 use futures::stream;
 use futures::Stream;
@@ -92,12 +93,16 @@ impl<S> Slices<S> where S: Slice {
         self.slices.reserve(additional)
     }
 
-    /// Appends the given slice.
-    pub fn append(&mut self, slice: S) {
-        assert!(slice.end() > self.len, "end {} <= len {} while adding slice {:?} to slices:\n{:?}",
-                slice.end(), self.len, slice, self);
+    /// Appends the given slice, which must have end > the Slice's current len.
+    pub fn append(&mut self, slice: S) -> Result<(), Error> {
+        if slice.end() <= self.len {
+            return Err(Error::new(
+                    format!("end {} <= len {} while adding slice {:?} to slices:\n{:?}",
+                            slice.end(), self.len, slice, self)));
+        }
         self.len = slice.end();
         self.slices.push(slice);
+        Ok(())
     }
 
     /// Returns the total byte length of all slices.
@@ -182,11 +187,11 @@ mod tests {
     lazy_static! {
         static ref SLICES: Slices<FakeSlice> = {
             let mut s = Slices::new();
-            s.append(FakeSlice{end: 5, name: "a"});
-            s.append(FakeSlice{end: 5+13, name: "b"});
-            s.append(FakeSlice{end: 5+13+7, name: "c"});
-            s.append(FakeSlice{end: 5+13+7+17, name: "d"});
-            s.append(FakeSlice{end: 5+13+7+17+19, name: "e"});
+            s.append(FakeSlice{end: 5, name: "a"}).unwrap();
+            s.append(FakeSlice{end: 5+13, name: "b"}).unwrap();
+            s.append(FakeSlice{end: 5+13+7, name: "c"}).unwrap();
+            s.append(FakeSlice{end: 5+13+7+17, name: "d"}).unwrap();
+            s.append(FakeSlice{end: 5+13+7+17+19, name: "e"}).unwrap();
             s
         };
     }