Verify GetFullPathName return value
GetFullPathName previously failed silently on long path names resulting
in uninitialized path result.
Signed-off-by: Fredrik Medley <fredrik.medley@autoliv.com>
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 @@
}
// 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 @@
} // 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 @@
}
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::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 @@
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 @@
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, 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 @@
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);
+}
