Fix undefined behavior due to unassigned action

This wasn't detected earlier because the default Callback implementation always
assigns action (except for in the demo app, which is how this issue was
detected). This assigns a default action of kRead to all actions. The added unit
tests override the callback methods to not assign an action. I verified with
msan that this does indeed trigger a read of uninitialized memory, which I also
verified is then fixed by this change.

Change-Id: Ifd79721aefeb797a40b4f42e1b16b831f508be0a
diff --git a/webm_parser/demo/demo.cc b/webm_parser/demo/demo.cc
index 67ac081..554d5cb 100644
--- a/webm_parser/demo/demo.cc
+++ b/webm_parser/demo/demo.cc
@@ -1005,9 +1005,10 @@
   }
 
   Status OnBlockGroupBegin(const ElementMetadata& metadata,
-                           Action* /* action */) override {
+                           Action* action) override {
     indent = 2;
     PrintElementMetadata("BlockGroup", metadata);
+    *action = Action::kRead;
     return Status(Status::kOkCompleted);
   }
 
diff --git a/webm_parser/src/block_group_parser.h b/webm_parser/src/block_group_parser.h
index c9727bb..feca6a4 100644
--- a/webm_parser/src/block_group_parser.h
+++ b/webm_parser/src/block_group_parser.h
@@ -44,7 +44,7 @@
     *num_bytes_read = 0;
 
     if (!parse_started_event_completed()) {
-      Action action;
+      Action action = Action::kRead;
       Status status = OnParseStarted(callback, &action);
       if (!status.completed_ok()) {
         return status;
diff --git a/webm_parser/src/block_parser.cc b/webm_parser/src/block_parser.cc
index 86981e4..b851f51 100644
--- a/webm_parser/src/block_parser.cc
+++ b/webm_parser/src/block_parser.cc
@@ -128,7 +128,7 @@
       }
 
       case State::kGettingAction: {
-        Action action;
+        Action action = Action::kRead;
         status = BasicBlockBegin(frame_metadata_.parent_element, value_,
                                  callback, &action);
         if (!status.completed_ok()) {
diff --git a/webm_parser/src/master_parser.h b/webm_parser/src/master_parser.h
index 4d4fa37..2000167 100644
--- a/webm_parser/src/master_parser.h
+++ b/webm_parser/src/master_parser.h
@@ -184,7 +184,7 @@
   ElementParser* child_parser_;
 
   // The current parsing action for the child that is currently being parsed.
-  Action action_;
+  Action action_ = Action::kRead;
 
   // The current state of the parser.
   State state_;
diff --git a/webm_parser/src/master_value_parser.h b/webm_parser/src/master_value_parser.h
index 1a1d8d9..2a02a3f 100644
--- a/webm_parser/src/master_value_parser.h
+++ b/webm_parser/src/master_value_parser.h
@@ -393,7 +393,7 @@
 
  private:
   T value_;
-  Action action_;
+  Action action_ = Action::kRead;
   bool parse_complete_;
   bool started_done_;
   // master_parser_ must be after value_ to ensure correct initialization order.
diff --git a/webm_parser/src/segment_parser.h b/webm_parser/src/segment_parser.h
index 68c4ae9..b5bb702 100644
--- a/webm_parser/src/segment_parser.h
+++ b/webm_parser/src/segment_parser.h
@@ -45,7 +45,7 @@
   bool parse_completed_;
 
   // The action requested by Callback::OnSegmentBegin.
-  Action action_;
+  Action action_ = Action::kRead;
 };
 
 }  // namespace webm
diff --git a/webm_parser/test_utils/parser_test.h b/webm_parser/test_utils/parser_test.h
index 73399cd..61d35b2 100644
--- a/webm_parser/test_utils/parser_test.h
+++ b/webm_parser/test_utils/parser_test.h
@@ -12,6 +12,7 @@
 #include <new>
 #include <vector>
 
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #include "test_utils/limited_reader.h"
@@ -96,7 +97,7 @@
   T parser_;
 
   // The callback that is used during parsing.
-  MockCallback callback_;
+  testing::NiceMock<MockCallback> callback_;
 
   // The reader used for feeding data into the parser.
   BufferReader reader_;
diff --git a/webm_parser/tests/block_group_parser_test.cc b/webm_parser/tests/block_group_parser_test.cc
index 5e74969..d5b0088 100644
--- a/webm_parser/tests/block_group_parser_test.cc
+++ b/webm_parser/tests/block_group_parser_test.cc
@@ -17,6 +17,7 @@
 using testing::_;
 using testing::InSequence;
 using testing::NotNull;
+using testing::Return;
 
 using webm::Block;
 using webm::BlockAdditions;
@@ -70,6 +71,20 @@
   ParseAndVerify();
 }
 
