From 97cc07352bdd1999691c61479c5e80e24033ac4d Mon Sep 17 00:00:00 2001
From: Justin Bogner <mail@justinbogner.com>
Date: Mon, 2 Mar 2015 17:26:43 +0000
Subject: [PATCH] Detect malformed YAML sequence in
 yaml::Input::beginSequence()

When reading a yaml::SequenceTraits object, YAMLIO does not report an
error if the yaml item is not a sequence. Instead, YAMLIO reads an
empty sequence. For example:

---
seq:
    foo: 1
    bar: 2
...

If `seq` is a SequenceTraits object, then reading the above yaml will
yield `seq` as an empty sequence.

Fix this to report an error for the above mapping ("not a sequence")

Patch by William Fisher. Thanks!

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@230976 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Support/YAMLTraits.cpp       | 17 ++++---
 unittests/Support/YAMLIOTest.cpp | 86 ++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/lib/Support/YAMLTraits.cpp b/lib/Support/YAMLTraits.cpp
index 43a0e102d7e..d888e698937 100644
--- a/lib/Support/YAMLTraits.cpp
+++ b/lib/Support/YAMLTraits.cpp
@@ -168,9 +168,17 @@ void Input::endMapping() {
 }
 
 unsigned Input::beginSequence() {
-  if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
+  if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode))
     return SQ->Entries.size();
+  if (isa<EmptyHNode>(CurrentNode))
+    return 0;
+  // Treat case where there's a scalar "null" value as an empty sequence.
+  if (ScalarHNode *SN = dyn_cast<ScalarHNode>(CurrentNode)) {
+    if (isNull(SN->value()))
+      return 0;
   }
+  // Any other type of HNode is an error.
+  setError(CurrentNode, "not a sequence");
   return 0;
 }
 
@@ -192,12 +200,7 @@ void Input::postflightElement(void *SaveInfo) {
   CurrentNode = reinterpret_cast<HNode *>(SaveInfo);
 }
 
-unsigned Input::beginFlowSequence() {
-  if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
-    return SQ->Entries.size();
-  }
-  return 0;
-}
+unsigned Input::beginFlowSequence() { return beginSequence(); }
 
 bool Input::preflightFlowElement(unsigned index, void *&SaveInfo) {
   if (EC)
diff --git a/unittests/Support/YAMLIOTest.cpp b/unittests/Support/YAMLIOTest.cpp
index 074e27f8318..3104726941e 100644
--- a/unittests/Support/YAMLIOTest.cpp
+++ b/unittests/Support/YAMLIOTest.cpp
@@ -46,6 +46,9 @@ typedef std::vector<FooBar> FooBarSequence;
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(FooBar)
 
+struct FooBarContainer {
+  FooBarSequence fbs;
+};
 
 namespace llvm {
 namespace yaml {
@@ -56,6 +59,12 @@ namespace yaml {
       io.mapRequired("bar",    fb.bar);
     }
   };
+
+  template <> struct MappingTraits<FooBarContainer> {
+    static void mapping(IO &io, FooBarContainer &fb) {
+      io.mapRequired("fbs", fb.fbs);
+    }
+  };
 }
 }
 
@@ -109,6 +118,83 @@ TEST(YAMLIO, TestSequenceMapRead) {
   EXPECT_EQ(map2.bar, 9);
 }
 
+//
+// Test the reading of a map containing a yaml sequence of mappings
+//
+TEST(YAMLIO, TestContainerSequenceMapRead) {
+  {
+    FooBarContainer cont;
+    Input yin2("---\nfbs:\n - foo: 3\n   bar: 5\n - foo: 7\n   bar: 9\n...\n");
+    yin2 >> cont;
+
+    EXPECT_FALSE(yin2.error());
+    EXPECT_EQ(cont.fbs.size(), 2UL);
+    EXPECT_EQ(cont.fbs[0].foo, 3);
+    EXPECT_EQ(cont.fbs[0].bar, 5);
+    EXPECT_EQ(cont.fbs[1].foo, 7);
+    EXPECT_EQ(cont.fbs[1].bar, 9);
+  }
+
+  {
+    FooBarContainer cont;
+    Input yin("---\nfbs:\n...\n");
+    yin >> cont;
+    // Okay: Empty node represents an empty array.
+    EXPECT_FALSE(yin.error());
+    EXPECT_EQ(cont.fbs.size(), 0UL);
+  }
+
+  {
+    FooBarContainer cont;
+    Input yin("---\nfbs: !!null null\n...\n");
+    yin >> cont;
+    // Okay: null represents an empty array.
+    EXPECT_FALSE(yin.error());
+    EXPECT_EQ(cont.fbs.size(), 0UL);
+  }
+
+  {
+    FooBarContainer cont;
+    Input yin("---\nfbs: ~\n...\n");
+    yin >> cont;
+    // Okay: null represents an empty array.
+    EXPECT_FALSE(yin.error());
+    EXPECT_EQ(cont.fbs.size(), 0UL);
+  }
+
+  {
+    FooBarContainer cont;
+    Input yin("---\nfbs: null\n...\n");
+    yin >> cont;
+    // Okay: null represents an empty array.
+    EXPECT_FALSE(yin.error());
+    EXPECT_EQ(cont.fbs.size(), 0UL);
+  }
+}
+
+//
+// Test the reading of a map containing a malformed yaml sequence
+//
+TEST(YAMLIO, TestMalformedContainerSequenceMapRead) {
+  {
+    FooBarContainer cont;
+    Input yin("---\nfbs:\n   foo: 3\n   bar: 5\n...\n", nullptr,
+              suppressErrorMessages);
+    yin >> cont;
+    // Error: fbs is not a sequence.
+    EXPECT_TRUE(!!yin.error());
+    EXPECT_EQ(cont.fbs.size(), 0UL);
+  }
+
+  {
+    FooBarContainer cont;
+    Input yin("---\nfbs: 'scalar'\n...\n", nullptr, suppressErrorMessages);
+    yin >> cont;
+    // This should be an error.
+    EXPECT_TRUE(!!yin.error());
+    EXPECT_EQ(cont.fbs.size(), 0UL);
+  }
+}
 
 //
 // Test writing then reading back a sequence of mappings
-- 
2.34.1