From bc3af2c577b3be07001ec603d0ac1dd624df84c4 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Thu, 14 Jul 2022 07:11:13 -0700 Subject: [PATCH 1/2] Add factory for creating paths relative to well-known roots This change adds a factory for creating `PathFragments` relative to pre-defined (named) roots (e.g., relative to `%workspace%`). The syntax is choosen to match existing ad-hoc solutions for `%workspace%`, or `%builtins%` in other places (so that we can ideally migrate them in a follow-up). We'll use this for parsing paths from the command-line (e.g., `--credential_helper=%workspace%/foo`). Progress on https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md Closes #15805. PiperOrigin-RevId: 460950483 Change-Id: Ie263fb6d6c2ea938a850a72793d551135df6862e --- .../lib/runtime/CommandLinePathFactory.java | 113 ++++++++++ .../runtime/CommandLinePathFactoryTest.java | 203 ++++++++++++++++++ 2 files changed, 316 insertions(+) create mode 100644 src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java create mode 100644 src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java new file mode 100644 index 00000000000000..e39bf074b2d91c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java @@ -0,0 +1,113 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.runtime; + +import com.google.common.base.Preconditions; +import com.google.common.base.Splitter; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.Symlinks; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.util.Locale; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Factory for creating {@link PathFragment}s from command-line options. + * + *

