Invalid JSON output when BinaryValue type present as part of input object.

JSON doesn't have a syntax to support the base::BinaryValue type, so when these are present under a base::DictionarValue or base::ListValue, they must be suppressed.  However, in doing so, the current code doesn't properly suppress the commas between members as part of the overall suppression.

Furthermore, if suppression was not enabled by the caller, we need to report failure if a BinaryValue is encountered. 

BUG=340441
BUG=340733
R=thakis@chromium.org

Review URL: https://codereview.chromium.org/130563010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@249830 0039d316-1c4b-4281-b951-d872f2087c98


CrOS-Libchrome-Original-Commit: 4d3cc0101211d30d74f37c9ecfc8526402a3bd2f
diff --git a/base/json/json_writer.cc b/base/json/json_writer.cc
index b606c10..efe57ec 100644
--- a/base/json/json_writer.cc
+++ b/base/json/json_writer.cc
@@ -21,22 +21,24 @@
 #endif
 
 // static
-void JSONWriter::Write(const Value* const node, std::string* json) {
-  WriteWithOptions(node, 0, json);
+bool JSONWriter::Write(const Value* const node, std::string* json) {
+  return WriteWithOptions(node, 0, json);
 }
 
 // static
-void JSONWriter::WriteWithOptions(const Value* const node, int options,
+bool JSONWriter::WriteWithOptions(const Value* const node, int options,
                                   std::string* json) {
   json->clear();
   // Is there a better way to estimate the size of the output?
   json->reserve(1024);
 
   JSONWriter writer(options, json);
-  writer.BuildJSONString(node, 0U);
+  bool result = writer.BuildJSONString(node, 0U);
 
   if (options & OPTIONS_PRETTY_PRINT)
     json->append(kPrettyPrintLineEnding);
+
+  return result;
 }
 
 JSONWriter::JSONWriter(int options, std::string* json)
@@ -48,11 +50,11 @@
   DCHECK(json);
 }
 
-void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
+bool JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
   switch (node->GetType()) {
     case Value::TYPE_NULL: {
       json_string_->append("null");
-      break;
+      return true;
     }
 
     case Value::TYPE_BOOLEAN: {
@@ -60,7 +62,7 @@
       bool result = node->GetAsBoolean(&value);
       DCHECK(result);
       json_string_->append(value ? "true" : "false");
-      break;
+      return result;
     }
 
     case Value::TYPE_INTEGER: {
@@ -68,7 +70,7 @@
       bool result = node->GetAsInteger(&value);
       DCHECK(result);
       json_string_->append(IntToString(value));
-      break;
+      return result;
     }
 
     case Value::TYPE_DOUBLE: {
@@ -80,7 +82,7 @@
           value >= kint64min &&
           std::floor(value) == value) {
         json_string_->append(Int64ToString(static_cast<int64>(value)));
-        break;
+        return result;
       }
       std::string real = DoubleToString(value);
       // Ensure that the number has a .0 if there's no decimal or 'e'.  This
@@ -100,7 +102,7 @@
         real.insert(1U, 1U, '0');
       }
       json_string_->append(real);
-      break;
+      return result;
     }
 
     case Value::TYPE_STRING: {
@@ -108,7 +110,7 @@
       bool result = node->GetAsString(&value);
       DCHECK(result);
       EscapeJSONString(value, true, json_string_);
-      break;
+      return result;
     }
 
     case Value::TYPE_LIST: {
@@ -116,27 +118,32 @@
       if (pretty_print_)
         json_string_->push_back(' ');
 
-      const ListValue* list = static_cast<const ListValue*>(node);
+      const ListValue* list = NULL;
+      bool first_value_has_been_output = false;
+      bool result = node->GetAsList(&list);
+      DCHECK(result);
       for (ListValue::const_iterator it = list->begin(); it != list->end();
            ++it) {
         const Value* value = *it;
-
         if (omit_binary_values_ && value->GetType() == Value::TYPE_BINARY)
           continue;
 
-        if (it != list->begin()) {
+        if (first_value_has_been_output) {
           json_string_->push_back(',');
           if (pretty_print_)
             json_string_->push_back(' ');
         }
 
-        BuildJSONString(value, depth);
+        if (!BuildJSONString(value, depth))
+          result = false;
+
+        first_value_has_been_output = true;
       }
 
       if (pretty_print_)
         json_string_->push_back(' ');
       json_string_->push_back(']');
-      break;
+      return result;
     }
 
     case Value::TYPE_DICTIONARY: {
@@ -144,17 +151,18 @@
       if (pretty_print_)
         json_string_->append(kPrettyPrintLineEnding);
 
-      const DictionaryValue* dict =
-          static_cast<const DictionaryValue*>(node);
-      bool first_entry = true;
+      const DictionaryValue* dict = NULL;
+      bool first_value_has_been_output = false;
+      bool result = node->GetAsDictionary(&dict);
+      DCHECK(result);
       for (DictionaryValue::Iterator itr(*dict); !itr.IsAtEnd();
-           itr.Advance(), first_entry = false) {
+           itr.Advance()) {
         if (omit_binary_values_ &&
             itr.value().GetType() == Value::TYPE_BINARY) {
           continue;
         }
 
-        if (!first_entry) {
+        if (first_value_has_been_output) {
           json_string_->push_back(',');
           if (pretty_print_)
             json_string_->append(kPrettyPrintLineEnding);
@@ -164,34 +172,32 @@
           IndentLine(depth + 1U);
 
         EscapeJSONString(itr.key(), true, json_string_);
-
         json_string_->push_back(':');
         if (pretty_print_)
           json_string_->push_back(' ');
-        BuildJSONString(&itr.value(), depth + 1U);
+
+        if (!BuildJSONString(&itr.value(), depth + 1U))
+          result = false;
+
+        first_value_has_been_output = true;
       }
 
       if (pretty_print_) {
         json_string_->append(kPrettyPrintLineEnding);
         IndentLine(depth);
-        json_string_->push_back('}');
-      } else {
-        json_string_->push_back('}');
       }
-      break;
+
+      json_string_->push_back('}');
+      return result;
     }
 
-    case Value::TYPE_BINARY: {
-      if (!omit_binary_values_) {
-        NOTREACHED() << "Cannot serialize binary value.";
-      }
-      break;
-    }
-
-    default: {
-      NOTREACHED() << "unknown json type";
-    }
+    case Value::TYPE_BINARY:
+      // Successful only if we're allowed to omit it.
+      DLOG_IF(ERROR, !omit_binary_values_) << "Cannot serialize binary value.";
+      return omit_binary_values_;
   }
+  NOTREACHED();
+  return false;
 }
 
 void JSONWriter::IndentLine(size_t depth) {