+TEST_F(BlockGroupParserTest, DefaultActionIsRead) {
+  {
+    InSequence dummy;
+
+    // This intentionally does not set the action and relies on the parser using
+    // a default action value of kRead.
+    EXPECT_CALL(callback_, OnBlockGroupBegin(metadata_, NotNull()))
+        .WillOnce(Return(Status(Status::kOkCompleted)));
+    EXPECT_CALL(callback_, OnBlockGroupEnd(metadata_, BlockGroup{})).Times(1);
+  }
+
+  ParseAndVerify();
+}
+
 TEST_F(BlockGroupParserTest, DefaultValues) {
   SetReaderData({
       0xA1,  // ID = 0xA1 (Block).
diff --git a/webm_parser/tests/block_parser_test.cc b/webm_parser/tests/block_parser_test.cc
index 9768b68..3128117 100644
--- a/webm_parser/tests/block_parser_test.cc
+++ b/webm_parser/tests/block_parser_test.cc
@@ -529,7 +529,8 @@
 class BasicBlockParserTest : public ElementParserTest<T, id> {
  public:
   // Sets expectations for a normal (i.e. successful parse) test.
-  void SetExpectations(const TestData& test_data, bool incremental) {
+  void SetExpectations(const TestData& test_data, bool incremental,
+                       bool set_action) {
     InSequence dummy;
 
     const SimpleBlock expected_simple_block = ExpectedSimpleBlock(test_data);
@@ -537,15 +538,24 @@
 
     FrameMetadata metadata = FirstFrameMetadata(test_data);
     if (std::is_same<T, SimpleBlockParser>::value) {
-      EXPECT_CALL(callback_,
-                  OnSimpleBlockBegin(metadata.parent_element,
-                                     expected_simple_block, NotNull()))
-          .Times(1);
+      auto& expectation = EXPECT_CALL(
+          callback_, OnSimpleBlockBegin(metadata.parent_element,
+                                        expected_simple_block, NotNull()));
+      if (set_action) {
+        expectation.Times(1);
+      } else {
+        expectation.WillOnce(Return(Status(Status::kOkCompleted)));
+      }
       EXPECT_CALL(callback_, OnBlockBegin(_, _, _)).Times(0);
     } else {
-      EXPECT_CALL(callback_, OnBlockBegin(metadata.parent_element,
-                                          expected_block, NotNull()))
-          .Times(1);
+      auto& expectation = EXPECT_CALL(
+          callback_,
+          OnBlockBegin(metadata.parent_element, expected_block, NotNull()));
+      if (set_action) {
+        expectation.Times(1);
+      } else {
+        expectation.WillOnce(Return(Status(Status::kOkCompleted)));
+      }
       EXPECT_CALL(callback_, OnSimpleBlockBegin(_, _, _)).Times(0);
     }
 
@@ -583,7 +593,7 @@
   // Runs a single test using the provided test data.
   void RunTest(const TestData& test_data) {
     SetReaderData(test_data.data);
-    SetExpectations(test_data, false);
+    SetExpectations(test_data, false, true);
 
     ParseAndVerify();
 
@@ -593,7 +603,7 @@
   // Same as RunTest(), except it forces parsers to parse one byte at a time.
   void RunIncrementalTest(const TestData& test_data) {
     SetReaderData(test_data.data);
-    SetExpectations(test_data, true);
+    SetExpectations(test_data, true, true);
 
     IncrementalParseAndVerify();
 
@@ -753,6 +763,13 @@
   RunIncrementalTest(fixed_lacing);
 }
 
+TEST_F(BlockParserTest, DefaultActionIsRead) {
+  SetReaderData(fixed_lacing_one_frame.data);
+  SetExpectations(fixed_lacing_one_frame, false, false);
+  ParseAndVerify();
+  ValidateBlock(fixed_lacing_one_frame, parser_.value());
+}
+
 TEST_F(BlockParserTest, IncrementalNoLacing) { RunIncrementalTest(no_lacing); }
 
 class SimpleBlockParserTest
@@ -838,4 +855,11 @@
   RunIncrementalTest(no_lacing);
 }
 
+TEST_F(SimpleBlockParserTest, DefaultActionIsRead) {
+  SetReaderData(fixed_lacing_one_frame.data);
+  SetExpectations(fixed_lacing_one_frame, false, false);
+  ParseAndVerify();
+  ValidateBlock(fixed_lacing_one_frame, parser_.value());
+}
+
 }  // namespace
diff --git a/webm_parser/tests/cluster_parser_test.cc b/webm_parser/tests/cluster_parser_test.cc
index 6d2fd40..fb879b6 100644
--- a/webm_parser/tests/cluster_parser_test.cc
+++ b/webm_parser/tests/cluster_parser_test.cc
@@ -50,6 +50,20 @@
   ParseAndVerify();
 }
 
+TEST_F(ClusterParserTest, DefaultActionIsRead) {
+  {
+    InSequence dummy;
+
+    // This intentionally does not set the action and relies on the parser using
+    // a default action value of kRead.
+    EXPECT_CALL(callback_, OnClusterBegin(metadata_, Cluster{}, NotNull()))
+        .WillOnce(Return(Status(Status::kOkCompleted)));
+    EXPECT_CALL(callback_, OnClusterEnd(metadata_, Cluster{})).Times(1);
+  }
+
+  ParseAndVerify();
+}
+
 TEST_F(ClusterParserTest, DefaultValues) {
   SetReaderData({
       0xE7,  // ID = 0xE7 (Timecode).
diff --git a/webm_parser/tests/master_parser_test.cc b/webm_parser/tests/master_parser_test.cc
index 84edb27..5f36fd2 100644
--- a/webm_parser/tests/master_parser_test.cc
+++ b/webm_parser/tests/master_parser_test.cc
@@ -163,6 +163,27 @@
   ParseAndVerify();
 }
 
+TEST_F(MasterParserTest, DefaultActionIsRead) {
+  SetReaderData({
+      0xEC,  // ID = 0xEC (Void).
+      0x80,  // Size = 0.
+  });
+
+  {
+    InSequence dummy;
+
+    const ElementMetadata metadata = {Id::kVoid, 2, 0, 0};
+
+    // This intentionally does not set the action and relies on the parser using
+    // a default action value of kRead.
+    EXPECT_CALL(callback_, OnElementBegin(metadata, NotNull()))
+        .WillOnce(Return(Status(Status::kOkCompleted)));
+    EXPECT_CALL(callback_, OnVoid(metadata, NotNull(), NotNull())).Times(1);
+  }
+
+  ParseAndVerify();
+}
+
 // Unrecognized children should be skipped over.
 TEST_F(MasterParserTest, UnknownChildren) {
   SetReaderData({
diff --git a/webm_parser/tests/segment_parser_test.cc b/webm_parser/tests/segment_parser_test.cc
index cb1cc08..d1a444b 100644
--- a/webm_parser/tests/segment_parser_test.cc
+++ b/webm_parser/tests/segment_parser_test.cc
@@ -47,6 +47,20 @@
   ParseAndVerify();
 }
 
+TEST_F(SegmentParserTest, DefaultActionIsRead) {
+  {
+    InSequence dummy;
+
+    // This intentionally does not set the action and relies on the parser using
+    // a default action value of kRead.
+    EXPECT_CALL(callback_, OnSegmentBegin(metadata_, NotNull()))
+        .WillOnce(Return(Status(Status::kOkCompleted)));
+    EXPECT_CALL(callback_, OnSegmentEnd(metadata_)).Times(1);
+  }
+
+  ParseAndVerify();
+}
+
 TEST_F(SegmentParserTest, EmptyParseWithDelayedStart) {
   {
     InSequence dummy;
diff --git a/webm_parser/tests/webm_parser_test.cc b/webm_parser/tests/webm_parser_test.cc
index 67d8667..26abbaf 100644
--- a/webm_parser/tests/webm_parser_test.cc
+++ b/webm_parser/tests/webm_parser_test.cc
@@ -106,6 +106,31 @@
   EXPECT_EQ(Status::kOkCompleted, status.code);
 }
 
+TEST_F(WebmParserTest, DefaultActionIsRead) {
+  BufferReader reader = {
+      0x1A, 0x45, 0xDF, 0xA3,  // ID = 0x1A45DFA3 (EBML).
+      0x80,  // Size = 0.
+  };
+
+  MockCallback callback;
+  {
+    InSequence dummy;
+
+    const ElementMetadata metadata = {Id::kEbml, 5, 0, 0};
+    const Ebml ebml{};
+
+    // This intentionally does not set the action and relies on the parser using
+    // a default action value of kRead.
+    EXPECT_CALL(callback, OnElementBegin(metadata, NotNull()))
+        .WillOnce(Return(Status(Status::kOkCompleted)));
+    EXPECT_CALL(callback, OnEbml(metadata, ebml)).Times(1);
+  }
+
+  WebmParser parser;
+  Status status = parser.Feed(&callback, &reader);
+  EXPECT_EQ(Status::kOkCompleted, status.code);
+}
+
 TEST_F(WebmParserTest, UnknownElement) {
   BufferReader reader = {
       0x80,  // ID = 0x80.