From 21b8e0b6df5d599fab6d9d7def4ab557ae2d5250 Mon Sep 17 00:00:00 2001
From: Scott Lamb <slamb@slamb.org>
Date: Fri, 6 Jan 2017 20:48:06 -0800
Subject: [PATCH] gracefully handle bad video_indexes during upgrade

Dolf reported hitting this problem:

$ sudo -u moonfire-nvr RUST_LOG=info RUST_BACKTRACE=1 release/moonfire-nvr --upgrade
Jan 06 17:10:57.148 INFO Upgrading database from version 0 to version 1...
Jan 06 17:10:57.149 INFO ...database now in journal_mode delete (requested delete).
Jan 06 17:10:57.149 INFO ...from version 0 to version 1
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { description: "zero duration only allowed at end; have 3123 bytes left", cause: None }', /buildslave/rust-buildbot/slave/stable-dist-rustc-cross-host-linux/build/src/libcore/result.rs:837

The indexes were being scanned on upgrade to set the trailing zero flag which
is some sanity checking for /view.mp4 URLs. It's not a big problem to skip it
for some funny recordings to let the update proceed.

Separately, I'll add validation of the pts when writing a recording; it will
report error and end the recording (retrying a second later) rather than write
an unplayable database enty.

Probably also a good time to add a --check to spot database problems such as
this and recording rows without a matching sample file or vice versa.
---
 src/upgrade/v0_to_v1.rs | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/upgrade/v0_to_v1.rs b/src/upgrade/v0_to_v1.rs
index cc27119..e94773e 100644
--- a/src/upgrade/v0_to_v1.rs
+++ b/src/upgrade/v0_to_v1.rs
@@ -35,6 +35,7 @@ use error::Error;
 use recording;
 use rusqlite;
 use std::collections::HashMap;
+use strutil;
 
 pub fn run(tx: &rusqlite::Transaction) -> Result<(), Error> {
     // These create statements match the schema.sql when version 1 was the latest.
@@ -101,6 +102,12 @@ struct CameraState {
     next_recording_id: i32,
 }
 
+fn has_trailing_zero(video_index: &[u8]) -> Result<bool, Error> {
+    let mut it = recording::SampleIndexIterator::new();
+    while it.next(video_index)? {}
+    Ok(it.duration_90k == 0)
+}
+
 /// Fills the `recording` and `recording_playback` tables from `old_recording`, returning
 /// the `camera_state` map for use by a following call to `fill_cameras`.
 fn fill_recording(tx: &rusqlite::Transaction) -> Result<HashMap<i32, CameraState>, Error> {
@@ -116,7 +123,8 @@ fn fill_recording(tx: &rusqlite::Transaction) -> Result<HashMap<i32, CameraState
         video_sample_entry_id,
         sample_file_uuid,
         sample_file_sha1,
-        video_index
+        video_index,
+        id
       from
         old_recording
     "#)?;
@@ -153,11 +161,13 @@ fn fill_recording(tx: &rusqlite::Transaction) -> Result<HashMap<i32, CameraState
         let sample_file_uuid: Vec<u8> = row.get_checked(8)?;
         let sample_file_sha1: Vec<u8> = row.get_checked(9)?;
         let video_index: Vec<u8> = row.get_checked(10)?;
-        let trailing_zero = {
-            let mut it = recording::SampleIndexIterator::new();
-            while it.next(&video_index)? {}
-            it.duration_90k == 0
-        };
+        let old_id: i32 = row.get_checked(11)?;
+        let trailing_zero = has_trailing_zero(&video_index).unwrap_or_else(|e| {
+            warn!("recording {}/{} (sample file {}, formerly recording {}) has corrupt \
+                  video_index: {}",
+                  camera_id, composite_id & 0xFFFF, strutil::hex(&sample_file_uuid), old_id, e);
+            false
+        });
         let run_id = match camera_state.current_run {
             Some((run_id, expected_start)) if expected_start == start_time_90k => run_id,
             _ => composite_id,