diff --git a/src/csv_reporter.cc b/src/csv_reporter.cc index d2f1d27eb6..af2c18fc8a 100644 --- a/src/csv_reporter.cc +++ b/src/csv_reporter.cc @@ -37,6 +37,18 @@ std::vector elements = { "error_occurred", "error_message"}; } // namespace +std::string CsvEscape(const std::string & s) { + std::string tmp; + tmp.reserve(s.size() + 2); + for (char c : s) { + switch (c) { + case '"' : tmp += "\"\""; break; + default : tmp += c; break; + } + } + return '"' + tmp + '"'; +} + bool CSVReporter::ReportContext(const Context& context) { PrintBasicContext(&GetErrorStream(), context); return true; @@ -89,18 +101,11 @@ void CSVReporter::ReportRuns(const std::vector& reports) { void CSVReporter::PrintRunData(const Run& run) { std::ostream& Out = GetOutputStream(); - - // Field with embedded double-quote characters must be doubled and the field - // delimited with double-quotes. - std::string name = run.benchmark_name(); - ReplaceAll(&name, "\"", "\"\""); - Out << '"' << name << "\","; + Out << CsvEscape(run.benchmark_name()) << ","; if (run.error_occurred) { Out << std::string(elements.size() - 3, ','); Out << "true,"; - std::string msg = run.error_message; - ReplaceAll(&msg, "\"", "\"\""); - Out << '"' << msg << "\"\n"; + Out << CsvEscape(run.error_message) << "\n"; return; } @@ -130,11 +135,7 @@ void CSVReporter::PrintRunData(const Run& run) { } Out << ","; if (!run.report_label.empty()) { - // Field with embedded double-quote characters must be doubled and the field - // delimited with double-quotes. - std::string label = run.report_label; - ReplaceAll(&label, "\"", "\"\""); - Out << "\"" << label << "\""; + Out << CsvEscape(run.report_label); } Out << ",,"; // for error_occurred and error_message diff --git a/src/json_reporter.cc b/src/json_reporter.cc index dd4ba30e38..cf1de2547a 100644 --- a/src/json_reporter.cc +++ b/src/json_reporter.cc @@ -32,30 +32,48 @@ namespace benchmark { namespace { +std::string StrEscape(const std::string & s) { + std::string tmp; + tmp.reserve(s.size()); + for (char c : s) { + switch (c) { + case '\b': tmp += "\\b"; break; + case '\f': tmp += "\\f"; break; + case '\n': tmp += "\\n"; break; + case '\r': tmp += "\\r"; break; + case '\t': tmp += "\\t"; break; + case '\\': tmp += "\\\\"; break; + case '"' : tmp += "\\\""; break; + default : tmp += c; break; + } + } + return tmp; +} + std::string FormatKV(std::string const& key, std::string const& value) { - return StrFormat("\"%s\": \"%s\"", key.c_str(), value.c_str()); + return StrFormat("\"%s\": \"%s\"", StrEscape(key).c_str(), StrEscape(value).c_str()); } std::string FormatKV(std::string const& key, const char* value) { - return StrFormat("\"%s\": \"%s\"", key.c_str(), value); + return StrFormat("\"%s\": \"%s\"", StrEscape(key).c_str(), StrEscape(value).c_str()); } std::string FormatKV(std::string const& key, bool value) { - return StrFormat("\"%s\": %s", key.c_str(), value ? "true" : "false"); + return StrFormat("\"%s\": %s", StrEscape(key).c_str(), value ? "true" : "false"); } std::string FormatKV(std::string const& key, int64_t value) { std::stringstream ss; - ss << '"' << key << "\": " << value; + ss << '"' << StrEscape(key) << "\": " << value; return ss.str(); } std::string FormatKV(std::string const& key, double value) { std::stringstream ss; - ss << '"' << key << "\": "; + ss << '"' << StrEscape(key) << "\": "; if (std::isnan(value)) - ss << "NaN"; + ss << (value < 0 ? "-" : "") << "NaN"; else if (std::isinf(value)) ss << (value < 0 ? "-" : "") << "Infinity"; else { @@ -88,12 +106,7 @@ bool JSONReporter::ReportContext(const Context& context) { out << indent << FormatKV("host_name", context.sys_info.name) << ",\n"; if (Context::executable_name) { - // windows uses backslash for its path separator, - // which must be escaped in JSON otherwise it blows up conforming JSON - // decoders - std::string executable_name = Context::executable_name; - ReplaceAll(&executable_name, "\\", "\\\\"); - out << indent << FormatKV("executable", executable_name) << ",\n"; + out << indent << FormatKV("executable", Context::executable_name) << ",\n"; } CPUInfo const& info = context.cpu_info; diff --git a/src/string_util.cc b/src/string_util.cc index 05ac5b4ea3..39b01a1719 100644 --- a/src/string_util.cc +++ b/src/string_util.cc @@ -160,15 +160,6 @@ std::string StrFormat(const char* format, ...) { return tmp; } -void ReplaceAll(std::string* str, const std::string& from, - const std::string& to) { - std::size_t start = 0; - while ((start = str->find(from, start)) != std::string::npos) { - str->replace(start, from.length(), to); - start += to.length(); - } -} - #ifdef BENCHMARK_STL_ANDROID_GNUSTL /* * GNU STL in Android NDK lacks support for some C++11 functions, including diff --git a/src/string_util.h b/src/string_util.h index c5dcee6fcd..09d7b4bd2a 100644 --- a/src/string_util.h +++ b/src/string_util.h @@ -37,9 +37,6 @@ inline std::string StrCat(Args&&... args) { return ss.str(); } -void ReplaceAll(std::string* str, const std::string& from, - const std::string& to); - #ifdef BENCHMARK_STL_ANDROID_GNUSTL /* * GNU STL in Android NDK lacks support for some C++11 functions, including diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index 8b7ae935aa..c8090d4aca 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -704,6 +704,37 @@ ADD_CASES( "manual_time_stddev\",%csv_report$"}, {"^\"BM_UserStats/iterations:5/repeats:3/manual_time_\",%csv_report$"}}); +// ========================================================================= // +// ------------------------- Testing StrEscape JSON ------------------------ // +// ========================================================================= // +#if 0 // enable when csv testing code correctly handles multi-line fields +void BM_JSON_Format(benchmark::State& state) { + state.SkipWithError("val\b\f\n\r\t\\\"with\"es,capes"); + for (auto _ : state) { + } +} +BENCHMARK(BM_JSON_Format); +ADD_CASES(TC_JSONOut, {{"\"name\": \"BM_JSON_Format\",$"}, + {"\"run_name\": \"BM_JSON_Format\",$", MR_Next}, + {"\"run_type\": \"iteration\",$", MR_Next}, + {"\"repetitions\": 0,$", MR_Next}, + {"\"repetition_index\": 0,$", MR_Next}, + {"\"threads\": 1,$", MR_Next}, + {"\"error_occurred\": true,$", MR_Next}, + {R"("error_message": "val\\b\\f\\n\\r\\t\\\\\\"with\\"es,capes",$)", MR_Next}}); +#endif +// ========================================================================= // +// -------------------------- Testing CsvEscape ---------------------------- // +// ========================================================================= // + +void BM_CSV_Format(benchmark::State& state) { + state.SkipWithError("\"freedom\""); + for (auto _ : state) { + } +} +BENCHMARK(BM_CSV_Format); +ADD_CASES(TC_CSVOut, {{"^\"BM_CSV_Format\",,,,,,,,true,\"\"\"freedom\"\"\"$"}}); + // ========================================================================= // // --------------------------- TEST CASES END ------------------------------ // // ========================================================================= //