Properly verify that input data is just 'pssh' boxes

media/formats/mp4/box_reader.h

Index: media/formats/mp4/box_reader.h
diff --git a/media/formats/mp4/box_reader.h b/media/formats/mp4/box_reader.h
index 92e85fc44fade4ce118914b1f47d1d363decf549..62f28578284431e8ff9a5c18641f49cda0683935 100644
--- a/media/formats/mp4/box_reader.h
+++ b/media/formats/mp4/box_reader.h
@@ -141,8 +141,18 @@ class MEDIA_EXPORT BoxReader : public BufferReader {
   // Read all children, regardless of FourCC. This is used from exactly one box,
   // corresponding to a rather significant inconsistency in the BMFF spec.
   // Note that this method is mutually exclusive with ScanChildren().

[ddorwin, 2015/07/01 20:22:35]

and ReadAllChildrenAndCheckFourCC()

[jrummell, 2015/07/01 20:57:59]

Done.

-  template<typename T> bool ReadAllChildren(
-      std::vector<T>* children) WARN_UNUSED_RESULT;
+  template <typename T>
+  bool ReadAllChildren(std::vector<T>* children) WARN_UNUSED_RESULT {

[ddorwin, 2015/07/01 20:22:35]

Given the comment about this being called in one place, it seems we should
rename this to discourage use. Maybe in a separate CL.

[jrummell, 2015/07/01 20:57:59]

Acknowledged.

+    return ReadAllChildrenInternal(children, false);
+  }
+
+  // Read all children and verify that the FourCC matches what is expected.
+  // Note that this method is mutually exclusive with ScanChildren().

[ddorwin, 2015/07/01 20:22:35]

and ReadAllChildren()

[jrummell, 2015/07/01 20:57:59]

Done.

+  template <typename T>

[ddorwin, 2015/07/01 20:22:35]

// Returns true if all children are successfully parsed(?) and have the correct
box type for |T|.

[jrummell, 2015/07/01 20:57:59]

Done.

+  bool ReadAllChildrenAndCheckFourCC(std::vector<T>* children)
+      WARN_UNUSED_RESULT {
+    return ReadAllChildrenInternal(children, true);
+  }
 
   // Populate the values of 'version()' and 'flags()' from a full box header.
   // Many boxes, but not all, use these values. This call should happen after
@@ -169,6 +179,11 @@ class MEDIA_EXPORT BoxReader : public BufferReader {
   // true, the error is unrecoverable and the stream should be aborted.
   bool ReadHeader(bool* err);
 
+  // Read all children, optionally checking FourCC. Note that this method is
+  // mutually exclusive with ScanChildren().
+  template <typename T>

[ddorwin, 2015/07/01 20:22:35]

// Returns true if all children are successfully parsed(?) and, if
|check_box_type|,  have the correct box type for |T|.

[jrummell, 2015/07/01 20:57:59]

Done.

+  bool ReadAllChildrenInternal(std::vector<T>* children, bool check_box_type);
+
   LogCB log_cb_;
   FourCC type_;
   uint8 version_;
@@ -214,8 +229,9 @@ bool BoxReader::MaybeReadChildren(std::vector<T>* children) {
   return true;
 }
 
-template<typename T>
-bool BoxReader::ReadAllChildren(std::vector<T>* children) {
+template <typename T>
+bool BoxReader::ReadAllChildrenInternal(std::vector<T>* children,
+                                        bool check_box_type) {
   DCHECK(!scanned_);
   scanned_ = true;
 
@@ -224,6 +240,7 @@ bool BoxReader::ReadAllChildren(std::vector<T>* children) {
     BoxReader child_reader(&buf_[pos_], size_ - pos_, log_cb_, is_EOS_);
     if (!child_reader.ReadHeader(&err)) break;
     T child;
+    RCHECK(!check_box_type || child_reader.type() == child.BoxType());
     RCHECK(child.Parse(&child_reader));
     children->push_back(child);
     pos_ += child_reader.size();