[PATCH] [YAML] Recover gracefully when deserializing invalid YAML input.

[Andrew Tulloch]

tulloch added you to the CC list for the revision "[YAML] Recover gracefully when deserializing invalid YAML input.".

Hi gribozavr, kledzik, rsmith, bkramer,

1. See http://llvm.org/bugs/show_bug.cgi?id=16221 for a user's bug report.
2. We weren't checking for `Document::getRoot()` returning `NULL`. This happens
in the case where the input YAML/JSON object is malformed.
3. `Document::getRoot()` is passed to `isa<NullNode>`, which asserts on it's
argument being `NULL`.
4. To fix this, we check for this return value, and set the error code
accordingly.  We also modify the calling code to check the return value
of `input::setCurrentDocument()`.

The added unit tests hit the assert in trunk, while passing on this branch.

Additionally, the command that reproduces this assert in the bug report:

```
llvm-build/Debug+Asserts/bin/clang-format  -style="{BasedOnStyle:llvm}" tools/clang/tools/clang-format/ClangFormat.cpp
```

now outputs http://llvm-reviews.chandlerc.com/P55

http://llvm-reviews.chandlerc.com/D1236

Files:
  include/llvm/Support/YAMLTraits.h
  lib/Support/YAMLTraits.cpp
  unittests/Support/YAMLIOTest.cpp

Index: include/llvm/Support/YAMLTraits.h
===================================================================
--- include/llvm/Support/YAMLTraits.h
+++ include/llvm/Support/YAMLTraits.h
@@ -956,7 +956,8 @@
 inline
 typename llvm::enable_if_c<has_MappingTraits<T>::value,Input &>::type
 operator>>(Input &yin, T &docMap) {
-  yin.setCurrentDocument();
+  if (!yin.setCurrentDocument())
+    return yin;
   yamlize(yin, docMap, true);
   return yin;
 }
@@ -967,7 +968,8 @@
 inline
 typename llvm::enable_if_c<has_SequenceTraits<T>::value,Input &>::type
 operator>>(Input &yin, T &docSeq) {
-  yin.setCurrentDocument();
+  if (!yin.setCurrentDocument())
+    return yin;
   yamlize(yin, docSeq, true);
   return yin;
 }
Index: lib/Support/YAMLTraits.cpp
===================================================================
--- lib/Support/YAMLTraits.cpp
+++ lib/Support/YAMLTraits.cpp
@@ -66,6 +66,12 @@
 bool Input::setCurrentDocument() {
   if (DocIterator != Strm->end()) {
     Node *N = DocIterator->getRoot();
+    if (!N) {
+      assert(Strm->failed() && "Root is NULL iff parsing failed");
+      EC = make_error_code(errc::invalid_argument);
+      return false;
+    }
+
     if (isa<NullNode>(N)) {
       // Empty files are allowed and ignored
       ++DocIterator;
Index: unittests/Support/YAMLIOTest.cpp
===================================================================
--- unittests/Support/YAMLIOTest.cpp
+++ unittests/Support/YAMLIOTest.cpp
@@ -58,12 +58,23 @@
 //
 TEST(YAMLIO, TestMapRead) {
   FooBar doc;
-  Input yin("---\nfoo:  3\nbar:  5\n...\n");
-  yin >> doc;
+  {
+    Input yin("---\nfoo:  3\nbar:  5\n...\n");
+    yin >> doc;
 
-  EXPECT_FALSE(yin.error());
-  EXPECT_EQ(doc.foo, 3);
-  EXPECT_EQ(doc.bar,5);
+    EXPECT_FALSE(yin.error());
+    EXPECT_EQ(doc.foo, 3);
+    EXPECT_EQ(doc.bar,5);
+  }
+
+  {
+    Input yin("{foo: 3, bar: 5}");
+    yin >> doc;
+
+    EXPECT_FALSE(yin.error());
+    EXPECT_EQ(doc.foo, 3);
+    EXPECT_EQ(doc.bar,5);
+  }
 }
 
 
@@ -1297,3 +1308,22 @@
   EXPECT_TRUE(yin.error());
 }
 
+//
+// Test error handling with a malformed object
+//
+TEST(YAMLIO, TestMalformedMap) {
+  FooBar doc;
+  {
+    Input yin("{foo:3, bar: 5}");
+    yin.setDiagHandler(suppressErrorMessages);
+    yin >> doc;
+    EXPECT_TRUE(yin.error());
+  }
+
+  {
+    Input yin("---\nfoo:3\nbar: 5\n...\n");
+    yin.setDiagHandler(suppressErrorMessages);
+    yin >> doc;
+    EXPECT_TRUE(yin.error());
+  }
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1236.2.patch
Type: text/x-patch
Size: 2450 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130730/db202fe4/attachment.bin>