trace_processor: Replace dangerous characters in debug annotation names

If a debug annotation name contains dots or brackets, it can break JSON
exporter. The proposed solution is to replace them with underscores.

Also skips all malformed argument names in JSON exporter.

Bug: chromium:1039352
Change-Id: I94f2bb62a766ea513b30ac533ab38df231a57d4a

src/trace_processor/export_json.cc

diff --git a/src/trace_processor/export_json.cc b/src/trace_processor/export_json.cc
index 0f768b1..e10da4e 100644
--- a/src/trace_processor/export_json.cc
+++ b/src/trace_processor/export_json.cc
@@ -389,6 +389,11 @@
                  const Json::Value& value) {
     Json::Value* target = &args_sets_[set_id];
     for (base::StringSplitter parts(key, '.'); parts.Next();) {
+      if (PERFETTO_UNLIKELY(!target->isNull() && !target->isObject())) {
+        PERFETTO_DLOG("Malformed arguments. Can't append %s to %s.",
+                      key.c_str(), args_sets_[set_id].toStyledString().c_str());
+        return;
+      }
       std::string key_part = parts.cur_token();
       size_t bracketpos = key_part.find('[');
       if (bracketpos == key_part.npos) {  // A single item
@@ -398,6 +403,12 @@
         while (bracketpos != key_part.npos) {
           std::string index = key_part.substr(
               bracketpos + 1, key_part.find(']', bracketpos) - bracketpos - 1);
+          if (PERFETTO_UNLIKELY(!target->isNull() && !target->isArray())) {
+            PERFETTO_DLOG("Malformed arguments. Can't append %s to %s.",
+                          key.c_str(),
+                          args_sets_[set_id].toStyledString().c_str());
+            return;
+          }
           target = &(*target)[stoi(index)];
           bracketpos = key_part.find('[', bracketpos + 1);
         }

src/trace_processor/importers/proto/proto_trace_parser_unittest.cc

diff --git a/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc b/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
index 6026684..99ad6e7 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
+++ b/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
@@ -1792,6 +1792,9 @@
     auto* annotation8 = event->add_debug_annotations();
     annotation8->set_name_iid(8);
     annotation8->set_legacy_json_value("val8");
+    auto* annotation9 = event->add_debug_annotations();
+    annotation9->set_name_iid(9);
+    annotation9->set_int_value(15);
     auto* legacy_event = event->set_legacy_event();
     legacy_event->set_name_iid(1);
     legacy_event->set_phase('E');
@@ -1815,6 +1818,9 @@
     auto an8 = interned_data->add_debug_annotation_names();
     an8->set_iid(8);
     an8->set_name("an8");
+    auto an9 = interned_data->add_debug_annotation_names();
+    an9->set_iid(9);
+    an9->set_name("an8.foo");
   }
 
   Tokenize();
@@ -1865,6 +1871,8 @@
       .WillOnce(Return(17));
   EXPECT_CALL(*storage_, InternString(base::StringView("val8")))
       .WillOnce(Return(18));
+  EXPECT_CALL(*storage_, InternString(base::StringView("debug.an8_foo")))
+      .WillOnce(Return(19));
 
   EXPECT_CALL(*storage_, GetString(StringId(4))).WillOnce(Return("debug.an2"));
 
@@ -1901,6 +1909,8 @@
   EXPECT_CALL(inserter,
               AddArg(StringId(15), StringId(15), Variadic::String(16)));
   EXPECT_CALL(inserter, AddArg(StringId(17), StringId(17), Variadic::Json(18)));
+  EXPECT_CALL(inserter,
+              AddArg(StringId(19), StringId(19), Variadic::Integer(15)));
 
   context_.sorter->ExtractEventsForced();
 }

src/trace_processor/importers/proto/track_event_parser.cc

diff --git a/src/trace_processor/importers/proto/track_event_parser.cc b/src/trace_processor/importers/proto/track_event_parser.cc
index 42073fb..a172e61 100644
--- a/src/trace_processor/importers/proto/track_event_parser.cc
+++ b/src/trace_processor/importers/proto/track_event_parser.cc
@@ -86,6 +86,15 @@
   // By returning false we expect this field to be handled like regular.
   return true;
 }
+
+std::string SafeDebugAnnotationName(const std::string& raw_name) {
+  std::string result = raw_name;
+  std::replace(result.begin(), result.end(), '.', '_');
+  std::replace(result.begin(), result.end(), '[', '_');
+  std::replace(result.begin(), result.end(), ']', '_');
+  result = "debug." + result;
+  return result;
+}
 }  // namespace
 
 TrackEventParser::TrackEventParser(TraceProcessorContext* context)
@@ -838,7 +847,8 @@
     if (!decoder)
       return;
 
-    std::string name_prefixed = "debug." + decoder->name().ToStdString();
+    std::string name_prefixed =
+        SafeDebugAnnotationName(decoder->name().ToStdString());
     name_id = storage->InternString(base::StringView(name_prefixed));
   } else if (annotation.has_name()) {
     name_id = storage->InternString(annotation.name());