The difference between this and using {@link PathFragment#create(String)} directly is that + * this factory replaces values starting with {@code %%} with the corresponding (named) roots + * (e.g., {@code %workspace%/foo} becomes {@code /foo}). + */ +public final class CommandLinePathFactory { + private static final Pattern REPLACEMENT_PATTERN = Pattern.compile("^(%([a-z_]+)%/)?([^%].*)$"); + + private static final Splitter PATH_SPLITTER = Splitter.on(File.pathSeparator); + + private final FileSystem fileSystem; + private final ImmutableMap roots; + + public CommandLinePathFactory(FileSystem fileSystem, ImmutableMap roots) { + this.fileSystem = Preconditions.checkNotNull(fileSystem); + this.roots = Preconditions.checkNotNull(roots); + } + + /** Creates a {@link Path}. */ + public Path create(Map env, String value) throws IOException { + Preconditions.checkNotNull(env); + Preconditions.checkNotNull(value); + + Matcher matcher = REPLACEMENT_PATTERN.matcher(value); + Preconditions.checkArgument(matcher.matches()); + + String rootName = matcher.group(2); + PathFragment path = PathFragment.create(matcher.group(3)); + if (path.containsUplevelReferences()) { + throw new IllegalArgumentException( + String.format( + Locale.US, "Path must not contain any uplevel references ('..'), got '%s'", value)); + } + + // Case 1: `path` is relative to a well-known root. + if (!Strings.isNullOrEmpty(rootName)) { + // The regex above cannot check that `value` is not of form `%foo%//abc` (group 2 will be + // `foo` and group 3 will be `/abc`). + Preconditions.checkArgument(!path.isAbsolute()); + + Path root = roots.get(rootName); + if (root == null) { + throw new IllegalArgumentException(String.format(Locale.US, "Unknown root %s", rootName)); + } + return root.getRelative(path); + } + + // Case 2: `value` is an absolute path. + if (path.isAbsolute()) { + return fileSystem.getPath(path); + } + + // Case 3: `value` is a relative path. + // + // Since relative paths from the command-line are ambiguous to where they are relative to (i.e., + // relative to the workspace?, the directory Bazel is running in? relative to the `.bazelrc` the + // flag is from?), we only allow relative paths with a single segment (i.e., no `/`) and treat + // it as relative to the user's `PATH`. + if (path.segmentCount() > 1) { + throw new IllegalArgumentException( + "Path must either be absolute or not contain any path separators"); + } + + String pathVariable = env.getOrDefault("PATH", ""); + if (!Strings.isNullOrEmpty(pathVariable)) { + for (String lookupPath : PATH_SPLITTER.split(pathVariable)) { + Path maybePath = fileSystem.getPath(lookupPath).getRelative(path); + if (maybePath.exists(Symlinks.FOLLOW) + && maybePath.isFile(Symlinks.FOLLOW) + && maybePath.isExecutable()) { + return maybePath; + } + } + } + + throw new FileNotFoundException( + String.format( + Locale.US, "Could not find file with name '%s' on PATH '%s'", path, pathVariable)); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java b/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java new file mode 100644 index 00000000000000..58276e928c146f --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java @@ -0,0 +1,203 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.runtime; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.OutputStream; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link CommandLinePathFactory}. */ +@RunWith(JUnit4.class) +public class CommandLinePathFactoryTest { + private static final Joiner PATH_JOINER = Joiner.on(File.pathSeparator); + + private FileSystem filesystem = null; + + @Before + public void prepareFilesystem() throws Exception { + filesystem = new InMemoryFileSystem(DigestHashFunction.SHA256); + } + + private void createExecutable(String path) throws Exception { + Preconditions.checkNotNull(path); + + createExecutable(filesystem.getPath(path)); + } + + private void createExecutable(Path path) throws Exception { + Preconditions.checkNotNull(path); + + path.getParentDirectory().createDirectoryAndParents(); + try (OutputStream stream = path.getOutputStream()) { + // Just create an empty file, nothing to do. + } + path.setExecutable(true); + } + + @Test + public void emptyPathIsRejected() { + CommandLinePathFactory factory = new CommandLinePathFactory(filesystem, ImmutableMap.of()); + + assertThrows(IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "")); + } + + @Test + public void createFromAbsolutePath() throws Exception { + CommandLinePathFactory factory = new CommandLinePathFactory(filesystem, ImmutableMap.of()); + + assertThat(factory.create(ImmutableMap.of(), "/absolute/path/1")) + .isEqualTo(filesystem.getPath("/absolute/path/1")); + assertThat(factory.create(ImmutableMap.of(), "/absolute/path/2")) + .isEqualTo(filesystem.getPath("/absolute/path/2")); + } + + @Test + public void createWithNamedRoot() throws Exception { + CommandLinePathFactory factory = + new CommandLinePathFactory( + filesystem, + ImmutableMap.of( + "workspace", filesystem.getPath("/path/to/workspace"), + "output_base", filesystem.getPath("/path/to/output/base"))); + + assertThat(factory.create(ImmutableMap.of(), "/absolute/path/1")) + .isEqualTo(filesystem.getPath("/absolute/path/1")); + assertThat(factory.create(ImmutableMap.of(), "/absolute/path/2")) + .isEqualTo(filesystem.getPath("/absolute/path/2")); + + assertThat(factory.create(ImmutableMap.of(), "%workspace%/foo")) + .isEqualTo(filesystem.getPath("/path/to/workspace/foo")); + assertThat(factory.create(ImmutableMap.of(), "%workspace%/foo/bar")) + .isEqualTo(filesystem.getPath("/path/to/workspace/foo/bar")); + + assertThat(factory.create(ImmutableMap.of(), "%output_base%/foo")) + .isEqualTo(filesystem.getPath("/path/to/output/base/foo")); + assertThat(factory.create(ImmutableMap.of(), "%output_base%/foo/bar")) + .isEqualTo(filesystem.getPath("/path/to/output/base/foo/bar")); + } + + @Test + public void pathLeakingOutsideOfRoot() { + CommandLinePathFactory factory = + new CommandLinePathFactory( + filesystem, ImmutableMap.of("a", filesystem.getPath("/path/to/a"))); + + assertThrows( + IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "%a%/../foo")); + assertThrows( + IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "%a%/b/../..")); + } + + @Test + public void unknownRoot() { + CommandLinePathFactory factory = + new CommandLinePathFactory( + filesystem, ImmutableMap.of("a", filesystem.getPath("/path/to/a"))); + + assertThrows( + IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "%workspace%/foo")); + assertThrows( + IllegalArgumentException.class, + () -> factory.create(ImmutableMap.of(), "%output_base%/foo")); + } + + @Test + public void rootWithDoubleSlash() { + CommandLinePathFactory factory = + new CommandLinePathFactory( + filesystem, ImmutableMap.of("a", filesystem.getPath("/path/to/a"))); + + assertThrows( + IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "%a%//foo")); + } + + @Test + public void relativePathWithMultipleSegments() { + CommandLinePathFactory factory = new CommandLinePathFactory(filesystem, ImmutableMap.of()); + + assertThrows(IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "a/b")); + assertThrows( + IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "a/b/c/d")); + } + + @Test + public void pathLookup() throws Exception { + CommandLinePathFactory factory = new CommandLinePathFactory(filesystem, ImmutableMap.of()); + + createExecutable("/bin/true"); + createExecutable("/bin/false"); + createExecutable("/usr/bin/foo-bar.exe"); + createExecutable("/usr/local/bin/baz"); + createExecutable("/home/yannic/bin/abc"); + createExecutable("/home/yannic/bin/true"); + + var path = + ImmutableMap.of( + "PATH", PATH_JOINER.join("/bin", "/usr/bin", "/usr/local/bin", "/home/yannic/bin")); + assertThat(factory.create(path, "true")).isEqualTo(filesystem.getPath("/bin/true")); + assertThat(factory.create(path, "false")).isEqualTo(filesystem.getPath("/bin/false")); + assertThat(factory.create(path, "foo-bar.exe")) + .isEqualTo(filesystem.getPath("/usr/bin/foo-bar.exe")); + assertThat(factory.create(path, "baz")).isEqualTo(filesystem.getPath("/usr/local/bin/baz")); + assertThat(factory.create(path, "abc")).isEqualTo(filesystem.getPath("/home/yannic/bin/abc")); + + // `.exe` is required. + assertThrows(FileNotFoundException.class, () -> factory.create(path, "foo-bar")); + } + + @Test + public void pathLookupWithUndefinedPath() { + CommandLinePathFactory factory = new CommandLinePathFactory(filesystem, ImmutableMap.of()); + + assertThrows(FileNotFoundException.class, () -> factory.create(ImmutableMap.of(), "a")); + assertThrows(FileNotFoundException.class, () -> factory.create(ImmutableMap.of(), "foo")); + } + + @Test + public void pathLookupWithNonExistingDirectoryOnPath() { + CommandLinePathFactory factory = new CommandLinePathFactory(filesystem, ImmutableMap.of()); + + assertThrows( + FileNotFoundException.class, + () -> factory.create(ImmutableMap.of("PATH", "/does/not/exist"), "a")); + } + + @Test + public void pathLookupWithExistingAndNonExistingDirectoryOnPath() throws Exception { + CommandLinePathFactory factory = new CommandLinePathFactory(filesystem, ImmutableMap.of()); + + createExecutable("/bin/foo"); + createExecutable("/usr/bin/bar"); + assertThrows( + FileNotFoundException.class, + () -> + factory.create( + ImmutableMap.of("PATH", PATH_JOINER.join("/bin", "/does/not/exist", "/usr/bin")), + "a")); + } +} From 21833ade2e05eb1adaafbaa42ae55a3fb2f88df2 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Wed, 20 Jul 2022 14:14:55 +0200 Subject: [PATCH 2/2] Remove use of var --- .../lib/runtime/CommandLinePathFactoryTest.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java b/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java index 58276e928c146f..223f39b7709923 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java @@ -156,18 +156,18 @@ public void pathLookup() throws Exception { createExecutable("/home/yannic/bin/abc"); createExecutable("/home/yannic/bin/true"); - var path = + ImmutableMap env = ImmutableMap.of( "PATH", PATH_JOINER.join("/bin", "/usr/bin", "/usr/local/bin", "/home/yannic/bin")); - assertThat(factory.create(path, "true")).isEqualTo(filesystem.getPath("/bin/true")); - assertThat(factory.create(path, "false")).isEqualTo(filesystem.getPath("/bin/false")); - assertThat(factory.create(path, "foo-bar.exe")) + assertThat(factory.create(env, "true")).isEqualTo(filesystem.getPath("/bin/true")); + assertThat(factory.create(env, "false")).isEqualTo(filesystem.getPath("/bin/false")); + assertThat(factory.create(env, "foo-bar.exe")) .isEqualTo(filesystem.getPath("/usr/bin/foo-bar.exe")); - assertThat(factory.create(path, "baz")).isEqualTo(filesystem.getPath("/usr/local/bin/baz")); - assertThat(factory.create(path, "abc")).isEqualTo(filesystem.getPath("/home/yannic/bin/abc")); + assertThat(factory.create(env, "baz")).isEqualTo(filesystem.getPath("/usr/local/bin/baz")); + assertThat(factory.create(env, "abc")).isEqualTo(filesystem.getPath("/home/yannic/bin/abc")); // `.exe` is required. - assertThrows(FileNotFoundException.class, () -> factory.create(path, "foo-bar")); + assertThrows(FileNotFoundException.class, () -> factory.create(env, "foo-bar")); } @Test