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
This commit is contained in:
parent
9ae2999dab
commit
bf9ed3fba2
2 changed files with 34 additions and 2 deletions
|
@ -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
|
||||||
|
|
|
@ -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",
|
||||||
|
|
Loading…
Reference in a new issue