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:

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..8244c7565b7ddf 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.readJunction(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..32154db922518c 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,12 +44,12 @@ 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;
 
-  // 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;
   private static final int CREATE_JUNCTION_TARGET_NAME_TOO_LONG = 2;
@@ -58,7 +58,14 @@ private WindowsFileOperations() {
   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 READ_JUNCTION_* values in sync with src/main/native/windows/file.h.
+  private static final int READ_JUNCTION_SUCCESS = 0;
+  private static final int READ_JUNCTION_ERROR = 1;
+  private static final int READ_JUNCTION_DOES_NOT_EXIST = 2;
+  private static final int READ_JUNCTION_ACCESS_DENIED = 3;
+  private static final int READ_JUNCTION_NOT_A_JUNCTION = 4;
+
+  // 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;
   private static final int DELETE_PATH_DOES_NOT_EXIST = 2;
@@ -71,6 +78,8 @@ private WindowsFileOperations() {
 
   private static native int nativeCreateJunction(String name, String target, String[] error);
 
+  private static native int nativeReadJunction(String path, String[] result, int[] DEBUG_target_len, String[] error);
+
   private static native int nativeDeletePath(String path, String[] error);
 
   /** Determines whether `path` is a junction point or directory symlink. */
@@ -166,10 +175,41 @@ public static void createJunction(String name, String target) throws IOException
     }
   }
 
+  public static String readJunction(String path) throws IOException {
+    WindowsJniLoader.loadJni();
+    String[] target = new String[] {null};
+    int[] DEBUG_target_len = new int[] { 0 };
+    String[] error = new String[] {null};
+
+    path = asLongPath(path);
+    int result = nativeReadJunction(path, target, DEBUG_target_len, error);
+    if (result == READ_JUNCTION_SUCCESS) {
+      error[0] = removeUncPrefixAndUseSlashes(target[0]);
+      System.err.printf(" | DEBUG | readJunc(%s) -> (%s) %d %d (%s)%n",
+          path, target[0], DEBUG_target_len[0], target[0].length(), error[0]);
+      return error[0];
+    } else {
+      switch (result) {
+        case READ_JUNCTION_DOES_NOT_EXIST:
+          error[0] = "path does not exist";
+          break;
+        case READ_JUNCTION_ACCESS_DENIED:
+          error[0] = "access is denied";
+          break;
+        case READ_JUNCTION_NOT_A_JUNCTION:
+          error[0] = "not a junction";
+          break;
+        default:
+          break;
+      }
+      throw new IOException(String.format("Cannot read junction '%s': %s", path, 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;
diff --git a/src/main/native/windows/file-jni.cc b/src/main/native/windows/file-jni.cc
index 09934ec5af6e68..03c59d137288c3 100644
--- a/src/main/native/windows/file-jni.cc
+++ b/src/main/native/windows/file-jni.cc
@@ -97,6 +97,32 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeCreat
   return result;
 }
 
+extern "C" JNIEXPORT jint JNICALL
+Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeReadJunction(
+    JNIEnv* env, jclass clazz, jstring path, jobjectArray result_holder,
+    jintArray DEBUG_result_len_holder, jobjectArray error_msg_holder) {
+  std::wstring target, error;
+  int DEBUG_target_len;
+  std::wstring wpath(bazel::windows::GetJavaWstring(env, path));
+  int result = bazel::windows::ReadJunction(wpath, &target, &DEBUG_target_len, &error);
+  if (result == bazel::windows::ReadJunctionResult::kSuccess) {
+    env->SetObjectArrayElement(
+        result_holder, 0,
+        env->NewString(reinterpret_cast<const jchar*>(target.c_str()),
+                       target.size()));
+    jint DEBUG_targetlen = DEBUG_target_len;
+    env->SetIntArrayRegion(DEBUG_result_len_holder, 0, 1, &DEBUG_targetlen);
+  } else {
+    if (!error.empty() && CanReportError(env, error_msg_holder)) {
+      ReportLastError(
+          bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                                           L"nativeReadJunction", wpath, error),
+          env, error_msg_holder);
+    }
+  }
+  return result;
+}
+
 extern "C" JNIEXPORT jint JNICALL
 Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeDeletePath(
     JNIEnv* env, jclass clazz, jstring path, jobjectArray error_msg_holder) {
diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc
index efe99c946400a2..1e223146a8ec8c 100644
--- a/src/main/native/windows/file.cc
+++ b/src/main/native/windows/file.cc
@@ -18,6 +18,7 @@
 
 #include <windows.h>
 #include <WinIoCtl.h>
+#include <fileapi.h>
 
 #include <stdint.h>  // uint8_t
 
@@ -143,8 +144,35 @@ typedef struct _JunctionDescription {
   Descriptor descriptor;
   WCHAR PathBuffer[ANYSIZE_ARRAY];
 } JunctionDescription;
+
+typedef struct _AllocatedJunctionDescription {
+  union {
+    uint8_t raw_data[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
+    JunctionDescription data;
+  };
+} AllocatedJunctionDescription;
 #pragma pack(pop)
 
+static bool ReadJunctionByHandle(
+    HANDLE handle, JunctionDescription* reparse_buffer, WCHAR** target_path,
+    DWORD* target_path_len, DWORD* err) {
+  DWORD bytes_returned;
+  if (!::DeviceIoControl(handle, FSCTL_GET_REPARSE_POINT, NULL, 0,
+                         reparse_buffer, MAXIMUM_REPARSE_DATA_BUFFER_SIZE,
+                         &bytes_returned, NULL)) {
+    *err = GetLastError();
+    return false;
+  }
+
+  *target_path = reparse_buffer->PathBuffer +
+                 reparse_buffer->descriptor.SubstituteNameOffset +
+                 /* "\??\" prefix */ 4;
+  *target_path_len =
+      reparse_buffer->descriptor.SubstituteNameLength / sizeof(WCHAR) -
+      /* "\??\" prefix */ 4;
+  return true;
+}
+
 int CreateJunction(const wstring& junction_name, const wstring& junction_target,
                    wstring* error) {
   if (!IsAbsoluteNormalizedWindowsPath(junction_name)) {
@@ -309,15 +337,14 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target,
     }
   }
 
-  uint8_t reparse_buffer_bytes[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
-  JunctionDescription* reparse_buffer =
-      reinterpret_cast<JunctionDescription*>(reparse_buffer_bytes);
+  AllocatedJunctionDescription reparse_buffer_bytes;
+  JunctionDescription* reparse_buffer = &reparse_buffer_bytes.data;
   if (create) {
     // The junction doesn't exist yet, and we have an open handle to the
     // candidate directory with write access and no sharing. Proceed to turn the
     // directory into a junction.
 
-    memset(reparse_buffer_bytes, 0, MAXIMUM_REPARSE_DATA_BUFFER_SIZE);
+    memset(&reparse_buffer_bytes, 0, sizeof(AllocatedJunctionDescription));
 
     // "\??\" is meaningful to the kernel, it's a synomym for the "\DosDevices\"
     // object path. (NOT to be confused with "\\?\" which is meaningful for the
@@ -372,25 +399,19 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target,
     }
   } else {
     // The junction already exists. Check if it points to the right target.
-
-    DWORD bytes_returned;
-    if (!::DeviceIoControl(handle, FSCTL_GET_REPARSE_POINT, NULL, 0,
-                           reparse_buffer, MAXIMUM_REPARSE_DATA_BUFFER_SIZE,
-                           &bytes_returned, NULL)) {
-      DWORD err = GetLastError();
+    WCHAR* actual_target;
+    DWORD target_path_len, err;
+    if (!ReadJunctionByHandle(handle, reparse_buffer, &actual_target,
+                              &target_path_len, &err)) {
       // Some unknown error occurred.
       if (error) {
-        *error = MakeErrorMessage(WSTR(__FILE__), __LINE__, L"DeviceIoControl",
-                                  name, err);
+        *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                                  L"ReadJunctionByHandle", name, err);
       }
       return CreateJunctionResult::kError;
     }
 
-    WCHAR* actual_target = reparse_buffer->PathBuffer +
-                           reparse_buffer->descriptor.SubstituteNameOffset +
-                           /* "\??\" prefix */ 4;
-    if (reparse_buffer->descriptor.SubstituteNameLength !=
-            (/* "\??\" prefix */ 4 + target.size()) * sizeof(WCHAR) ||
+    if (target_path_len != target.size() ||
         _wcsnicmp(actual_target, target.c_str(), target.size()) != 0) {
       return CreateJunctionResult::kAlreadyExistsWithDifferentTarget;
     }
@@ -399,6 +420,99 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target,
   return CreateJunctionResult::kSuccess;
 }
 
+static inline HANDLE OpenJunction(const wstring& path) {
+  return CreateFileW(
+      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);
+}
+
+int ReadJunction(const wstring& path, wstring* result, int* DEBUG_result_len,
+                 wstring* error) {
+  AutoHandle handle(OpenJunction(path));
+  if (!handle.IsValid()) {
+    DWORD err = GetLastError();
+    if (err == ERROR_SHARING_VIOLATION) {
+      // The junction is held open by another process.
+      return ReadJunctionResult::kAccessDenied;
+    } else if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
+      // The junction does not exist or one of its parent directories is not a
+      // directory.
+      return ReadJunctionResult::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 ReadJunctionResult::kError;
+  }
+
+  BY_HANDLE_FILE_INFORMATION info;
+  if (!GetFileInformationByHandle(handle, &info)) {
+    DWORD err = GetLastError();
+    // Some unknown error occurred.
+    if (error) {
+      *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                                L"GetFileInformationByHandle", path, err);
+    }
+    return ReadJunctionResult::kError;
+  }
+
+  if (info.dwFileAttributes == INVALID_FILE_ATTRIBUTES) {
+    // Some unknown error occurred.
+    if (error) {
+      *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                                L"Failed to get attributes", path, 0);
+    }
+    return ReadJunctionResult::kError;
+  }
+
+  if (!(info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) {
+    return ReadJunctionResult::kNotAJunction;
+  }
+
+  AllocatedJunctionDescription junc;
+  memset(&junc, 0, sizeof(AllocatedJunctionDescription));
+
+  WCHAR* target_path;
+  DWORD target_path_len, err;
+  if (!ReadJunctionByHandle(handle, &junc.data, &target_path, &target_path_len,
+                            &err)) {
+    if (err == ERROR_NOT_A_REPARSE_POINT) {
+      return ReadJunctionResult::kNotAJunction;
+    }
+
+    if (error) {
+      *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                                L"ReadJunctionByHandle", path, err);
+    }
+    return ReadJunctionResult::kError;
+  }
+
+  // DEBUG
+//  {
+//    FILE* fff=fopen("C:\\src\\bazel\\jni.log", "at");
+//    fseek(fff, 0, SEEK_END);
+//    fprintf(fff, "ReadJunc(%ls) -> %d %d %d (%ls) | %d %d (%ls)\n",
+//        path.c_str(),
+//        junc.data.descriptor.SubstituteNameOffset,
+//        junc.data.descriptor.SubstituteNameLength,
+//        (int) wcslen(junc.data.PathBuffer + junc.data.descriptor.SubstituteNameOffset),
+//        junc.data.PathBuffer + junc.data.descriptor.SubstituteNameOffset,
+//        target_path_len,
+//        (int) wcslen(target_path),
+//        target_path);
+//    fclose(fff);
+//  }
+
+  result->assign(target_path);
+  *DEBUG_result_len = target_path_len;
+  return ReadJunctionResult::kSuccess;
+}
+
 struct DirectoryStatus {
   enum {
     kDoesNotExist = 0,
diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h
index 84cbde3362ef51..3013a4e8cec996 100644
--- a/src/main/native/windows/file.h
+++ b/src/main/native/windows/file.h
@@ -77,6 +77,16 @@ struct CreateJunctionResult {
   };
 };
 
+struct ReadJunctionResult {
+  enum {
+    kSuccess = 0,
+    kError = 1,
+    kDoesNotExist = 2,
+    kAccessDenied = 3,
+    kNotAJunction = 4,
+  };
+};
+
 // Determines whether `path` is a junction (or directory symlink).
 //
 // `path` should be an absolute, normalized, Windows-style path, with "\\?\"
@@ -124,6 +134,9 @@ wstring GetLongPath(const WCHAR* path, unique_ptr<WCHAR[]>* result);
 int CreateJunction(const wstring& junction_name, const wstring& junction_target,
                    wstring* error);
 
+int ReadJunction(const wstring& path, wstring* result, int* DEBUG_result_len,
+                 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..c8a4343967f245 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(".*not a junction");
+    }
+
+    try {
+      dirPath.readSymbolicLink();
+      fail("expected exception");
+    } catch (IOException expected) {
+      assertThat(expected).hasMessageThat().matches(".*not a junction");
+    }
+
+    assertThat(juncPath.readSymbolicLink()).isEqualTo(dirPath.asFragment());
+  }
 }