Merge "Optimize out empty path components" into main

This commit is contained in:
Treehugger Robot 2023-11-02 22:36:11 +00:00 committed by Gerrit Code Review
commit c012b631ae
2 changed files with 34 additions and 2 deletions

View file

@ -1915,7 +1915,9 @@ func (p InstallPaths) Strings() []string {
// validatePathInternal ensures that a path does not leave its component, and // validatePathInternal ensures that a path does not leave its component, and
// optionally doesn't contain Ninja variables. // optionally doesn't contain Ninja variables.
func validatePathInternal(allowNinjaVariables bool, pathComponents ...string) (string, error) { 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, "$") { if !allowNinjaVariables && strings.Contains(path, "$") {
return "", fmt.Errorf("Path contains invalid character($): %s", 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, "/") { if path == ".." || strings.HasPrefix(path, "../") || strings.HasPrefix(path, "/") {
return "", fmt.Errorf("Path is outside directory: %s", 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 // TODO: filepath.Join isn't necessarily correct with embedded ninja
// variables. '..' may remove the entire ninja variable, even if it // variables. '..' may remove the entire ninja variable, even if it
// will be expanded to multiple nested directories. // 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 // validateSafePath validates a path that we trust (may contain ninja

View file

@ -36,6 +36,22 @@ var commonValidatePathTestCases = []strsTestCase{
in: []string{""}, in: []string{""},
out: "", out: "",
}, },
{
in: []string{"", ""},
out: "",
},
{
in: []string{"a", ""},
out: "a",
},
{
in: []string{"", "a"},
out: "a",
},
{
in: []string{"", "a", ""},
out: "a",
},
{ {
in: []string{"a/b"}, in: []string{"a/b"},
out: "a/b", out: "a/b",