diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/graph.cc | 15 | ||||
-rw-r--r-- | src/graph_test.cc | 11 | ||||
-rw-r--r-- | src/manifest_parser_test.cc | 4 | ||||
-rw-r--r-- | src/util.cc | 104 | ||||
-rw-r--r-- | src/util.h | 7 | ||||
-rw-r--r-- | src/util_test.cc | 23 |
6 files changed, 152 insertions, 12 deletions
diff --git a/src/graph.cc b/src/graph.cc index 9801a7b..65f9244 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -250,16 +250,13 @@ string EdgeEnv::MakePathList(vector<Node*>::iterator begin, char sep) { string result; for (vector<Node*>::iterator i = begin; i != end; ++i) { - if (!result.empty()) - result.push_back(sep); + if (!result.empty()) result.push_back(sep); const string& path = (*i)->path(); - if (path.find(" ") != string::npos) { - result.append("\""); - result.append(path); - result.append("\""); - } else { - result.append(path); - } +#if _WIN32 + GetWin32EscapedString(path, &result); +#else + GetShellEscapedString(path, &result); +#endif } return result; } diff --git a/src/graph_test.cc b/src/graph_test.cc index 8521216..14dc678 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -139,13 +139,18 @@ TEST_F(GraphTest, RootNodes) { } } -TEST_F(GraphTest, VarInOutQuoteSpaces) { +TEST_F(GraphTest, VarInOutPathEscaping) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -"build a$ b: cat nospace with$ space nospace2\n")); +"build a$ b: cat no'space with$ space$$ no\"space2\n")); Edge* edge = GetNode("a b")->in_edge(); - EXPECT_EQ("cat nospace \"with space\" nospace2 > \"a b\"", +#if _WIN32 + EXPECT_EQ("cat no'space \"with space$\" \"no\\\"space2\" > \"a b\"", edge->EvaluateCommand()); +#else + EXPECT_EQ("cat 'no'\\''space' 'with space$' 'no\"space2' > 'a b'", + edge->EvaluateCommand()); +#endif } // Regression test for https://github.com/martine/ninja/issues/380 diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 0b4c042..152b965 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -236,7 +236,11 @@ TEST_F(ParserTest, Dollars) { "build $x: foo y\n" )); EXPECT_EQ("$dollar", state.bindings_.LookupVariable("x")); +#ifdef _WIN32 EXPECT_EQ("$dollarbar$baz$blah", state.edges_[0]->EvaluateCommand()); +#else + EXPECT_EQ("'$dollar'bar$baz$blah", state.edges_[0]->EvaluateCommand()); +#endif } TEST_F(ParserTest, EscapeSpaces) { diff --git a/src/util.cc b/src/util.cc index 6ba3c6c..0e4dc59 100644 --- a/src/util.cc +++ b/src/util.cc @@ -20,6 +20,7 @@ #include <share.h> #endif +#include <assert.h> #include <errno.h> #include <fcntl.h> #include <stdarg.h> @@ -175,6 +176,109 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { return true; } +static inline bool IsKnownShellSafeCharacter(char ch) { + if ('A' <= ch && ch <= 'Z') return true; + if ('a' <= ch && ch <= 'z') return true; + if ('0' <= ch && ch <= '9') return true; + + switch (ch) { + case '_': + case '-': + case '.': + case '/': + return true; + default: + return false; + } +} + +static inline bool IsKnownWin32SafeCharacter(char ch) { + switch (ch) { + case '\\': + case ' ': + case '"': + return false; + default: + return true; + } +} + +static inline bool StringNeedsShellEscaping(const string& input) { + for (size_t i = 0; i < input.size(); ++i) { + if (!IsKnownShellSafeCharacter(input[i])) return true; + } + return false; +} + +static inline bool StringNeedsWin32Escaping(const string& input) { + for (size_t i = 0; i < input.size(); ++i) { + if (!IsKnownWin32SafeCharacter(input[i])) return true; + } + return false; +} + +void GetShellEscapedString(const string& input, string* result) { + assert(result); + + if (!StringNeedsShellEscaping(input)) { + result->append(input); + return; + } + + const char kQuote = '\''; + const char kEscapeSequence[] = "'\\'"; + + result->push_back(kQuote); + + string::const_iterator span_begin = input.begin(); + for (string::const_iterator it = input.begin(), end = input.end(); it != end; + ++it) { + if (*it == kQuote) { + result->append(span_begin, it); + result->append(kEscapeSequence); + span_begin = it; + } + } + result->append(span_begin, input.end()); + result->push_back(kQuote); +} + + +void GetWin32EscapedString(const string& input, string* result) { + assert(result); + if (!StringNeedsWin32Escaping(input)) { + result->append(input); + return; + } + + const char kQuote = '"'; + const char kBackslash = '\\'; + + result->push_back(kQuote); + size_t consecutive_backslash_count = 0; + string::const_iterator span_begin = input.begin(); + for (string::const_iterator it = input.begin(), end = input.end(); it != end; + ++it) { + switch (*it) { + case kBackslash: + ++consecutive_backslash_count; + break; + case kQuote: + result->append(span_begin, it); + result->append(consecutive_backslash_count + 1, kBackslash); + span_begin = it; + consecutive_backslash_count = 0; + break; + default: + consecutive_backslash_count = 0; + break; + } + } + result->append(span_begin, input.end()); + result->append(consecutive_backslash_count, kBackslash); + result->push_back(kQuote); +} + int ReadFile(const string& path, string* contents, string* err) { FILE* f = fopen(path.c_str(), "r"); if (!f) { @@ -45,6 +45,13 @@ bool CanonicalizePath(string* path, string* err); bool CanonicalizePath(char* path, size_t* len, string* err); +/// Appends |input| to |*result|, escaping according to the whims of either +/// Bash, or Win32's CommandLineToArgvW(). +/// Appends the string directly to |result| without modification if we can +/// determine that it contains no problematic characters. +void GetShellEscapedString(const string& input, string* result); +void GetWin32EscapedString(const string& input, string* result); + /// Read a file to a string (in text mode: with CRLF conversion /// on Windows). /// Returns -errno and fills in \a err on error. diff --git a/src/util_test.cc b/src/util_test.cc index 1e29053..f6728fb 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -136,6 +136,29 @@ TEST(CanonicalizePath, NotNullTerminated) { EXPECT_EQ("file ./file bar/.", string(path)); } +TEST(PathEscaping, TortureTest) { + string result; + + GetWin32EscapedString("foo bar\\\"'$@d!st!c'\\path'\\", &result); + EXPECT_EQ("\"foo bar\\\\\\\"'$@d!st!c'\\path'\\\\\"", result); + result.clear(); + + GetShellEscapedString("foo bar\"/'$@d!st!c'/path'", &result); + EXPECT_EQ("'foo bar\"/'\\''$@d!st!c'\\''/path'\\'''", result); +} + +TEST(PathEscaping, SensiblePathsAreNotNeedlesslyEscaped) { + const char* path = "some/sensible/path/without/crazy/characters.cc"; + string result; + + GetWin32EscapedString(path, &result); + EXPECT_EQ(path, result); + result.clear(); + + GetShellEscapedString(path, &result); + EXPECT_EQ(path, result); +} + TEST(StripAnsiEscapeCodes, EscapeAtEnd) { string stripped = StripAnsiEscapeCodes("foo\33"); EXPECT_EQ("foo", stripped); |