Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(102)

Unified Diff: src/libplatform/tracing/trace-writer.cc

Issue 2309943005: [Tracing] Minor bug fixes related to trace serialization (Closed)
Patch Set: Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/cctest/libplatform/test-tracing.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/libplatform/tracing/trace-writer.cc
diff --git a/src/libplatform/tracing/trace-writer.cc b/src/libplatform/tracing/trace-writer.cc
index 6d94350cfd9d165a5c632cb8088a928a598afa96..ec95527d5f6a892e4cc9437b5f526af471219830 100644
--- a/src/libplatform/tracing/trace-writer.cc
+++ b/src/libplatform/tracing/trace-writer.cc
@@ -13,17 +13,45 @@ namespace v8 {
namespace platform {
namespace tracing {
-// Currently we do not support JSON-escaping strings in trace arguments.
-// Thus we perform an IsJSONString() check before writing any string argument.
-// In particular, this means strings cannot have control characters or \.
-V8_INLINE static bool IsJSONString(const char* str) {
+// Writes the given string to a stream, taking care to escape characters
+// when necessary.
+V8_INLINE static void WriteJSONStringToStream(const char* str,
+ std::ostream& stream) {
size_t len = strlen(str);
+ stream << "\"";
for (size_t i = 0; i < len; ++i) {
- if (iscntrl(str[i]) || str[i] == '\\') {
- return false;
+ // All of the permitted escape sequences in JSON strings, as per
+ // https://mathiasbynens.be/notes/javascript-escapes
+ switch (str[i]) {
+ case '\b':
+ stream << "\\b";
+ break;
+ case '\f':
+ stream << "\\f";
+ break;
+ case '\n':
+ stream << "\\n";
+ break;
+ case '\r':
+ stream << "\\r";
+ break;
+ case '\t':
+ stream << "\\t";
+ break;
+ case '\"':
+ stream << "\\\"";
+ break;
+ case '\\':
+ stream << "\\\\";
+ break;
+ // Note that because we use double quotes for JSON strings,
+ // we don't need to escape single quotes.
+ default:
+ stream << str[i];
+ break;
}
}
- return true;
+ stream << "\"";
}
void JSONTraceWriter::AppendArgValue(uint8_t type,
@@ -72,10 +100,11 @@ void JSONTraceWriter::AppendArgValue(uint8_t type,
break;
case TRACE_VALUE_TYPE_STRING:
case TRACE_VALUE_TYPE_COPY_STRING:
- // Strings are currently not JSON-escaped, so we need to perform a check
- // to see if they are valid JSON strings.
- CHECK(value.as_string != nullptr && IsJSONString(value.as_string));
- stream_ << (value.as_string ? value.as_string : "NULL");
+ if (value.as_string == nullptr) {
+ stream_ << "\"NULL\"";
+ } else {
+ WriteJSONStringToStream(value.as_string, stream_);
+ }
break;
default:
UNREACHABLE();
« no previous file with comments | « no previous file | test/cctest/libplatform/test-tracing.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698