Merge pull request #1307 from moroten/fix-too-long-path-test-bug

Verify GetFullPathName return value

3 files changed, 74 insertions, 14 deletions

diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc
index 795542b..79bf5b4 100644
--- a/src/includes_normalize-win32.cc
+++ b/src/includes_normalize-win32.cc
@@ -26,6 +26,21 @@
 
 namespace {
 
+bool InternalGetFullPathName(const StringPiece& file_name, char* buffer,
+                             size_t buffer_length, string *err) {
+  DWORD result_size = GetFullPathNameA(file_name.AsString().c_str(),
+                                       buffer_length, buffer, NULL);
+  if (result_size == 0) {
+    *err = "GetFullPathNameA(" + file_name.AsString() + "): " +
+        GetLastErrorString();
+    return false;
+  } else if (result_size > buffer_length) {
+    *err = "path too long";
+    return false;
+  }
+  return true;
+}
+
 bool IsPathSeparator(char c) {
   return c == '/' ||  c == '\\';
 }
@@ -54,15 +69,19 @@ bool SameDriveFast(StringPiece a, StringPiece b) {
 }
 
 // Return true if paths a and b are on the same Windows drive.
-bool SameDrive(StringPiece a, StringPiece b)  {
+bool SameDrive(StringPiece a, StringPiece b, string* err)  {
   if (SameDriveFast(a, b)) {
     return true;
   }
 
   char a_absolute[_MAX_PATH];
   char b_absolute[_MAX_PATH];
-  GetFullPathNameA(a.AsString().c_str(), sizeof(a_absolute), a_absolute, NULL);
-  GetFullPathNameA(b.AsString().c_str(), sizeof(b_absolute), b_absolute, NULL);
+  if (!InternalGetFullPathName(a, a_absolute, sizeof(a_absolute), err)) {
+    return false;
+  }
+  if (!InternalGetFullPathName(b, b_absolute, sizeof(b_absolute), err)) {
+    return false;
+  }
   char a_drive[_MAX_DIR];
   char b_drive[_MAX_DIR];
   _splitpath(a_absolute, a_drive, NULL, NULL, NULL);
@@ -106,11 +125,15 @@ bool IsFullPathName(StringPiece s) {
 }  // anonymous namespace
 
 IncludesNormalize::IncludesNormalize(const string& relative_to) {
-  relative_to_ = AbsPath(relative_to);
+  string err;
+  relative_to_ = AbsPath(relative_to, &err);
+  if (!err.empty()) {
+    Fatal("Initializing IncludesNormalize(): %s", err.c_str());
+  }
   split_relative_to_ = SplitStringPiece(relative_to_, '/');
 }
 
-string IncludesNormalize::AbsPath(StringPiece s) {
+string IncludesNormalize::AbsPath(StringPiece s, string* err) {
   if (IsFullPathName(s)) {
     string result = s.AsString();
     for (size_t i = 0; i < result.size(); ++i) {
@@ -122,7 +145,9 @@ string IncludesNormalize::AbsPath(StringPiece s) {
   }
 
   char result[_MAX_PATH];
-  GetFullPathNameA(s.AsString().c_str(), sizeof(result), result, NULL);
+  if (!InternalGetFullPathName(s, result, sizeof(result), err)) {
+    return "";
+  }
   for (char* c = result; *c; ++c)
     if (*c == '\\')
       *c = '/';
@@ -130,8 +155,10 @@ string IncludesNormalize::AbsPath(StringPiece s) {
 }
 
 string IncludesNormalize::Relativize(
-    StringPiece path, const vector<StringPiece>& start_list) {
-  string abs_path = AbsPath(path);
+    StringPiece path, const vector<StringPiece>& start_list, string* err) {
+  string abs_path = AbsPath(path, err);
+  if (!err->empty())
+    return "";
   vector<StringPiece> path_list = SplitStringPiece(abs_path, '/');
   int i;
   for (i = 0; i < static_cast<int>(min(start_list.size(), path_list.size()));
@@ -165,12 +192,18 @@ bool IncludesNormalize::Normalize(const string& input,
   if (!CanonicalizePath(copy, &len, &slash_bits, err))
     return false;
   StringPiece partially_fixed(copy, len);
-  string abs_input = AbsPath(partially_fixed);
+  string abs_input = AbsPath(partially_fixed, err);
+  if (!err->empty())
+    return false;
 
-  if (!SameDrive(abs_input, relative_to_)) {
+  if (!SameDrive(abs_input, relative_to_, err)) {
+    if (!err->empty())
+      return false;
     *result = partially_fixed.AsString();
     return true;
   }
-  *result = Relativize(abs_input, split_relative_to_);
+  *result = Relativize(abs_input, split_relative_to_, err);
+  if (!err->empty())
+    return false;
   return true;
 }
diff --git a/src/includes_normalize.h b/src/includes_normalize.h
index 3811e53..0339581 100644
--- a/src/includes_normalize.h
+++ b/src/includes_normalize.h
@@ -25,9 +25,9 @@ struct IncludesNormalize {
   IncludesNormalize(const string& relative_to);
 
   // Internal utilities made available for testing, maybe useful otherwise.
-  static string AbsPath(StringPiece s);
+  static string AbsPath(StringPiece s, string* err);
   static string Relativize(StringPiece path,
-                           const vector<StringPiece>& start_list);
+                           const vector<StringPiece>& start_list, string* err);
 
   /// Normalize by fixing slashes style, fixing redundant .. and . and makes the
   /// path |input| relative to |this->relative_to_| and store to |result|.
diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc
index eac36fd..dbcdbe0 100644
--- a/src/includes_normalize_test.cc
+++ b/src/includes_normalize_test.cc
@@ -58,9 +58,12 @@ TEST(IncludesNormalize, Simple) {
 }
 
 TEST(IncludesNormalize, WithRelative) {
+  string err;
   string currentdir = GetCurDir();
   EXPECT_EQ("c", NormalizeRelativeAndCheckNoError("a/b/c", "a/b"));
-  EXPECT_EQ("a", NormalizeAndCheckNoError(IncludesNormalize::AbsPath("a")));
+  EXPECT_EQ("a",
+            NormalizeAndCheckNoError(IncludesNormalize::AbsPath("a", &err)));
+  EXPECT_EQ("", err);
   EXPECT_EQ(string("../") + currentdir + string("/a"),
             NormalizeRelativeAndCheckNoError("a", "../b"));
   EXPECT_EQ(string("../") + currentdir + string("/a/b"),
@@ -138,3 +141,27 @@ TEST(IncludesNormalize, LongInvalidPath) {
   EXPECT_EQ(forward_slashes.substr(cwd_len + 1),
             NormalizeAndCheckNoError(kExactlyMaxPath));
 }
+
+TEST(IncludesNormalize, ShortRelativeButTooLongAbsolutePath) {
+  string result, err;
+  IncludesNormalize normalizer(".");
+  // A short path should work
+  EXPECT_TRUE(normalizer.Normalize("a", &result, &err));
+  EXPECT_EQ("", err);
+
+  // Construct max size path having cwd prefix.
+  // kExactlyMaxPath = "aaaa\\aaaa...aaaa\0";
+  char kExactlyMaxPath[_MAX_PATH + 1];
+  for (int i = 0; i < _MAX_PATH; ++i) {
+    if (i < _MAX_PATH - 1 && i % 10 == 4)
+      kExactlyMaxPath[i] = '\\';
+    else
+      kExactlyMaxPath[i] = 'a';
+  }
+  kExactlyMaxPath[_MAX_PATH] = '\0';
+  EXPECT_EQ(strlen(kExactlyMaxPath), _MAX_PATH);
+
+  // Make sure a path that's exactly _MAX_PATH long fails with a proper error.
+  EXPECT_FALSE(normalizer.Normalize(kExactlyMaxPath, &result, &err));
+  EXPECT_TRUE(err.find("GetFullPathName") != string::npos);
+}