Repository: bazelbuild/bazel
Issue: Windows: Path.readSymbolicLink fails for junctions
Labels: type: bug
P2
area-Windows
team-OSS

### Description of the problem / feature request:

Path.readSymbolicLink calls FileSystem.readSymbolicLink:
https://github.com/bazelbuild/bazel/blob/d7aa53ec68f573f9f130be18ac3ac1350d1d6be6/src/main/java/com/google/devtools/build/lib/vfs/Path.java#L599

On Windows, this calls into JavaIoFileSystem, which calls java.nio.file.Files.readSymbolicLink, which is unaware of junctions.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

The bug is rooted in java.nio, so here's an out-of-bazel repro:

BUILD:
```
java_binary(
    name = "x",
    srcs = ["ReadJunc.java"],
    main_class = "ReadJunc",
)
```

ReadJunc.java:
```
import java.nio.file.Files;
import java.nio.file.FileSystems;
import java.nio.file.Path;

public class ReadJunc {
  public static void main(String[] args) throws Exception {
    System.err.println("Hello ReadJunc");

    Path path = FileSystems.getDefault().getPath("c:\\src\\bazel-releases\\current");
    System.err.printf("path=(%s) isdir=%d%n",
        path, path.toFile().isDirectory() ? 1 : 0);

    Path path2 = Files.readSymbolicLink(path);
    System.err.printf("path2=(%s)%n", path2);
  }
}
```

output:
```
C:\src\tmp>bazel run x
...
path=(c:\src\bazel-releases\current) isdir=1
Exception in thread "main" java.nio.file.NotLinkException: Reparse point is not a symbolic link
        at sun.nio.fs.WindowsLinkSupport.readLinkImpl(WindowsLinkSupport.java:318)
        at sun.nio.fs.WindowsLinkSupport.readLink(WindowsLinkSupport.java:59)
        at sun.nio.fs.WindowsFileSystemProvider.readSymbolicLink(WindowsFileSystemProvider.java:628)
        at java.nio.file.Files.readSymbolicLink(Files.java:1432)
        at ReadJunc.main(ReadJunc.java:13)
```

The particular path:
```
C:\src\tmp>dir c:\src\bazel-releases
...
2019-03-27  11:12    <JUNCTION>     current [C:\src\bazel-releases\0.24.0]
```

### What operating system are you running Bazel on?

Windows 10

Issue comments:

Pull request review comments:
Why comment here? Is `READ_SYMLINK_OR_JUNCTION_ERROR` set or used anywhere?
For documentation, and to indicate that the value exists. The variable is not used anywhere (handled by `switch.default`).

Removed.
Nit: maybe 1, 2, 3, 4,?
These values match constants in `src/main/native/windows/file.h`. Added comments.

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index d39173a5dba482..ee457b70652b5d 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
@@ -89,6 +89,12 @@ protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) th
     }
   }
 
