[GN] Add support to rebase_path to resolve paths above the source root.
Currently, rebase_path clips any path which references the filesystem
above or outside of the source root. Extend this functionality to allow
rebase_path to properly resolve paths outside the source root when the
absolute path to the source root is known. When the source root is
unknown, the behavior will continue to clip.
BUG=554241
TEST=gn_unittests
Review URL: https://codereview.chromium.org/1455203002
Cr-Original-Commit-Position: refs/heads/master@{#363080}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 910ac443ab1827eae049ae052af867d432ad7aac
diff --git a/tools/gn/filesystem_utils.cc b/tools/gn/filesystem_utils.cc
index a3455fd..5905cdb 100644
--- a/tools/gn/filesystem_utils.cc
+++ b/tools/gn/filesystem_utils.cc
@@ -377,7 +377,7 @@
#endif
}
-void NormalizePath(std::string* path) {
+void NormalizePath(std::string* path, const base::StringPiece& source_root) {
char* pathbuf = path->empty() ? nullptr : &(*path)[0];
// top_index is the first character we can modify in the path. Anything
@@ -433,9 +433,48 @@
// up more levels. Otherwise "../.." would collapse to
// nothing.
top_index = dest_i;
+ } else if (top_index == 2 && !source_root.empty()) {
+ // |path| was passed in as a source-absolute path. Prepend
+ // |source_root| to make |path| absolute. |source_root| must not
+ // end with a slash unless we are at root.
+ DCHECK(source_root.size() == 1u ||
+ !IsSlash(source_root[source_root.size() - 1u]));
+ size_t source_root_len = source_root.size();
+
+#if defined(OS_WIN)
+ // On Windows, if the source_root does not start with a slash,
+ // append one here for consistency.
+ if (!IsSlash(source_root[0])) {
+ path->insert(0, "/" + source_root.as_string());
+ source_root_len++;
+ } else {
+ path->insert(0, source_root.data(), source_root_len);
+ }
+
+ // Normalize slashes in source root portion.
+ for (size_t i = 0; i < source_root_len; ++i) {
+ if ((*path)[i] == '\\')
+ (*path)[i] = '/';
+ }
+#else
+ path->insert(0, source_root.data(), source_root_len);
+#endif
+
+ // |path| is now absolute, so |top_index| is 1. |dest_i| and
+ // |src_i| should be incremented to keep the same relative
+ // position. Comsume the leading "//" by decrementing |dest_i|.
+ top_index = 1;
+ pathbuf = &(*path)[0];
+ dest_i += source_root_len - 2;
+ src_i += source_root_len;
+
+ // Just find the previous slash or the beginning of input.
+ while (dest_i > 0 && !IsSlash(pathbuf[dest_i - 1]))
+ dest_i--;
}
- // Otherwise we're at the beginning of an absolute path. Don't
- // allow ".." to go up another level and just eat it.
+ // Otherwise we're at the beginning of a system-absolute path, or
+ // a source-absolute path for which we don't know the absolute
+ // path. Don't allow ".." to go up another level, and just eat it.
} else {
// Just find the previous slash or the beginning of input.
while (dest_i > 0 && !IsSlash(pathbuf[dest_i - 1]))
diff --git a/tools/gn/filesystem_utils.h b/tools/gn/filesystem_utils.h
index 4d71ca7..e6a17ab 100644
--- a/tools/gn/filesystem_utils.h
+++ b/tools/gn/filesystem_utils.h
@@ -111,8 +111,15 @@
const base::StringPiece& path,
std::string* dest);
-// Collapses "." and sequential "/"s and evaluates "..".
-void NormalizePath(std::string* path);
+// Collapses "." and sequential "/"s and evaluates "..". |path| may be
+// system-absolute, source-absolute, or relative. If |path| is source-absolute
+// and |source_root| is non-empty, |path| may be system absolute after this
+// function returns, if |path| references the filesystem outside of
+// |source_root| (ex. path = "//.."). In this case on Windows, |path| will have
+// a leading slash. Otherwise, |path| will retain its relativity. |source_root|
+// must not end with a slash.
+void NormalizePath(std::string* path,
+ const base::StringPiece& source_root = base::StringPiece());
// Converts slashes to backslashes for Windows. Keeps the string unchanged
// for other systems.
diff --git a/tools/gn/filesystem_utils_unittest.cc b/tools/gn/filesystem_utils_unittest.cc
index 9f89512..27ccab2 100644
--- a/tools/gn/filesystem_utils_unittest.cc
+++ b/tools/gn/filesystem_utils_unittest.cc
@@ -204,7 +204,7 @@
NormalizePath(&input);
EXPECT_EQ("../bar", input);
- input = "/../foo"; // Don't go aboe the root dir.
+ input = "/../foo"; // Don't go above the root dir.
NormalizePath(&input);
EXPECT_EQ("/foo", input);
@@ -241,6 +241,134 @@
input = "//foo/bar/";
NormalizePath(&input);
EXPECT_EQ("//foo/bar/", input);
+
+#if defined(OS_WIN)
+ // Go above and outside of the source root.
+ input = "//../foo";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("/C:/source/foo", input);
+
+ input = "//../foo";
+ NormalizePath(&input, "C:\\source\\root");
+ EXPECT_EQ("/C:/source/foo", input);
+
+ input = "//../";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("/C:/source/", input);
+
+ input = "//../foo.txt";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("/C:/source/foo.txt", input);
+
+ input = "//../foo/bar/";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("/C:/source/foo/bar/", input);
+
+ // Go above and back into the source root. This should return a system-
+ // absolute path. We could arguably return this as a source-absolute path,
+ // but that would require additional handling to account for a rare edge
+ // case.
+ input = "//../root/foo";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("/C:/source/root/foo", input);
+
+ input = "//../root/foo/bar/";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("/C:/source/root/foo/bar/", input);
+
+ // Stay inside the source root
+ input = "//foo/bar";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("//foo/bar", input);
+
+ input = "//foo/bar/";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("//foo/bar/", input);
+
+ // The path should not go above the system root. Note that on Windows, this
+ // will consume the drive (C:).
+ input = "//../../../../../foo/bar";
+ NormalizePath(&input, "/C:/source/root");
+ EXPECT_EQ("/foo/bar", input);
+
+ // Test when the source root is the letter drive.
+ input = "//../foo";
+ NormalizePath(&input, "/C:");
+ EXPECT_EQ("/foo", input);
+
+ input = "//../foo";
+ NormalizePath(&input, "C:");
+ EXPECT_EQ("/foo", input);
+
+ input = "//../foo";
+ NormalizePath(&input, "/");
+ EXPECT_EQ("/foo", input);
+
+ input = "//../";
+ NormalizePath(&input, "\\C:");
+ EXPECT_EQ("/", input);
+
+ input = "//../foo.txt";
+ NormalizePath(&input, "/C:");
+ EXPECT_EQ("/foo.txt", input);
+#else
+ // Go above and outside of the source root.
+ input = "//../foo";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("/source/foo", input);
+
+ input = "//../";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("/source/", input);
+
+ input = "//../foo.txt";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("/source/foo.txt", input);
+
+ input = "//../foo/bar/";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("/source/foo/bar/", input);
+
+ // Go above and back into the source root. This should return a system-
+ // absolute path. We could arguably return this as a source-absolute path,
+ // but that would require additional handling to account for a rare edge
+ // case.
+ input = "//../root/foo";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("/source/root/foo", input);
+
+ input = "//../root/foo/bar/";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("/source/root/foo/bar/", input);
+
+ // Stay inside the source root
+ input = "//foo/bar";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("//foo/bar", input);
+
+ input = "//foo/bar/";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("//foo/bar/", input);
+
+ // The path should not go above the system root.
+ input = "//../../../../../foo/bar";
+ NormalizePath(&input, "/source/root");
+ EXPECT_EQ("/foo/bar", input);
+
+ // Test when the source root is the system root.
+ input = "//../foo/bar/";
+ NormalizePath(&input, "/");
+ EXPECT_EQ("/foo/bar/", input);
+
+ input = "//../";
+ NormalizePath(&input, "/");
+ EXPECT_EQ("/", input);
+
+ input = "//../foo.txt";
+ NormalizePath(&input, "/");
+ EXPECT_EQ("/foo.txt", input);
+
+#endif
}
TEST(FilesystemUtils, RebasePath) {
diff --git a/tools/gn/function_rebase_path_unittest.cc b/tools/gn/function_rebase_path_unittest.cc
index bed5663..456d4fb 100644
--- a/tools/gn/function_rebase_path_unittest.cc
+++ b/tools/gn/function_rebase_path_unittest.cc
@@ -43,7 +43,7 @@
EXPECT_EQ("../..", RebaseOne(scope, "../..", "//out/Debug", "."));
EXPECT_EQ("../../", RebaseOne(scope, "../../", "//out/Debug", "."));
- // We don't allow going above the root source dir.
+ // Without a source root defined, we cannot move out of the source tree.
EXPECT_EQ("../..", RebaseOne(scope, "../../..", "//out/Debug", "."));
// Source-absolute input paths.
@@ -62,19 +62,36 @@
// Test system path output.
#if defined(OS_WIN)
- setup.build_settings()->SetRootPath(base::FilePath(L"C:/source"));
- EXPECT_EQ("C:/source", RebaseOne(scope, ".", "", "//"));
- EXPECT_EQ("C:/source/", RebaseOne(scope, "//", "", "//"));
- EXPECT_EQ("C:/source/foo", RebaseOne(scope, "foo", "", "//"));
- EXPECT_EQ("C:/source/foo/", RebaseOne(scope, "foo/", "", "//"));
- EXPECT_EQ("C:/source/tools/gn/foo", RebaseOne(scope, "foo", "", "."));
+ setup.build_settings()->SetRootPath(base::FilePath(L"C:/path/to/src"));
+ EXPECT_EQ("C:/path/to/src", RebaseOne(scope, ".", "", "//"));
+ EXPECT_EQ("C:/path/to/src/", RebaseOne(scope, "//", "", "//"));
+ EXPECT_EQ("C:/path/to/src/foo", RebaseOne(scope, "foo", "", "//"));
+ EXPECT_EQ("C:/path/to/src/foo/", RebaseOne(scope, "foo/", "", "//"));
+ EXPECT_EQ("C:/path/to/src/tools/gn/foo", RebaseOne(scope, "foo", "", "."));
+ EXPECT_EQ("C:/path/to/other/tools",
+ RebaseOne(scope, "//../other/tools", "", "//"));
+ EXPECT_EQ("C:/path/to/src/foo/bar",
+ RebaseOne(scope, "//../src/foo/bar", "", "//"));
+ EXPECT_EQ("C:/path/to", RebaseOne(scope, "//..", "", "//"));
+ EXPECT_EQ("C:/path", RebaseOne(scope, "../../../..", "", "."));
+ EXPECT_EQ("C:/path/to/external/dir/",
+ RebaseOne(scope, "//../external/dir/", "", "//"));
+
#else
- setup.build_settings()->SetRootPath(base::FilePath("/source"));
- EXPECT_EQ("/source", RebaseOne(scope, ".", "", "//"));
- EXPECT_EQ("/source/", RebaseOne(scope, "//", "", "//"));
- EXPECT_EQ("/source/foo", RebaseOne(scope, "foo", "", "//"));
- EXPECT_EQ("/source/foo/", RebaseOne(scope, "foo/", "", "//"));
- EXPECT_EQ("/source/tools/gn/foo", RebaseOne(scope, "foo", "", "."));
+ setup.build_settings()->SetRootPath(base::FilePath("/path/to/src"));
+ EXPECT_EQ("/path/to/src", RebaseOne(scope, ".", "", "//"));
+ EXPECT_EQ("/path/to/src/", RebaseOne(scope, "//", "", "//"));
+ EXPECT_EQ("/path/to/src/foo", RebaseOne(scope, "foo", "", "//"));
+ EXPECT_EQ("/path/to/src/foo/", RebaseOne(scope, "foo/", "", "//"));
+ EXPECT_EQ("/path/to/src/tools/gn/foo", RebaseOne(scope, "foo", "", "."));
+ EXPECT_EQ("/path/to/other/tools",
+ RebaseOne(scope, "//../other/tools", "", "//"));
+ EXPECT_EQ("/path/to/src/foo/bar",
+ RebaseOne(scope, "//../src/foo/bar", "", "//"));
+ EXPECT_EQ("/path/to", RebaseOne(scope, "//..", "", "//"));
+ EXPECT_EQ("/path", RebaseOne(scope, "../../../..", "", "."));
+ EXPECT_EQ("/path/to/external/dir/",
+ RebaseOne(scope, "//../external/dir/", "", "//"));
#endif
}
diff --git a/tools/gn/source_dir.cc b/tools/gn/source_dir.cc
index 0966cc6..d9fb4cc 100644
--- a/tools/gn/source_dir.cc
+++ b/tools/gn/source_dir.cc
@@ -71,7 +71,7 @@
if (str.size() >= 2 && str[0] == '/' && str[1] == '/') {
// Source-relative.
ret.value_.assign(str.data(), str.size());
- NormalizePath(&ret.value_);
+ NormalizePath(&ret.value_, source_root);
return ret;
} else if (IsPathAbsolute(str)) {
if (source_root.empty() ||
@@ -145,7 +145,7 @@
ret.value_.assign(str.data(), str.size());
if (!EndsWithSlash(ret.value_))
ret.value_.push_back('/');
- NormalizePath(&ret.value_);
+ NormalizePath(&ret.value_, source_root);
return ret;
} else if (IsPathAbsolute(str)) {
if (source_root.empty() ||
diff --git a/tools/gn/source_dir_unittest.cc b/tools/gn/source_dir_unittest.cc
index 04684e6..0c73865 100644
--- a/tools/gn/source_dir_unittest.cc
+++ b/tools/gn/source_dir_unittest.cc
@@ -61,11 +61,41 @@
Value(nullptr, "../../foo"), &err, source_root) ==
SourceFile("/C:/source/foo"));
EXPECT_FALSE(err.has_error());
+
+ EXPECT_TRUE(base.ResolveRelativeFile(
+ Value(nullptr, "//../foo"), &err, source_root) ==
+ SourceFile("/C:/source/foo"));
+ EXPECT_FALSE(err.has_error());
+
+ EXPECT_TRUE(base.ResolveRelativeFile(
+ Value(nullptr, "//../root/foo"), &err, source_root) ==
+ SourceFile("/C:/source/root/foo"));
+ EXPECT_FALSE(err.has_error());
+
+ EXPECT_TRUE(base.ResolveRelativeFile(
+ Value(nullptr, "//../../../foo/bar"), &err, source_root) ==
+ SourceFile("/foo/bar"));
+ EXPECT_FALSE(err.has_error());
#else
EXPECT_TRUE(base.ResolveRelativeFile(
Value(nullptr, "../../foo"), &err, source_root) ==
SourceFile("/source/foo"));
EXPECT_FALSE(err.has_error());
+
+ EXPECT_TRUE(base.ResolveRelativeFile(
+ Value(nullptr, "//../foo"), &err, source_root) ==
+ SourceFile("/source/foo"));
+ EXPECT_FALSE(err.has_error());
+
+ EXPECT_TRUE(base.ResolveRelativeFile(
+ Value(nullptr, "//../root/foo"), &err, source_root) ==
+ SourceFile("/source/root/foo"));
+ EXPECT_FALSE(err.has_error());
+
+ EXPECT_TRUE(base.ResolveRelativeFile(
+ Value(nullptr, "//../../../foo/bar"), &err, source_root) ==
+ SourceFile("/foo/bar"));
+ EXPECT_FALSE(err.has_error());
#endif
#if defined(OS_WIN)
@@ -120,11 +150,27 @@
Value(nullptr, "../../foo"), &err, source_root) ==
SourceDir("/C:/source/foo/"));
EXPECT_FALSE(err.has_error());
+ EXPECT_TRUE(base.ResolveRelativeDir(
+ Value(nullptr, "//../foo"), &err, source_root) ==
+ SourceDir("/C:/source/foo/"));
+ EXPECT_FALSE(err.has_error());
+ EXPECT_TRUE(base.ResolveRelativeDir(
+ Value(nullptr, "//.."), &err, source_root) ==
+ SourceDir("/C:/source/"));
+ EXPECT_FALSE(err.has_error());
#else
EXPECT_TRUE(base.ResolveRelativeDir(
Value(nullptr, "../../foo"), &err, source_root) ==
SourceDir("/source/foo/"));
EXPECT_FALSE(err.has_error());
+ EXPECT_TRUE(base.ResolveRelativeDir(
+ Value(nullptr, "//../foo"), &err, source_root) ==
+ SourceDir("/source/foo/"));
+ EXPECT_FALSE(err.has_error());
+ EXPECT_TRUE(base.ResolveRelativeDir(
+ Value(nullptr, "//.."), &err, source_root) ==
+ SourceDir("/source/"));
+ EXPECT_FALSE(err.has_error());
#endif
#if defined(OS_WIN)
