Fix two packager crash bug with invalid arguments

Fix b/18005827 Packager crashes in debug mode with widevine decryption
enabled given a non-existing file.

Fix b/18005632 Packager crashes with invalid fixed_key_encryption
parameters.

Also cleans up some code.

Change-Id: I097f5c8dc113eb6d33b42b19a4bf5de125c47c30

packager/app/packager_util.cc

@@ -4,6 +4,8 @@
 // license that can be found in the LICENSE file or at
 // https://developers.google.com/open-source/licenses/bsd
 
+#include "packager/app/packager_util.h"
+
 #include <gflags/gflags.h>
 #include <iostream>
 

packager/app/validate_flag.h

@@ -6,6 +6,9 @@
 //
 // Flag validation help functions.
 
+#ifndef APP_VALIDATE_FLAG_H_
+#define APP_VALIDATE_FLAG_H_
+
 #include <string>
 
 namespace edash_packager {
@@ -31,3 +34,5 @@ bool ValidateFlag(const char* flag_name,
 void PrintError(const std::string& error_message);
 
 }  // namespace edash_packager
+
+#endif  // APP_VALIDATE_FLAG_H_

packager/app/widevine_encryption_flags.cc

@@ -142,7 +142,8 @@ bool ValidateWidevineCryptoFlags() {
   if (FLAGS_crypto_period_duration < 0) {
     PrintError("--crypto_period_duration should not be negative.");
     success = false;
-  } else if (FLAGS_crypto_period_duration > 0 && !FLAGS_enable_widevine_encryption) {
+  } else if (FLAGS_crypto_period_duration > 0 &&
+             !FLAGS_enable_widevine_encryption) {
     PrintError(
         "--crypto_period_duration should be specified only if "
         "--enable_widevine_encryption.");

packager/media/base/widevine_key_source.cc

@@ -146,6 +146,7 @@ WidevineKeySource::WidevineKeySource(const std::string& server_url)
       key_production_started_(false),
       start_key_production_(false, false),
       first_crypto_period_index_(0) {
+  key_production_thread_.Start();
 }
 
 WidevineKeySource::~WidevineKeySource() {
@@ -168,7 +169,7 @@ Status WidevineKeySource::FetchKeys(const std::vector<uint8_t>& content_id,
   BytesToBase64String(content_id, &content_id_base64_string);
   request_dict_.SetString("content_id", content_id_base64_string);
   request_dict_.SetString("policy", policy);
-  return FetchKeysCommon(false);
+  return FetchKeysInternal(!kEnableKeyRotation, 0, false);
 }
 
 Status WidevineKeySource::FetchKeys(const std::vector<uint8_t>& pssh_data) {
@@ -177,29 +178,17 @@ Status WidevineKeySource::FetchKeys(const std::vector<uint8_t>& pssh_data) {
   std::string pssh_data_base64_string;
   BytesToBase64String(pssh_data, &pssh_data_base64_string);
   request_dict_.SetString("pssh_data", pssh_data_base64_string);
-  return FetchKeysCommon(false);
+  return FetchKeysInternal(!kEnableKeyRotation, 0, false);
 }
 
 Status WidevineKeySource::FetchKeys(uint32_t asset_id) {
   base::AutoLock scoped_lock(lock_);
   request_dict_.Clear();
   request_dict_.SetInteger("asset_id", asset_id);
-  return FetchKeysCommon(true);
+  return FetchKeysInternal(!kEnableKeyRotation, 0, true);
 }
 
-Status WidevineKeySource::FetchKeysCommon(bool widevine_classic) {
+Status WidevineKeySource::GetKey(TrackType track_type, EncryptionKey* key) {
-  // TODO(tinskip): Make this method callable multiple times to fetch
-  // different keys.
-  DCHECK(!key_production_thread_.HasBeenStarted());
-  key_production_thread_.Start();
-
-  // Perform a fetch request to find out if the key source is healthy.
-  // It also stores the keys fetched for consumption later.
-  return FetchKeysInternal(!kEnableKeyRotation, 0, widevine_classic);
-}
-
-Status WidevineKeySource::GetKey(TrackType track_type,
-                                 EncryptionKey* key) {
   DCHECK(key);
   if (encryption_key_map_.find(track_type) == encryption_key_map_.end()) {
     return Status(error::INTERNAL_ERROR,
@@ -235,7 +224,8 @@ Status WidevineKeySource::GetCryptoPeriodKey(uint32_t crypto_period_index,
     if (!key_production_started_) {
       // Another client may have a slightly smaller starting crypto period
       // index. Set the initial value to account for that.
-      first_crypto_period_index_ = crypto_period_index ? crypto_period_index - 1 : 0;
+      first_crypto_period_index_ =
+          crypto_period_index ? crypto_period_index - 1 : 0;
       DCHECK(!key_pool_);
       key_pool_.reset(new EncryptionKeyQueue(crypto_period_count_,
                                              first_crypto_period_index_));

packager/media/base/widevine_key_source.h

@@ -26,7 +26,7 @@ template <class T> class ProducerConsumerQueue;
 class WidevineKeySource : public KeySource {
  public:
   /// @param server_url is the Widevine common encryption server url.
-  WidevineKeySource(const std::string& server_url);
+  explicit WidevineKeySource(const std::string& server_url);
 
   virtual ~WidevineKeySource();
 
@@ -67,9 +67,6 @@ class WidevineKeySource : public KeySource {
                         TrackType track_type,
                         EncryptionKey* key);
 
-  // Common implementation of FetchKeys methods above.
-  Status FetchKeysCommon(bool widevine_classic);
-
   // The closure task to fetch keys repeatedly.
   void FetchKeysTask();
 

packager/media/formats/mp4/box_definitions.cc

@@ -18,6 +18,9 @@ const uint32_t kBoxSize = kFourCCSize + sizeof(uint32_t);
 // Additional 1-byte version and 3-byte flags.
 const uint32_t kFullBoxSize = kBoxSize + 4;
 
+// Key Id size as defined in CENC spec.
+const uint32_t kCencKeyIdSize = 16;
+
 // 9 uint32_t in big endian formatted array.
 const uint8_t kUnityMatrix[] = {0, 1, 0, 0, 0, 0, 0, 0, 0,    0, 0, 0,
                                 0, 0, 0, 0, 0, 1, 0, 0, 0,    0, 0, 0,
@@ -226,12 +229,21 @@ TrackEncryption::~TrackEncryption() {}
 FourCC TrackEncryption::BoxType() const { return FOURCC_TENC; }
 
 bool TrackEncryption::ReadWrite(BoxBuffer* buffer) {
+  if (!buffer->Reading()) {
+    if (default_kid.size() != kCencKeyIdSize) {
+      LOG(WARNING) << "CENC defines key id length of " << kCencKeyIdSize
+                   << " bytes; got " << default_kid.size()
+                   << ". Resized accordingly.";
+      default_kid.resize(kCencKeyIdSize);
+    }
+  }
+
   uint8_t flag = is_encrypted ? 1 : 0;
   RCHECK(FullBox::ReadWrite(buffer) &&
          buffer->IgnoreBytes(2) &&  // reserved.
          buffer->ReadWriteUInt8(&flag) &&
          buffer->ReadWriteUInt8(&default_iv_size) &&
-         buffer->ReadWriteVector(&default_kid, 16));
+         buffer->ReadWriteVector(&default_kid, kCencKeyIdSize));
   if (buffer->Reading()) {
     is_encrypted = (flag != 0);
     if (is_encrypted) {
@@ -244,7 +256,7 @@ bool TrackEncryption::ReadWrite(BoxBuffer* buffer) {
 }
 
 uint32_t TrackEncryption::ComputeSize() {
-  atom_size = kFullBoxSize + sizeof(uint32_t) + default_kid.size();
+  atom_size = kFullBoxSize + sizeof(uint32_t) + kCencKeyIdSize;
   return atom_size;
 }
 
@@ -1674,8 +1686,7 @@ bool SampleGroupDescription::ReadWrite(BoxBuffer* buffer) {
     return true;
   }
 
-  const size_t kKeyIdSize = 16;
+  const size_t kEntrySize = sizeof(uint32_t) + kCencKeyIdSize;
-  const size_t kEntrySize = sizeof(uint32_t) + kKeyIdSize;
   uint32_t default_length = 0;
   if (version == 1) {
     if (buffer->Reading()) {
@@ -1699,14 +1710,20 @@ bool SampleGroupDescription::ReadWrite(BoxBuffer* buffer) {
       }
     }
 
-    if (!buffer->Reading())
+    if (!buffer->Reading()) {
-      RCHECK(entries[i].key_id.size() == kKeyIdSize);
+      if (entries[i].key_id.size() != kCencKeyIdSize) {
+        LOG(WARNING) << "CENC defines key id length of " << kCencKeyIdSize
+                     << " bytes; got " << entries[i].key_id.size()
+                     << ". Resized accordingly.";
+        entries[i].key_id.resize(kCencKeyIdSize);
+      }
+    }
 
     uint8_t flag = entries[i].is_encrypted ? 1 : 0;
     RCHECK(buffer->IgnoreBytes(2) &&  // reserved.
            buffer->ReadWriteUInt8(&flag) &&
            buffer->ReadWriteUInt8(&entries[i].iv_size) &&
-           buffer->ReadWriteVector(&entries[i].key_id, kKeyIdSize));
+           buffer->ReadWriteVector(&entries[i].key_id, kCencKeyIdSize));
 
     if (buffer->Reading()) {
       entries[i].is_encrypted = (flag != 0);
@@ -1726,8 +1743,7 @@ uint32_t SampleGroupDescription::ComputeSize() {
   // This box is optional. Skip it if it is not used.
   atom_size = 0;
   if (!entries.empty()) {
-    const size_t kKeyIdSize = 16;
+    const size_t kEntrySize = sizeof(uint32_t) + kCencKeyIdSize;
-    const size_t kEntrySize = sizeof(uint32_t) + kKeyIdSize;
     atom_size = kFullBoxSize + sizeof(grouping_type) +
                 (version == 1 ? sizeof(uint32_t) : 0) + sizeof(uint32_t) +
                 entries.size() * kEntrySize;

packager/media/formats/mp4/mp4_media_parser.cc

@@ -428,7 +428,11 @@ bool MP4MediaParser::EnqueueSample(bool* err) {
            << ", cts=" << runs_->cts()
            << ", size=" << runs_->sample_size();
 
-  new_sample_cb_.Run(runs_->track_id(), stream_sample);
+  if (!new_sample_cb_.Run(runs_->track_id(), stream_sample)) {
+    *err = true;
+    LOG(ERROR) << "Failed to process the sample.";
+    return false;
+  }
 
   runs_->AdvanceSample();
   return true;

packager/media/formats/wvm/wvm_media_parser.cc

@@ -488,25 +488,25 @@ bool WvmMediaParser::EmitLastSample(uint32_t stream_id,
                         .append(base::UintToString(stream_id));
   std::map<std::string, uint32_t>::iterator it =
       program_demux_stream_map_.find(key);
-  if (it != program_demux_stream_map_.end()) {
+  if (it == program_demux_stream_map_.end())
-      EmitSample(stream_id, (*it).second, new_sample, true);
-  } else {
     return false;
-  }
+  return EmitSample(stream_id, (*it).second, new_sample, true);
-  return true;
 }
 
-void WvmMediaParser::EmitPendingSamples() {
+bool WvmMediaParser::EmitPendingSamples() {
   // Emit queued samples which were built when not initialized.
   while (!media_sample_queue_.empty()) {
     DemuxStreamIdMediaSample& demux_stream_media_sample =
         media_sample_queue_.front();
-    EmitSample(
+    if (!EmitSample(demux_stream_media_sample.parsed_audio_or_video_stream_id,
-        demux_stream_media_sample.parsed_audio_or_video_stream_id,
+                    demux_stream_media_sample.demux_stream_id,
-        demux_stream_media_sample.demux_stream_id,
+                    demux_stream_media_sample.media_sample,
-        demux_stream_media_sample.media_sample, false);
+                    false)) {
+      return false;
+    }
     media_sample_queue_.pop_front();
   }
+  return true;
 }
 
 void WvmMediaParser::Flush() {
@@ -855,64 +855,76 @@ bool WvmMediaParser::Output() {
   } else {
     // flush the sample queue and emit all queued samples.
     while (!media_sample_queue_.empty()) {
-      EmitPendingSamples();
+      if (!EmitPendingSamples())
+        return false;
     }
     // Emit current sample.
-    EmitSample(prev_pes_stream_id_, (*it).second, media_sample_, false);
+    if (!EmitSample(prev_pes_stream_id_, (*it).second, media_sample_, false))
+      return false;
   }
   return true;
 }
 
-void WvmMediaParser::EmitSample(uint32_t parsed_audio_or_video_stream_id,
+bool WvmMediaParser::EmitSample(uint32_t parsed_audio_or_video_stream_id,
                                 uint32_t stream_id,
                                 scoped_refptr<MediaSample>& new_sample,
                                 bool isLastSample) {
   DCHECK(new_sample);
   if (isLastSample) {
-    if ((parsed_audio_or_video_stream_id & kPesStreamIdVideoMask)
+    if ((parsed_audio_or_video_stream_id & kPesStreamIdVideoMask) ==
-        == kPesStreamIdVideo) {
+        kPesStreamIdVideo) {
       new_sample->set_duration(prev_media_sample_data_.video_sample_duration);
-    } else if ((parsed_audio_or_video_stream_id & kPesStreamIdAudioMask)
+    } else if ((parsed_audio_or_video_stream_id & kPesStreamIdAudioMask) ==
-        == kPesStreamIdAudio) {
+               kPesStreamIdAudio) {
       new_sample->set_duration(prev_media_sample_data_.audio_sample_duration);
     }
-    new_sample_cb_.Run(stream_id, new_sample);
+    if (!new_sample_cb_.Run(stream_id, new_sample)) {
-    return;
+      LOG(ERROR) << "Failed to process the last sample.";
+      return false;
+    }
+    return true;
   }
 
   // Cannot emit current sample.  Compute duration first and then,
   // emit previous sample.
-  if ((parsed_audio_or_video_stream_id & kPesStreamIdVideoMask)
+  if ((parsed_audio_or_video_stream_id & kPesStreamIdVideoMask) ==
-      == kPesStreamIdVideo) {
+      kPesStreamIdVideo) {
     if (prev_media_sample_data_.video_sample == NULL) {
       prev_media_sample_data_.video_sample = new_sample;
       prev_media_sample_data_.video_stream_id = stream_id;
-      return;
+      return true;
     }
     prev_media_sample_data_.video_sample->set_duration(
         new_sample->dts() - prev_media_sample_data_.video_sample->dts());
     prev_media_sample_data_.video_sample_duration =
         prev_media_sample_data_.video_sample->duration();
-    new_sample_cb_.Run(prev_media_sample_data_.video_stream_id,
+    if (!new_sample_cb_.Run(prev_media_sample_data_.video_stream_id,
-                       prev_media_sample_data_.video_sample);
+                            prev_media_sample_data_.video_sample)) {
+      LOG(ERROR) << "Failed to process the video sample.";
+      return false;
+    }
     prev_media_sample_data_.video_sample = new_sample;
     prev_media_sample_data_.video_stream_id = stream_id;
-  } else if ((parsed_audio_or_video_stream_id & kPesStreamIdAudioMask)
+  } else if ((parsed_audio_or_video_stream_id & kPesStreamIdAudioMask) ==
-      == kPesStreamIdAudio) {
+             kPesStreamIdAudio) {
     if (prev_media_sample_data_.audio_sample == NULL) {
       prev_media_sample_data_.audio_sample = new_sample;
       prev_media_sample_data_.audio_stream_id = stream_id;
-      return;
+      return true;
     }
     prev_media_sample_data_.audio_sample->set_duration(
         new_sample->dts() - prev_media_sample_data_.audio_sample->dts());
     prev_media_sample_data_.audio_sample_duration =
         prev_media_sample_data_.audio_sample->duration();
-    new_sample_cb_.Run(prev_media_sample_data_.audio_stream_id,
+    if (!new_sample_cb_.Run(prev_media_sample_data_.audio_stream_id,
-                       prev_media_sample_data_.audio_sample);
+                            prev_media_sample_data_.audio_sample)) {
+      LOG(ERROR) << "Failed to process the audio sample.";
+      return false;
+    }
     prev_media_sample_data_.audio_sample = new_sample;
     prev_media_sample_data_.audio_stream_id = stream_id;
   }
+  return true;
 }
 
 bool WvmMediaParser::GetAssetKey(const uint32_t asset_id,

packager/media/formats/wvm/wvm_media_parser.h

@@ -201,12 +201,12 @@ class WvmMediaParser : public MediaParser {
 
   // Callback invoked by the ES media parser
   // to emit a new audio/video access unit.
-  void EmitSample(uint32_t parsed_audio_or_video_stream_id,
+  bool EmitSample(uint32_t parsed_audio_or_video_stream_id,
                   uint32_t stream_id,
                   scoped_refptr<MediaSample>& new_sample,
                   bool isLastSample);
 
-  void EmitPendingSamples();
+  bool EmitPendingSamples();
 
   bool EmitLastSample(uint32_t stream_id,
                       scoped_refptr<MediaSample>& new_sample);