+  @Override
+  protected PathFragment readSymbolicLink(Path path) throws IOException {
+    java.nio.file.Path nioPath = getNioPath(path);
+    return PathFragment.create(WindowsFileOperations.readSymlinkOrJunction(nioPath.toString()));
+  }
+
   @Override
   public boolean supportsSymbolicLinksNatively(Path path) {
     return false;
diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java
index c8243e485986d6..82086181f0d2f6 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java
@@ -44,33 +44,44 @@ private WindowsFileOperations() {
 
   private static final int MAX_PATH = 260;
 
-  // Keep IS_JUNCTION_* values in sync with src/main/native/windows/file.cc.
+  // Keep IS_JUNCTION_* values in sync with src/main/native/windows/file.h.
   private static final int IS_JUNCTION_YES = 0;
   private static final int IS_JUNCTION_NO = 1;
-  private static final int IS_JUNCTION_ERROR = 2;
+  // IS_JUNCTION_ERROR = 2;
 
-  // Keep CREATE_JUNCTION_* values in sync with src/main/native/windows/file.cc.
+  // Keep CREATE_JUNCTION_* values in sync with src/main/native/windows/file.h.
   private static final int CREATE_JUNCTION_SUCCESS = 0;
-  private static final int CREATE_JUNCTION_ERROR = 1;
+  // CREATE_JUNCTION_ERROR = 1;
   private static final int CREATE_JUNCTION_TARGET_NAME_TOO_LONG = 2;
   private static final int CREATE_JUNCTION_ALREADY_EXISTS_WITH_DIFFERENT_TARGET = 3;
   private static final int CREATE_JUNCTION_ALREADY_EXISTS_BUT_NOT_A_JUNCTION = 4;
   private static final int CREATE_JUNCTION_ACCESS_DENIED = 5;
   private static final int CREATE_JUNCTION_DISAPPEARED = 6;
 
-  // Keep DELETE_PATH_* values in sync with src/main/native/windows/file.cc.
+  // Keep DELETE_PATH_* values in sync with src/main/native/windows/file.h.
   private static final int DELETE_PATH_SUCCESS = 0;
-  private static final int DELETE_PATH_ERROR = 1;
+  // DELETE_PATH_ERROR = 1;
   private static final int DELETE_PATH_DOES_NOT_EXIST = 2;
   private static final int DELETE_PATH_DIRECTORY_NOT_EMPTY = 3;
   private static final int DELETE_PATH_ACCESS_DENIED = 4;
 
+  // Keep READ_SYMLINK_OR_JUNCTION_* values in sync with src/main/native/windows/file.h.
+  private static final int READ_SYMLINK_OR_JUNCTION_SUCCESS = 0;
+  // READ_SYMLINK_OR_JUNCTION_ERROR = 1;
+  private static final int READ_SYMLINK_OR_JUNCTION_ACCESS_DENIED = 2;
+  private static final int READ_SYMLINK_OR_JUNCTION_DOES_NOT_EXIST = 3;
+  private static final int READ_SYMLINK_OR_JUNCTION_NOT_A_LINK = 4;
+  private static final int READ_SYMLINK_OR_JUNCTION_UNKNOWN_LINK_TYPE = 5;
+
   private static native int nativeIsJunction(String path, String[] error);
 
   private static native boolean nativeGetLongPath(String path, String[] result, String[] error);
 
   private static native int nativeCreateJunction(String name, String target, String[] error);
 
+  private static native int nativeReadSymlinkOrJunction(
+      String name, String[] result, String[] error);
+
   private static native int nativeDeletePath(String path, String[] error);
 
   /** Determines whether `path` is a junction point or directory symlink. */
@@ -83,6 +94,7 @@ public static boolean isJunction(String path) throws IOException {
       case IS_JUNCTION_NO:
         return false;
       default:
+        // This is IS_JUNCTION_ERROR. The JNI code puts a custom message in 'error[0]'.
         throw new IOException(error[0]);
     }
   }
@@ -140,36 +152,63 @@ private static String removeUncPrefixAndUseSlashes(String p) {
   public static void createJunction(String name, String target) throws IOException {
     WindowsJniLoader.loadJni();
     String[] error = new String[] {null};
-    int result = nativeCreateJunction(name.replace('/', '\\'), target.replace('/', '\\'), error);
-    if (result != CREATE_JUNCTION_SUCCESS) {
-      switch (result) {
-        case CREATE_JUNCTION_TARGET_NAME_TOO_LONG:
-          error[0] = "target name is too long";
-          break;
-        case CREATE_JUNCTION_ALREADY_EXISTS_WITH_DIFFERENT_TARGET:
-          error[0] = "junction already exists with different target";
-          break;
-        case CREATE_JUNCTION_ALREADY_EXISTS_BUT_NOT_A_JUNCTION:
-          error[0] = "a file or directory already exists at the junction's path";
-          break;
-        case CREATE_JUNCTION_ACCESS_DENIED:
-          error[0] = "access is denied";
-          break;
-        case CREATE_JUNCTION_DISAPPEARED:
-          error[0] = "the junction's path got modified unexpectedly";
-          break;
-        default:
-          break;
-      }
-      throw new IOException(
-          String.format("Cannot create junction (name=%s, target=%s): %s", name, target, error[0]));
+    switch (nativeCreateJunction(asLongPath(name), asLongPath(target), error)) {
+      case CREATE_JUNCTION_SUCCESS:
+        return;
+      case CREATE_JUNCTION_TARGET_NAME_TOO_LONG:
+        error[0] = "target name is too long";
+        break;
+      case CREATE_JUNCTION_ALREADY_EXISTS_WITH_DIFFERENT_TARGET:
+        error[0] = "junction already exists with different target";
+        break;
+      case CREATE_JUNCTION_ALREADY_EXISTS_BUT_NOT_A_JUNCTION:
+        error[0] = "a file or directory already exists at the junction's path";
+        break;
+      case CREATE_JUNCTION_ACCESS_DENIED:
+        error[0] = "access is denied";
+        break;
+      case CREATE_JUNCTION_DISAPPEARED:
+        error[0] = "the junction's path got modified unexpectedly";
+        break;
+      default:
+        // This is CREATE_JUNCTION_ERROR (1). The JNI code puts a custom message in 'error[0]'.
+        break;
+    }
+    throw new IOException(
+        String.format("Cannot create junction (name=%s, target=%s): %s", name, target, error[0]));
+  }
+
+  public static String readSymlinkOrJunction(String name) throws IOException {
+    WindowsJniLoader.loadJni();
+    String[] target = new String[] {null};
+    String[] error = new String[] {null};
+    switch (nativeReadSymlinkOrJunction(asLongPath(name), target, error)) {
+      case READ_SYMLINK_OR_JUNCTION_SUCCESS:
+        return removeUncPrefixAndUseSlashes(target[0]);
+      case READ_SYMLINK_OR_JUNCTION_ACCESS_DENIED:
+        error[0] = "access is denied";
+        break;
+      case READ_SYMLINK_OR_JUNCTION_DOES_NOT_EXIST:
+        error[0] = "path does not exist";
+        break;
+      case READ_SYMLINK_OR_JUNCTION_NOT_A_LINK:
+        error[0] = "path is not a link";
+        break;
+      case READ_SYMLINK_OR_JUNCTION_UNKNOWN_LINK_TYPE:
+        error[0] = "unknown link type";
+        break;
+      default:
+        // This is READ_SYMLINK_OR_JUNCTION_ERROR (1). The JNI code puts a custom message in
+        // 'error[0]'.
+        break;
     }
+    throw new IOException(String.format("Cannot read link (name=%s): %s", name, error[0]));
   }
 
   public static boolean deletePath(String path) throws IOException {
     WindowsJniLoader.loadJni();
     String[] error = new String[] {null};
-    int result = nativeDeletePath(asLongPath(path.replace('/', '\\')), error);
+    int result = nativeDeletePath(asLongPath(path), error);
     switch (result) {
       case DELETE_PATH_SUCCESS:
         return true;
@@ -180,6 +219,7 @@ public static boolean deletePath(String path) throws IOException {
       case DELETE_PATH_ACCESS_DENIED:
         throw new java.nio.file.AccessDeniedException(path);
       default:
+        // This is DELETE_PATH_ERROR (1). The JNI code puts a custom message in 'error[0]'.
         throw new IOException(String.format("Cannot delete path '%s': %s", path, error[0]));
     }
   }
diff --git a/src/main/native/windows/file-jni.cc b/src/main/native/windows/file-jni.cc
index 09934ec5af6e68..3befe0fe27fcdc 100644
--- a/src/main/native/windows/file-jni.cc
+++ b/src/main/native/windows/file-jni.cc
@@ -94,7 +94,30 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeCreat
                         wname + L", " + wtarget, error),
                     env, error_msg_holder);
   }
-  return result;
+  return static_cast<jint>(result);
+}
+
+extern "C" JNIEXPORT jint JNICALL
+Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeReadSymlinkOrJunction(
+    JNIEnv* env, jclass clazz, jstring name, jobjectArray target_holder,
+    jobjectArray error_msg_holder) {
+  std::wstring wname(bazel::windows::GetJavaWstring(env, name));
+  std::wstring target, error;
+  int result = bazel::windows::ReadSymlinkOrJunction(wname, &target, &error);
+  if (result == bazel::windows::ReadSymlinkOrJunctionResult::kSuccess) {
+    env->SetObjectArrayElement(
+        target_holder, 0,
+        env->NewString(reinterpret_cast<const jchar*>(target.c_str()),
+                       target.size()));
+  } else {
+    if (!error.empty() && CanReportError(env, error_msg_holder)) {
+      ReportLastError(bazel::windows::MakeErrorMessage(
+                          WSTR(__FILE__), __LINE__,
+                          L"nativeReadSymlinkOrJunction", wname, error),
+                      env, error_msg_holder);
+    }
+  }
+  return static_cast<jint>(result);
 }
 
 extern "C" JNIEXPORT jint JNICALL
diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc
index 091f5274323116..47c5a52e1c8432 100644
--- a/src/main/native/windows/file.cc
+++ b/src/main/native/windows/file.cc
@@ -161,7 +161,7 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target,
           WSTR(__FILE__), __LINE__, L"CreateJunction", junction_name,
           L"expected an absolute Windows path for junction_name");
     }
-    CreateJunctionResult::kError;
+    return CreateJunctionResult::kError;
   }
   if (!IsAbsoluteNormalizedWindowsPath(junction_target)) {
     if (error) {
@@ -169,7 +169,7 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target,
           WSTR(__FILE__), __LINE__, L"CreateJunction", junction_target,
           L"expected an absolute Windows path for junction_target");
     }
-    CreateJunctionResult::kError;
+    return CreateJunctionResult::kError;
   }
 
   const WCHAR* target = HasUncPrefix(junction_target.c_str())
@@ -420,6 +420,82 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target,
   return CreateJunctionResult::kSuccess;
 }
 
+int ReadSymlinkOrJunction(const wstring& path, wstring* result,
+                          wstring* error) {
+  if (!IsAbsoluteNormalizedWindowsPath(path)) {
+    if (error) {
+      *error = MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"ReadSymlinkOrJunction", path,
+          L"expected an absolute Windows path for 'path'");
+    }
+    return ReadSymlinkOrJunctionResult::kError;
+  }
+
+  AutoHandle handle(CreateFileW(
+      AddUncPrefixMaybe(path).c_str(), 0,
+      FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
+      OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS,
+      NULL));
+  if (!handle.IsValid()) {
+    DWORD err = GetLastError();
+    if (err == ERROR_SHARING_VIOLATION) {
+      // The path is held open by another process.
+      return ReadSymlinkOrJunctionResult::kAccessDenied;
+    } else if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
+      // Path or a parent directory does not exist.
+      return ReadSymlinkOrJunctionResult::kDoesNotExist;
+    }
+
+    // The path seems to exist yet we cannot open it for metadata-reading.
+    // Report as much information as we have, then give up.
+    if (error) {
+      *error = MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateFileW",
+                                path, err);
+    }
+    return ReadSymlinkOrJunctionResult::kError;
+  }
+
+  uint8_t raw_buf[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
+  PREPARSE_DATA_BUFFER buf = reinterpret_cast<PREPARSE_DATA_BUFFER>(raw_buf);
+  DWORD bytes_returned;
+  if (!::DeviceIoControl(handle, FSCTL_GET_REPARSE_POINT, NULL, 0, buf,
+                         MAXIMUM_REPARSE_DATA_BUFFER_SIZE, &bytes_returned,
+                         NULL)) {
+    DWORD err = GetLastError();
+    if (err == ERROR_NOT_A_REPARSE_POINT) {
+      return ReadSymlinkOrJunctionResult::kNotALink;
+    }
+
+    // Some unknown error occurred.
+    if (error) {
+      *error = MakeErrorMessage(WSTR(__FILE__), __LINE__, L"DeviceIoControl",
+                                path, err);
+    }
+    return ReadSymlinkOrJunctionResult::kError;
+  }
+
+  switch (buf->ReparseTag) {
+    case IO_REPARSE_TAG_SYMLINK: {
+      wchar_t* p =
+          (wchar_t*) (((uint8_t*) buf->SymbolicLinkReparseBuffer.PathBuffer) +
+                          buf->SymbolicLinkReparseBuffer.SubstituteNameOffset);
+      *result = wstring(p, buf->SymbolicLinkReparseBuffer.SubstituteNameLength /
+                               sizeof(WCHAR));
+      return ReadSymlinkOrJunctionResult::kSuccess;
+    }
+    case IO_REPARSE_TAG_MOUNT_POINT: {
+      wchar_t* p =
+          (wchar_t*) (((uint8_t*) buf->MountPointReparseBuffer.PathBuffer) +
+                          buf->MountPointReparseBuffer.SubstituteNameOffset);
+      *result = wstring(p, buf->MountPointReparseBuffer.SubstituteNameLength /
+                               sizeof(WCHAR));
+      return ReadSymlinkOrJunctionResult::kSuccess;
+    }
+    default:
+      return ReadSymlinkOrJunctionResult::kUnknownLinkType;
+  }
+}
+
 struct DirectoryStatus {
   enum {
     kDoesNotExist = 0,
diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h
index 84cbde3362ef51..eb37c1b26b871b 100644
--- a/src/main/native/windows/file.h
+++ b/src/main/native/windows/file.h
@@ -77,6 +77,17 @@ struct CreateJunctionResult {
   };
 };
 
+struct ReadSymlinkOrJunctionResult {
+  enum {
+    kSuccess = 0,
+    kError = 1,
+    kAccessDenied = 2,
+    kDoesNotExist = 3,
+    kNotALink = 4,
+    kUnknownLinkType = 5,
+  };
+};
+
 // Determines whether `path` is a junction (or directory symlink).
 //
 // `path` should be an absolute, normalized, Windows-style path, with "\\?\"
@@ -124,6 +135,9 @@ wstring GetLongPath(const WCHAR* path, unique_ptr<WCHAR[]>* result);
 int CreateJunction(const wstring& junction_name, const wstring& junction_target,
                    wstring* error);
 
+int ReadSymlinkOrJunction(const wstring& path, wstring* result,
+                          wstring* error);
+
 // Deletes the file, junction, or empty directory at `path`.
 // Returns DELETE_PATH_SUCCESS if it successfully deleted the path, otherwise
 // returns one of the other DELETE_PATH_* constants (e.g. when the directory is


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
index e5470749b0ea68..b0c9454d5121dc 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
@@ -331,4 +331,42 @@ public String apply(File input) {
       assertThat(p.isFile()).isTrue();
     }
   }
