File::Copy avoids direct copying on Windows

There is a race condition for copying file on Windows, where CopyFile()
returns success but data has not been populated into destination file.
E.g process is killed or died in the middle.

This cl will change File::Copy as
1. Copy file to a temp file in the same directory of destination file.
2. Rename the file to the target file.

Bug: #42119
Change-Id: I39b6d451f6ace970bc554501148259d33de232c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149667
Commit-Queue: Zichang Guo <[email protected]>
Reviewed-by: Zach Anderson <[email protected]>

Author: zichangg

runtime/bin/file_win.cc

@@ -17,6 +17,7 @@
 #include <sys/utime.h>  // NOLINT
 
 #include "bin/builtin.h"
+#include "bin/crypto.h"
 #include "bin/directory.h"
 #include "bin/namespace.h"
 #include "bin/utils.h"
@@ -503,6 +504,85 @@ bool File::RenameLink(Namespace* namespc,
   return (move_status != 0);
 }
 
+static wchar_t* CopyToDartScopeString(wchar_t* string) {
+  wchar_t* wide_path = reinterpret_cast<wchar_t*>(
+      Dart_ScopeAllocate(MAX_PATH * sizeof(wchar_t) + 1));
+  wcscpy(wide_path, string);
+  return wide_path;
+}
+
+static wchar_t* CopyIntoTempFile(const char* src, const char* dest) {
+  // This function will copy the file to a temp file in the destination
+  // directory and return the path of temp file.
+  // Creating temp file name has the same logic as Directory::CreateTemp(),
+  // which tries with the rng and falls back to a uuid if it failed.
+  const char* last_backslash = strrchr(dest, '\\');
+  if (last_backslash == NULL) {
+    return NULL;
+  }
+  int length_of_parent_dir = last_backslash - dest + 1;
+  if (length_of_parent_dir + 8 > MAX_PATH) {
+    return NULL;
+  }
+  uint32_t suffix_bytes = 0;
+  const int kSuffixSize = sizeof(suffix_bytes);
+  if (Crypto::GetRandomBytes(kSuffixSize,
+                             reinterpret_cast<uint8_t*>(&suffix_bytes))) {
+    PathBuffer buffer;
+    char* dir = reinterpret_cast<char*>(
+        Dart_ScopeAllocate(1 + sizeof(char) * length_of_parent_dir));
+    memmove(dir, dest, length_of_parent_dir);
+    dir[length_of_parent_dir] = '\0';
+    if (!buffer.Add(dir)) {
+      return NULL;
+    }
+
+    char suffix[8 + 1];
+    Utils::SNPrint(suffix, sizeof(suffix), "%x", suffix_bytes);
+    Utf8ToWideScope source_path(src);
+    if (!buffer.Add(suffix)) {
+      return NULL;
+    }
+    if (CopyFileExW(source_path.wide(), buffer.AsStringW(), NULL, NULL, NULL,
+                    0) != 0) {
+      return CopyToDartScopeString(buffer.AsStringW());
+    }
+  }
+  // UUID has a total of 36 characters in the form of
+  // xxxxxxxx-xxxx-Mxxx-Nxxx-xxxxxxxxxxxx.
+  if (length_of_parent_dir + 36 > MAX_PATH) {
+    return NULL;
+  }
+  UUID uuid;
+  RPC_STATUS status = UuidCreateSequential(&uuid);
+  if ((status != RPC_S_OK) && (status != RPC_S_UUID_LOCAL_ONLY)) {
+    return NULL;
+  }
+  RPC_WSTR uuid_string;
+  status = UuidToStringW(&uuid, &uuid_string);
+  if (status != RPC_S_OK) {
+    return NULL;
+  }
+  PathBuffer buffer;
+  char* dir = reinterpret_cast<char*>(
+      Dart_ScopeAllocate(1 + sizeof(char) * length_of_parent_dir));
+  memmove(dir, dest, length_of_parent_dir);
+  dir[length_of_parent_dir] = '\0';
+  Utf8ToWideScope dest_path(dir);
+  if (!buffer.AddW(dest_path.wide()) ||
+      !buffer.AddW(reinterpret_cast<wchar_t*>(uuid_string))) {
+    return NULL;
+  }
+
+  RpcStringFreeW(&uuid_string);
+  Utf8ToWideScope source_path(src);
+  if (CopyFileExW(source_path.wide(), buffer.AsStringW(), NULL, NULL, NULL,
+                  0) != 0) {
+    return CopyToDartScopeString(buffer.AsStringW());
+  }
+  return NULL;
+}
+
 bool File::Copy(Namespace* namespc,
                 const char* old_path,
                 const char* new_path) {
@@ -511,11 +591,23 @@ bool File::Copy(Namespace* namespc,
     SetLastError(ERROR_FILE_NOT_FOUND);
     return false;
   }
-  Utf8ToWideScope system_old_path(old_path);
-  Utf8ToWideScope system_new_path(new_path);
-  bool success = CopyFileExW(system_old_path.wide(), system_new_path.wide(),
-                             NULL, NULL, NULL, 0) != 0;
-  return success;
+
+  wchar_t* temp_file = CopyIntoTempFile(old_path, new_path);
+  if (temp_file == NULL) {
+    return false;
+  }
+  Utf8ToWideScope system_new_dest(new_path);
+
+  // Remove the existing file. Otherwise, renaming will fail.
+  if (Exists(namespc, new_path)) {
+    DeleteFileW(system_new_dest.wide());
+  }
+
+  if (!MoveFileW(temp_file, system_new_dest.wide())) {
+    DeleteFileW(temp_file);
+    return false;
+  }
+  return true;
 }
 
 int64_t File::LengthFromPath(Namespace* namespc, const char* name) {

tests/standalone/io/file_copy_test.dart

@@ -30,6 +30,14 @@ void testCopySync() {
   Expect.equals(FILE_CONTENT2, file1.readAsStringSync());
   Expect.equals(FILE_CONTENT2, file2.readAsStringSync());
 
+  // Check there is no temporary files existing.
+  var list = tmp.listSync();
+  Expect.equals(2, list.length);
+  for (var file in list) {
+    final fileName = file.path.toString();
+    Expect.isTrue(fileName.contains("file1") || fileName.contains("file2"));
+  }
+
   // Fail when coping to directory.
   var dir = new Directory('${tmp.path}/dir')..createSync();
   Expect.throws(() => file1.copySync(dir.path));

tests/standalone_2/io/file_copy_test.dart

@@ -30,6 +30,14 @@ void testCopySync() {
   Expect.equals(FILE_CONTENT2, file1.readAsStringSync());
   Expect.equals(FILE_CONTENT2, file2.readAsStringSync());
 
+  // Check there is no temporary files existing.
+  var list = tmp.listSync();
+  Expect.equals(2, list.length);
+  for (var file in list) {
+    final fileName = file.path.toString();
+    Expect.isTrue(fileName.contains("file1") || fileName.contains("file2"));
+  }
+
   // Fail when coping to directory.
   var dir = new Directory('${tmp.path}/dir')..createSync();
   Expect.throws(() => file1.copySync(dir.path));