Skip to content

Commit

Permalink
Switch from getSegments() to containsUpLevelReferences() in `Arti…
Browse files Browse the repository at this point in the history
…factRoot#asDerivedRoot`.

`getSegments()` iterates over the path string twice and also creates garbage by taking substrings and putting them in a list. `containsUpLevelReferences()` takes constant time, as it does not need to look past the 3rd character of the already-normalized path string.

Additionally, clean up `PathFragment#containsUpLevelReferences` so that it does not perform string concatenation.

PiperOrigin-RevId: 357826526
  • Loading branch information
justinhorvitz authored and copybara-github committed Feb 17, 2021
1 parent 36de45a commit 2ccce39
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public static ArtifactRoot asDerivedRoot(
// Make sure that we are not creating a derived artifact under the execRoot.
Preconditions.checkArgument(!execPath.isEmpty(), "empty execPath");
Preconditions.checkArgument(
!execPath.getSegments().contains(".."),
!execPath.containsUplevelReferences(),
"execPath: %s contains parent directory reference (..)",
execPath);
Preconditions.checkArgument(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,8 @@ public PathFragment toRelative() {
*/
public boolean containsUplevelReferences() {
// Path is normalized, so any ".." would have to be the first segment.
return normalizedPath.equals("..") || normalizedPath.startsWith(".." + SEPARATOR_CHAR);
return normalizedPath.startsWith("..")
&& (normalizedPath.length() == 2 || normalizedPath.charAt(2) == SEPARATOR_CHAR);
}

/**
Expand Down

0 comments on commit 2ccce39

Please sign in to comment.