Fix escaping in compile_commands.json
In particular this case is broken
defines = ["INCLUDE=<header.h>"]
This is escaped as
-DINCLUDE=\\u003Cheader.h>
which JSON decodes as
-DINCLUDE=\u003Cheader.h>
which still has the JSON escape sequence \u003C in it. The current code
JSON escapes twice, but the command is only parsed once as JSON.
JSON escaping isn't shell escaping. Add the simplified shell escaping
described in the spec. This changed the output slightly, to quote spaces
instead of escaping them, but that's in line with the spec (including
the example).
VS Code is happy with this output, and not the previous output.
Bug: gn:181
Change-Id: I52f2470178066b072c2729181c91b947972b5bc4
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/9420
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/compile_commands_writer.cc b/src/gn/compile_commands_writer.cc
index d0367fc..05e78d0 100644
--- a/src/gn/compile_commands_writer.cc
+++ b/src/gn/compile_commands_writer.cc
@@ -75,8 +75,9 @@
bool has_precompiled_headers =
target->config_values().has_precompiled_headers();
- flags.defines = FlagsGetter<std::string>(target, &ConfigValues::defines,
- DefineWriter(ESCAPE_SPACE, true));
+ flags.defines =
+ FlagsGetter<std::string>(target, &ConfigValues::defines,
+ DefineWriter(ESCAPE_COMPILATION_DATABASE));
flags.framework_dirs =
FlagsGetter<SourceDir>(target, &ConfigValues::framework_dirs,
@@ -84,10 +85,10 @@
flags.frameworks = FlagsGetter<std::string>(
target, &ConfigValues::frameworks,
- FrameworksWriter(ESCAPE_SPACE, true, "-framework"));
+ FrameworksWriter(ESCAPE_COMPILATION_DATABASE, "-framework"));
flags.frameworks += FlagsGetter<std::string>(
target, &ConfigValues::weak_frameworks,
- FrameworksWriter(ESCAPE_SPACE, true, "-weak_framework"));
+ FrameworksWriter(ESCAPE_COMPILATION_DATABASE, "-weak_framework"));
flags.includes = FlagsGetter<SourceDir>(target, &ConfigValues::include_dirs,
IncludeWriter(path_output));
diff --git a/src/gn/compile_commands_writer_unittest.cc b/src/gn/compile_commands_writer_unittest.cc
index 9bbb076..29e6a57 100644
--- a/src/gn/compile_commands_writer_unittest.cc
+++ b/src/gn/compile_commands_writer_unittest.cc
@@ -237,6 +237,7 @@
target.config_values().defines().push_back("BOOL_DEF");
target.config_values().defines().push_back("INT_DEF=123");
target.config_values().defines().push_back("STR_DEF=\"ABCD 1\"");
+ target.config_values().defines().push_back("INCLUDE=<header.h>");
ASSERT_TRUE(target.OnResolved(&err));
targets.push_back(&target);
@@ -244,7 +245,8 @@
std::string out = writer.RenderJSON(build_settings(), targets);
const char expected[] =
- "-DBOOL_DEF -DINT_DEF=123 -DSTR_DEF=\\\\\\\"ABCD\\\\ 1\\\\\\\"";
+ "-DBOOL_DEF -DINT_DEF=123 \\\"-DSTR_DEF=\\\\\\\"ABCD 1\\\\\\\"\\\" "
+ "\\\"-DINCLUDE=\\u003Cheader.h>\\\"";
EXPECT_TRUE(out.find(expected) != std::string::npos);
}
diff --git a/src/gn/escape.cc b/src/gn/escape.cc
index 1874905..3c8dc3f 100644
--- a/src/gn/escape.cc
+++ b/src/gn/escape.cc
@@ -89,6 +89,36 @@
return i;
}
+inline bool ShouldEscapeCharForCompilationDatabase(char ch) {
+ return ch == '\\' || ch == '"';
+}
+
+size_t EscapeStringToString_CompilationDatabase(const std::string_view& str,
+ const EscapeOptions& options,
+ char* dest,
+ bool* needed_quoting) {
+ size_t i = 0;
+ bool quote = false;
+ for (const auto& elem : str) {
+ if (static_cast<unsigned>(elem) >= 0x80 ||
+ !kShellValid[static_cast<int>(elem)]) {
+ quote = true;
+ break;
+ }
+ }
+ if (quote)
+ dest[i++] = '"';
+
+ for (const auto& elem : str) {
+ if (ShouldEscapeCharForCompilationDatabase(elem))
+ dest[i++] = '\\';
+ dest[i++] = elem;
+ }
+ if (quote)
+ dest[i++] = '"';
+ return i;
+}
+
size_t EscapeStringToString_Depfile(const std::string_view& str,
const EscapeOptions& options,
char* dest,
@@ -224,6 +254,9 @@
return EscapeStringToString_Ninja(str, options, dest, needed_quoting);
case ESCAPE_DEPFILE:
return EscapeStringToString_Depfile(str, options, dest, needed_quoting);
+ case ESCAPE_COMPILATION_DATABASE:
+ return EscapeStringToString_CompilationDatabase(str, options, dest,
+ needed_quoting);
case ESCAPE_NINJA_COMMAND:
switch (options.platform) {
case ESCAPE_PLATFORM_CURRENT:
diff --git a/src/gn/escape.h b/src/gn/escape.h
index 78e6544..31f972e 100644
--- a/src/gn/escape.h
+++ b/src/gn/escape.h
@@ -31,6 +31,11 @@
// characters which we want to pass to the shell (like when writing tool
// commands). Only Ninja "$" are escaped.
ESCAPE_NINJA_PREFORMATTED_COMMAND,
+
+ // Shell escaping as described by JSON Compilation Database spec:
+ // Parameters use shell quoting and shell escaping of quotes, with ‘"’ and ‘\’
+ // being the only special characters.
+ ESCAPE_COMPILATION_DATABASE,
};
enum EscapingPlatform {
diff --git a/src/gn/escape_unittest.cc b/src/gn/escape_unittest.cc
index afc2bc4..004498e 100644
--- a/src/gn/escape_unittest.cc
+++ b/src/gn/escape_unittest.cc
@@ -101,3 +101,12 @@
EscapeJSONStringToStream(out2, "a: \"$\\b", opts);
EXPECT_EQ("a: \\\"$$\\\\b", buffer2.str());
}
+
+TEST(Escape, CompilationDatabase) {
+ EscapeOptions opts;
+ opts.mode = ESCAPE_COMPILATION_DATABASE;
+
+ // The only special characters are '"' and '\'.
+ std::string result = EscapeString("asdf:$ \\#*[|]bar", opts, nullptr);
+ EXPECT_EQ("\"asdf:$ \\\\#*[|]bar\"", result);
+}
diff --git a/src/gn/ninja_target_command_util.h b/src/gn/ninja_target_command_util.h
index 022d5e7..d9d30bc 100644
--- a/src/gn/ninja_target_command_util.h
+++ b/src/gn/ninja_target_command_util.h
@@ -17,24 +17,14 @@
struct DefineWriter {
DefineWriter() { options.mode = ESCAPE_NINJA_COMMAND; }
- DefineWriter(EscapingMode mode, bool escape_strings)
- : escape_strings(escape_strings) {
- options.mode = mode;
- }
+ DefineWriter(EscapingMode mode) { options.mode = mode; }
void operator()(const std::string& s, std::ostream& out) const {
out << " ";
- if (escape_strings) {
- std::string dest;
- base::EscapeJSONString(s, false, &dest);
- EscapeStringToStream(out, "-D" + dest, options);
- return;
- }
EscapeStringToStream(out, "-D" + s, options);
}
EscapeOptions options;
- bool escape_strings = false;
};
struct FrameworkDirsWriter {
@@ -59,29 +49,19 @@
struct FrameworksWriter {
explicit FrameworksWriter(const std::string& tool_switch)
- : FrameworksWriter(ESCAPE_NINJA_COMMAND, false, tool_switch) {}
- FrameworksWriter(EscapingMode mode,
- bool escape_strings,
- const std::string& tool_switch)
- : escape_strings_(escape_strings), tool_switch_(tool_switch) {
+ : FrameworksWriter(ESCAPE_NINJA_COMMAND, tool_switch) {}
+ FrameworksWriter(EscapingMode mode, const std::string& tool_switch)
+ : tool_switch_(tool_switch) {
options_.mode = mode;
}
void operator()(const std::string& s, std::ostream& out) const {
out << " " << tool_switch_;
std::string_view framework_name = GetFrameworkName(s);
-
- if (escape_strings_) {
- std::string dest;
- base::EscapeJSONString(framework_name, false, &dest);
- EscapeStringToStream(out, dest, options_);
- return;
- }
EscapeStringToStream(out, framework_name, options_);
}
EscapeOptions options_;
- bool escape_strings_;
std::string tool_switch_;
};
diff --git a/src/gn/ninja_target_command_util_unittest.cc b/src/gn/ninja_target_command_util_unittest.cc
index 248db30..8da62d3 100644
--- a/src/gn/ninja_target_command_util_unittest.cc
+++ b/src/gn/ninja_target_command_util_unittest.cc
@@ -57,13 +57,9 @@
#endif
{"FOO", "BAR=1", "BAZ=\"Baz\""});
- TestWriter(DefineWriter(ESCAPE_NINJA_COMMAND, true),
-// Escaping is different between Windows and Posix.
-#if defined(OS_WIN)
- " -DFOO -DBAR=1 \"-DBAZ=\\\\\\\"Baz\\\\\\\"\"",
-#else
- " -DFOO -DBAR=1 -DBAZ=\\\\\\\"Baz\\\\\\\"",
-#endif
+ TestWriter(DefineWriter(ESCAPE_COMPILATION_DATABASE),
+ // Escaping is different between Windows and Posix.
+ " -DFOO -DBAR=1 \"-DBAZ=\\\"Baz\\\"\"",
{"FOO", "BAR=1", "BAZ=\"Baz\""});
}
@@ -94,7 +90,7 @@
#endif
{"Foundation.framework", "Name With Spaces.framework"});
- TestWriter(FrameworksWriter(ESCAPE_SPACE, true, "-framework "),
+ TestWriter(FrameworksWriter(ESCAPE_SPACE, "-framework "),
" -framework Foundation -framework Name\\ With\\ Spaces",
{"Foundation.framework", "Name With Spaces.framework"});
}