+
+  @Test
+  public void testReadJunction() throws Exception {
+    testUtil.scratchFile("dir\\hello.txt", "hello");
+    testUtil.createJunctions(ImmutableMap.of("junc", "dir"));
+
+    Path dirPath = testUtil.createVfsPath(fs, "dir");
+    Path juncPath = testUtil.createVfsPath(fs, "junc");
+
+    assertThat(dirPath.isDirectory()).isTrue();
+    assertThat(juncPath.isDirectory()).isTrue();
+
+    assertThat(dirPath.isSymbolicLink()).isFalse();
+    assertThat(juncPath.isSymbolicLink()).isTrue();
+
+    try {
+      testUtil.createVfsPath(fs, "does-not-exist").readSymbolicLink();
+      fail("expected exception");
+    } catch (IOException expected) {
+      assertThat(expected).hasMessageThat().matches(".*path does not exist");
+    }
+
+    try {
+      testUtil.createVfsPath(fs, "dir\\hello.txt").readSymbolicLink();
+      fail("expected exception");
+    } catch (IOException expected) {
+      assertThat(expected).hasMessageThat().matches(".*path is not a link");
+    }
+
+    try {
+      dirPath.readSymbolicLink();
+      fail("expected exception");
+    } catch (IOException expected) {
+      assertThat(expected).hasMessageThat().matches(".*path is not a link");
+    }
+
+    assertThat(juncPath.readSymbolicLink()).isEqualTo(dirPath.asFragment());
+  }
 }