From bf9ed3fba2b27d0067f0701ca4396140c1d522eb Mon Sep 17 00:00:00 2001
From: Colin Cross <ccross@android.com>
Date: Tue, 24 Oct 2023 14:17:03 -0700
Subject: [PATCH] Optimize out empty path components

filepath.Join("foo", "") returns a newly allocated copy of "foo",
while filepath.Join("foo") does not.  Strip out any empty path
components before calling filepath.Join.

Test: TestValidatePath
Change-Id: Ib47dbcd9d6463809acfe260dfd9af87ea280b4de
---
 android/paths.go      | 20 ++++++++++++++++++--
 android/paths_test.go | 16 ++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/android/paths.go b/android/paths.go
index 8dd19668c..a6cda38f0 100644
--- a/android/paths.go
+++ b/android/paths.go
@@ -1915,7 +1915,9 @@ func (p InstallPaths) Strings() []string {
 // validatePathInternal ensures that a path does not leave its component, and
 // optionally doesn't contain Ninja variables.
 func validatePathInternal(allowNinjaVariables bool, pathComponents ...string) (string, error) {
-	for _, path := range pathComponents {
+	initialEmpty := 0
+	finalEmpty := 0
+	for i, path := range pathComponents {
 		if !allowNinjaVariables && strings.Contains(path, "$") {
 			return "", fmt.Errorf("Path contains invalid character($): %s", path)
 		}
@@ -1924,11 +1926,25 @@ func validatePathInternal(allowNinjaVariables bool, pathComponents ...string) (s
 		if path == ".." || strings.HasPrefix(path, "../") || strings.HasPrefix(path, "/") {
 			return "", fmt.Errorf("Path is outside directory: %s", path)
 		}
+
+		if i == initialEmpty && pathComponents[i] == "" {
+			initialEmpty++
+		}
+		if i == finalEmpty && pathComponents[len(pathComponents)-1-i] == "" {
+			finalEmpty++
+		}
 	}
+	// Optimization: filepath.Join("foo", "") returns a newly allocated copy
+	// of "foo", while filepath.Join("foo") does not.  Strip out any empty
+	// path components.
+	if initialEmpty == len(pathComponents) {
+		return "", nil
+	}
+	nonEmptyPathComponents := pathComponents[initialEmpty : len(pathComponents)-finalEmpty]
 	// TODO: filepath.Join isn't necessarily correct with embedded ninja
 	// variables. '..' may remove the entire ninja variable, even if it
 	// will be expanded to multiple nested directories.
-	return filepath.Join(pathComponents...), nil
+	return filepath.Join(nonEmptyPathComponents...), nil
 }
 
 // validateSafePath validates a path that we trust (may contain ninja
diff --git a/android/paths_test.go b/android/paths_test.go
index 2f87977a9..bf46c34f3 100644
--- a/android/paths_test.go
+++ b/android/paths_test.go
@@ -36,6 +36,22 @@ var commonValidatePathTestCases = []strsTestCase{
 		in:  []string{""},
 		out: "",
 	},
+	{
+		in:  []string{"", ""},
+		out: "",
+	},
+	{
+		in:  []string{"a", ""},
+		out: "a",
+	},
+	{
+		in:  []string{"", "a"},
+		out: "a",
+	},
+	{
+		in:  []string{"", "a", ""},
+		out: "a",
+	},
 	{
 		in:  []string{"a/b"},
 		out: "a/b",