Invalid JSON output when BinaryValue suppressed.

base/json/json_writer.cc

Index: base/json/json_writer.cc
diff --git a/base/json/json_writer.cc b/base/json/json_writer.cc
index b606c100f2a97c70f2f0a26f6a10504b9cd558d1..57a0c8ea375b31cf954dacb07a2b6b3cc81a585a 100644
--- a/base/json/json_writer.cc
+++ b/base/json/json_writer.cc
@@ -21,22 +21,24 @@ const char kPrettyPrintLineEnding[] = "\n";
 #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 @@ JSONWriter::JSONWriter(int options, std::string* json)
   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 @@ void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
       bool result = node->GetAsBoolean(&value);
       DCHECK(result);
       json_string_->append(value ? "true" : "false");
-      break;
+      return result;
     }
 
     case Value::TYPE_INTEGER: {
@@ -68,7 +70,7 @@ void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
       bool result = node->GetAsInteger(&value);
       DCHECK(result);
       json_string_->append(IntToString(value));
-      break;
+      return result;
     }
 
     case Value::TYPE_DOUBLE: {
@@ -80,7 +82,7 @@ void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
           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 @@ void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
         real.insert(1U, 1U, '0');
       }
       json_string_->append(real);
-      break;
+      return result;
     }
 
     case Value::TYPE_STRING: {
@@ -108,7 +110,7 @@ void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
       bool result = node->GetAsString(&value);
       DCHECK(result);
       EscapeJSONString(value, true, json_string_);
-      break;
+      return result;
     }
 
     case Value::TYPE_LIST: {
@@ -116,27 +118,32 @@ void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
       if (pretty_print_)
         json_string_->push_back(' ');
 
-      const ListValue* list = static_cast<const ListValue*>(node);
+      const ListValue* list = NULL;
+      bool first_value_output = false;

[Nico, 2014/02/07 18:17:06]

This can be parsed as both "is this the first value output" and "was the the
first value output" which have opposite meanings. "is_first_value" would be
unambiguous.

+      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_output) {
           json_string_->push_back(',');
           if (pretty_print_)
             json_string_->push_back(' ');
         }
 
-        BuildJSONString(value, depth);
+        if (!BuildJSONString(value, depth))
+          result = false;
+
+        first_value_output = true;
       }
 
       if (pretty_print_)
         json_string_->push_back(' ');
       json_string_->push_back(']');
-      break;
+      return result;
     }
 
     case Value::TYPE_DICTIONARY: {
@@ -144,17 +151,18 @@ void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
       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_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_output) {
           json_string_->push_back(',');
           if (pretty_print_)
             json_string_->append(kPrettyPrintLineEnding);
@@ -164,33 +172,29 @@ void JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
           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_output = true;
       }
 
       if (pretty_print_) {
         json_string_->append(kPrettyPrintLineEnding);
         IndentLine(depth);
-        json_string_->push_back('}');
-      } else {
-        json_string_->push_back('}');
       }
-      break;
-    }
 
-    case Value::TYPE_BINARY: {
-      if (!omit_binary_values_) {
-        NOTREACHED() << "Cannot serialize binary value.";
-      }
-      break;
+      json_string_->push_back('}');
+      return result;
     }
 
-    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_;
   }
 }