includes_normalize: Fix long path support.

Do not use fixed-size buffers in order to properly
support long file paths. This should get rid of the failure
described at [1].

This leverages the Win32 Unicode/UTF-8 conversions functions
introduced in a previous commit to call GetFullPathnameW()
which is required here, since GetFullPathNameA() will fail
with long input paths, even when long path support is enabled!

[1] #2442 (comment)

Author: digit-google

src/includes_normalize-win32.cc

@@ -12,32 +12,99 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "includes_normalize.h"
-
-#include "string_piece.h"
-#include "string_piece_util.h"
-#include "util.h"
+#include <string.h>
+#include <windows.h>
 
 #include <algorithm>
 #include <iterator>
 #include <sstream>
 
-#include <windows.h>
+#include "includes_normalize.h"
+#include "string_piece.h"
+#include "string_piece_util.h"
+#include "util.h"
 
 namespace {
 
-bool InternalGetFullPathName(const StringPiece& file_name, char* buffer,
-                             size_t buffer_length, std::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();
+// Get the full path of a given filename. On success set |*path| and return
+// true. On failure, clear |path|, set |*err| then result false.
+bool InternalGetFullPathName(const StringPiece& file_name, std::string* path,
+                             std::string* err) {
+  // IMPORTANT: Using GetFullPathNameA() with a long paths will fail with
+  // "The filename or extension is too long" even if long path supported is
+  // enabled. GetFullPathNameW() must be used for this function to work!
+#if 1
+  path->clear();
+  // Convert to wide filename first.
+  std::string filename_str = file_name.AsString();
+  std::wstring wide_filename;
+  if (!ConvertUTF8ToWin32Unicode(filename_str, &wide_filename, err))
+    return false;
+
+  // Call GetFullPathNameW()
+  DWORD wide_full_size = GetFullPathNameW(wide_filename.c_str(), 0, NULL, NULL);
+  if (wide_full_size == 0) {
+    *err = "GetFullPathNameW(" +
+           std::string(wide_filename.begin(), wide_filename.end()) +
+           "): " + GetLastErrorString();
+    return false;
+  }
+
+  // NOTE: wide_full_size includes the null-terminating character.
+  std::wstring wide_path;
+  wide_path.resize(static_cast<size_t>(wide_full_size - 1));
+  DWORD wide_full_size2 =
+      GetFullPathNameW(wide_filename.c_str(), wide_full_size,
+                       const_cast<wchar_t*>(wide_path.data()), NULL);
+  if (wide_full_size2 == 0) {
+    *err = "GetFullPathNameA(" + filename_str + "): " + GetLastErrorString();
+    path->clear();
+    return false;
+  }
+
+  // Convert wide_path to Unicode.
+  return ConvertWin32UnicodeToUTF8(wide_path, path, err);
+#else
+  path->clear();
+  std::string filename_str = file_name.AsString();
+  DWORD full_size = GetFullPathNameA(filename_str.c_str(), 0, NULL, NULL);
+  if (full_size == 0) {
+    *err = "GetFullPathNameA(" + filename_str + "): " + GetLastErrorString();
     return false;
-  } else if (result_size > buffer_length) {
-    *err = "path too long";
+  }
+
+  // NOTE: full_size includes the null-terminating character.
+  path->resize(static_cast<size_t>(full_size - 1));
+  DWORD result2 = GetFullPathNameA(filename_str.c_str(), full_size,
+                                   const_cast<char*>(path->data()), NULL);
+  if (result2 == 0) {
+    *err = "GetFullPathNameA(" + filename_str + "): " + GetLastErrorString();
     return false;
   }
+
+  path->resize(static_cast<size_t>(result2));
+  return true;
+#endif
+}
+
+// Get the drive prefix of a given filename. On success set |*drive| then return
+// true. On failure, clear |*drive|, set |*err| then return false.
+bool InternalGetDrive(const StringPiece& file_name, std::string* drive,
+                      std::string* err) {
+  std::string path;
+  if (!InternalGetFullPathName(file_name, &path, err))
+    return false;
+
+  char drive_buffer[_MAX_DRIVE];
+  errno_t ret = _splitpath_s(path.data(), drive_buffer, sizeof(drive_buffer),
+                             NULL, 0, NULL, 0, NULL, 0);
+  if (ret != 0) {
+    *err = "_splitpath_s() returned " + std::string(strerror(ret)) +
+           " for path: " + path;
+    drive->clear();
+    return false;
+  }
+  drive->assign(drive_buffer);
   return true;
 }
 
@@ -74,19 +141,13 @@ bool SameDrive(StringPiece a, StringPiece b, std::string* err) {
     return true;
   }
 
-  char a_absolute[_MAX_PATH];
-  char b_absolute[_MAX_PATH];
-  if (!InternalGetFullPathName(a, a_absolute, sizeof(a_absolute), err)) {
-    return false;
-  }
-  if (!InternalGetFullPathName(b, b_absolute, sizeof(b_absolute), err)) {
+  std::string a_drive;
+  std::string b_drive;
+  if (!InternalGetDrive(a, &a_drive, err) ||
+      !InternalGetDrive(b, &b_drive, err)) {
     return false;
   }
-  char a_drive[_MAX_DIR];
-  char b_drive[_MAX_DIR];
-  _splitpath(a_absolute, a_drive, NULL, NULL, NULL);
-  _splitpath(b_absolute, b_drive, NULL, NULL, NULL);
-  return _stricmp(a_drive, b_drive) == 0;
+  return _stricmp(a_drive.c_str(), b_drive.c_str()) == 0;
 }
 
 // Check path |s| is FullPath style returned by GetFullPathName.
@@ -144,13 +205,14 @@ std::string IncludesNormalize::AbsPath(StringPiece s, std::string* err) {
     return result;
   }
 
-  char result[_MAX_PATH];
-  if (!InternalGetFullPathName(s, result, sizeof(result), err)) {
+  std::string result;
+  if (!InternalGetFullPathName(s, &result, err)) {
     return "";
   }
-  for (char* c = result; *c; ++c)
-    if (*c == '\\')
-      *c = '/';
+  for (char& c : result) {
+    if (c == '\\')
+      c = '/';
+  }
   return result;
 }
 
@@ -183,24 +245,17 @@ std::string IncludesNormalize::Relativize(
 
 bool IncludesNormalize::Normalize(const std::string& input, std::string* result,
                                   std::string* err) const {
-  char copy[_MAX_PATH + 1];
-  size_t len = input.size();
-  if (len > _MAX_PATH) {
-    *err = "path too long";
-    return false;
-  }
-  strncpy(copy, input.c_str(), input.size() + 1);
+  std::string copy = input;
   uint64_t slash_bits;
-  CanonicalizePath(copy, &len, &slash_bits);
-  StringPiece partially_fixed(copy, len);
-  std::string abs_input = AbsPath(partially_fixed, err);
+  CanonicalizePath(&copy, &slash_bits);
+  std::string abs_input = AbsPath(copy, err);
   if (!err->empty())
     return false;
 
   if (!SameDrive(abs_input, relative_to_, err)) {
     if (!err->empty())
       return false;
-    *result = partially_fixed.AsString();
+    *result = copy;
     return true;
   }
   *result = Relativize(abs_input, split_relative_to_, err);

src/includes_normalize_test.cc

@@ -24,11 +24,11 @@
 
 namespace {
 
-std::string GetCurDir() {
+std::string GetCurrentDirName() {
   char buf[_MAX_PATH];
   _getcwd(buf, sizeof(buf));
   std::vector<StringPiece> parts = SplitStringPiece(buf, '\\');
-  return parts[parts.size() - 1].AsString();
+  return parts.back().AsString();
 }
 
 std::string NormalizeAndCheckNoError(const std::string& input) {
@@ -59,7 +59,7 @@ TEST(IncludesNormalize, Simple) {
 
 TEST(IncludesNormalize, WithRelative) {
   std::string err;
-  std::string currentdir = GetCurDir();
+  std::string currentdir = GetCurrentDirName();
   EXPECT_EQ("c", NormalizeRelativeAndCheckNoError("a/b/c", "a/b"));
   EXPECT_EQ("a",
             NormalizeAndCheckNoError(IncludesNormalize::AbsPath("a", &err)));
@@ -107,10 +107,9 @@ TEST(IncludesNormalize, LongInvalidPath) {
   // Too long, won't be canonicalized. Ensure doesn't crash.
   std::string result, err;
   IncludesNormalize normalizer(".");
-  EXPECT_FALSE(
-      normalizer.Normalize(kLongInputString, &result, &err));
-  EXPECT_EQ("path too long", err);
-
+  EXPECT_TRUE(normalizer.Normalize(kLongInputString, &result, &err));
+  EXPECT_FALSE(result.empty());
+  EXPECT_TRUE(err.empty()) << err;
 
   // Construct max size path having cwd prefix.
   // kExactlyMaxPath = "$cwd\\a\\aaaa...aaaa\0";
@@ -142,7 +141,7 @@ TEST(IncludesNormalize, LongInvalidPath) {
             NormalizeAndCheckNoError(kExactlyMaxPath));
 }
 
-TEST(IncludesNormalize, ShortRelativeButTooLongAbsolutePath) {
+TEST(IncludesNormalize, ShortRelativeButLongAbsolutePath) {
   std::string result, err;
   IncludesNormalize normalizer(".");
   // A short path should work
@@ -162,6 +161,5 @@ TEST(IncludesNormalize, ShortRelativeButTooLongAbsolutePath) {
   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") != std::string::npos);
+  EXPECT_TRUE(normalizer.Normalize(kExactlyMaxPath, &result, &err)) << err;
 }