Make path-fragment-base-name checking better on Windows: don't allow …

…'\' in the name.

PiperOrigin-RevId: 341823807

src/main/java/com/google/devtools/build/lib/vfs/OsPathPolicy.java

@@ -75,12 +75,15 @@ public interface OsPathPolicy {
    */
   boolean endsWith(String path, String suffix);
 
-  /** Returns the separator used for normalized paths. */
-  char getSeparator();
-
   /** Returns whether the unnormalized character c is a separator. */
   boolean isSeparator(char c);
 
+  /**
+   * Returns an additional character besides '/' for which {@link #isSeparator} is true. 0 means
+   * there is no such additional character.
+   */
+  char additionalSeparator();
+
   boolean isCaseSensitive();
 
   static OsPathPolicy getFilePathOs() {

src/main/java/com/google/devtools/build/lib/vfs/Path.java

@@ -74,7 +74,7 @@ public static FileSystem getFileSystemForSerialization() {
   }
 
   private static final OsPathPolicy OS = OsPathPolicy.getFilePathOs();
-  private static final char SEPARATOR = OS.getSeparator();
+  private static final char SEPARATOR = '/';
 
   private String path;
   private int driveStrLength; // 1 on Unix, 3 on Windows

src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java

@@ -55,7 +55,8 @@ public final class PathFragment
   private static final OsPathPolicy OS = OsPathPolicy.getFilePathOs();
 
   @SerializationConstant public static final PathFragment EMPTY_FRAGMENT = new PathFragment("", 0);
-  public static final char SEPARATOR_CHAR = OS.getSeparator();
+  public static final char SEPARATOR_CHAR = '/';
+  private static final char ADDITIONAL_SEPARATOR_CHAR = OS.additionalSeparator();
   public static final int INVALID_SEGMENT = -1;
 
   private final String normalizedPath;
@@ -739,8 +740,15 @@ private static void checkBaseName(String baseName) {
     if (baseName.equals(".") || baseName.equals("..")) {
       throw new IllegalArgumentException("baseName must not be '" + baseName + "'");
     }
-    if (baseName.indexOf('/') != -1) {
-      throw new IllegalArgumentException("baseName must not contain a slash: '" + baseName + "'");
+    if (baseName.indexOf(SEPARATOR_CHAR) != -1) {
+      throw new IllegalArgumentException(
+          "baseName must not contain " + SEPARATOR_CHAR + ": '" + baseName + "'");
+    }
+    if (ADDITIONAL_SEPARATOR_CHAR != 0) {
+      if (baseName.indexOf(ADDITIONAL_SEPARATOR_CHAR) != -1) {
+        throw new IllegalArgumentException(
+            "baseName must not contain " + ADDITIONAL_SEPARATOR_CHAR + ": '" + baseName + "'");
+      }
     }
   }
 

src/main/java/com/google/devtools/build/lib/vfs/UnixOsPathPolicy.java

@@ -122,13 +122,13 @@ public boolean endsWith(String path, String suffix) {
   }
 
   @Override
-  public char getSeparator() {
-    return '/';
+  public boolean isSeparator(char c) {
+    return c == '/';
   }
 
   @Override
-  public boolean isSeparator(char c) {
-    return c == '/';
+  public char additionalSeparator() {
+    return 0;
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/vfs/WindowsOsPathPolicy.java

@@ -224,13 +224,13 @@ public boolean endsWith(String path, String suffix) {
   }
 
   @Override
-  public char getSeparator() {
-    return '/';
+  public boolean isSeparator(char c) {
+    return c == '/' || c == '\\';
   }
 
   @Override
-  public boolean isSeparator(char c) {
-    return c == '/' || c == '\\';
+  public char additionalSeparator() {
+    return '\\';
   }
 
   @Override

src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java

@@ -172,6 +172,9 @@ public void testContainsSeparator() {
   public void testGetChildWorks() {
     PathFragment pf = create("../some/path");
     assertThat(pf.getChild("hi")).isEqualTo(create("../some/path/hi"));
+    assertThat(pf.getChild("h\\i")).isEqualTo(create("../some/path/h\\i"));
+    assertThat(create("../some/path").getChild(".hi")).isEqualTo(create("../some/path/.hi"));
+    assertThat(create("../some/path").getChild("..hi")).isEqualTo(create("../some/path/..hi"));
   }
 
   @Test

src/test/java/com/google/devtools/build/lib/vfs/PathFragmentWindowsTest.java

@@ -95,7 +95,7 @@ public void testGetRelativeWindows() {
   }
 
   @Test
-  public void testGetRelativeMixed() throws Exception {
+  public void testGetRelativeMixed() {
     assertThat(create("a").getRelative("b")).isEqualTo(create("a/b"));
     assertThat(create("a").getRelative("/b")).isEqualTo(create("/b"));
     assertThat(create("a").getRelative("E:/b")).isEqualTo(create("E:/b"));
@@ -110,7 +110,7 @@ public void testGetRelativeMixed() throws Exception {
   }
 
   @Test
-  public void testRelativeTo() throws Exception {
+  public void testRelativeTo() {
     assertThat(create("").relativeTo("").getPathString()).isEqualTo("");
     assertThrows(IllegalArgumentException.class, () -> create("").relativeTo("a"));
 
@@ -126,6 +126,16 @@ public void testRelativeTo() throws Exception {
   @Test
   public void testGetChildWorks() {
     assertThat(create("../some/path").getChild("hi")).isEqualTo(create("../some/path/hi"));
+    assertThat(create("../some/path").getChild(".hi")).isEqualTo(create("../some/path/.hi"));
+    assertThat(create("../some/path").getChild("..hi")).isEqualTo(create("../some/path/..hi"));
+  }
+
+  @Test
+  public void testGetChildRejectsInvalidBaseNames() {
+    assertThrows(IllegalArgumentException.class, () -> create("").getChild("."));
+    assertThrows(IllegalArgumentException.class, () -> create("").getChild(".."));
+    assertThrows(IllegalArgumentException.class, () -> create("").getChild("multi/segment"));
+    assertThrows(IllegalArgumentException.class, () -> create("").getChild("multi\\segment"));
   }
 
   @Test
@@ -226,7 +236,7 @@ public void testNormalizeWindows() {
   }
 
   @Test
-  public void testWindowsDriveRelativePaths() throws Exception {
+  public void testWindowsDriveRelativePaths() {
     // On Windows, paths that look like "C:foo" mean "foo relative to the current directory
     // of drive C:\".
     // Bazel doesn't resolve such paths, and just takes them literally like normal path segments.