[llvm] r230976 - Detect malformed YAML sequence in yaml::Input::beginSequence()

[Justin Bogner]

Author: bogner
Date: Mon Mar  2 11:26:43 2015
New Revision: 230976

URL: http://llvm.org/viewvc/llvm-project?rev=230976&view=rev
Log:
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!

Modified:
    llvm/trunk/lib/Support/YAMLTraits.cpp
    llvm/trunk/unittests/Support/YAMLIOTest.cpp

Modified: llvm/trunk/lib/Support/YAMLTraits.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/YAMLTraits.cpp?rev=230976&r1=230975&r2=230976&view=diff
==============================================================================
--- llvm/trunk/lib/Support/YAMLTraits.cpp (original)
+++ llvm/trunk/lib/Support/YAMLTraits.cpp Mon Mar  2 11:26:43 2015
@@ -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 *Save
   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)

Modified: llvm/trunk/unittests/Support/YAMLIOTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/YAMLIOTest.cpp?rev=230976&r1=230975&r2=230976&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/YAMLIOTest.cpp (original)
+++ llvm/trunk/unittests/Support/YAMLIOTest.cpp Mon Mar  2 11:26:43 2015
@@ -46,6 +46,9 @@ typedef std::vector<FooBar> FooBarSequen
 
 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