diff --git a/CHANGELOG.md b/CHANGELOG.md index 87f7b1f0711..50e71af4a31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,18 @@ * **Breaking change:** The `klass` getter on the `InstanceConstant` class in the Kernel AST API has been renamed to `classNode` for consistency. +* **Breaking change:** Updated `Link` implementation to utilize true symbolic + links instead of junctions on Windows. Existing junctions will continue to + work with the new `Link` implementation, but all new links will create + symbolic links. + + To create a symbolic link, Dart must be run with + administrative privileges or Developer Mode must be enabled, otherwise a + `FileSystemException` will be raised with errno set to + `ERROR_PRIVILEGE_NOT_HELD` (Issue [33966]). + +[33966]: https://github.com/dart-lang/sdk/issues/33966 + ### Dart VM ### Tool Changes diff --git a/runtime/bin/file_win.cc b/runtime/bin/file_win.cc index d7d5b3f84bf..0b59b20303b 100644 --- a/runtime/bin/file_win.cc +++ b/runtime/bin/file_win.cc @@ -375,63 +375,32 @@ typedef struct _REPARSE_DATA_BUFFER { static const int kReparseDataHeaderSize = sizeof ULONG + 2 * sizeof USHORT; static const int kMountPointHeaderSize = 4 * sizeof USHORT; +// Note: CreateLink used to create junctions on Windows instead of true +// symbolic links. All File::*Link methods now support handling links created +// as junctions and symbolic links. bool File::CreateLink(Namespace* namespc, const char* utf8_name, const char* utf8_target) { Utf8ToWideScope name(utf8_name); - int create_status = CreateDirectoryW(name.wide(), NULL); - // If the directory already existed, treat it as a success. - if ((create_status == 0) && - ((GetLastError() != ERROR_ALREADY_EXISTS) || - ((GetFileAttributesW(name.wide()) & FILE_ATTRIBUTE_DIRECTORY) != 0))) { - return false; - } - - HANDLE dir_handle = CreateFileW( - name.wide(), GENERIC_READ | GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, - OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, - NULL); - if (dir_handle == INVALID_HANDLE_VALUE) { - return false; - } - Utf8ToWideScope target(utf8_target); - int target_len = wcslen(target.wide()); - if (target_len > MAX_PATH - 1) { - CloseHandle(dir_handle); - return false; + DWORD flags = SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE; + + File::Type type = File::GetType(namespc, utf8_target, true); + if (type == kIsDirectory) { + flags |= SYMBOLIC_LINK_FLAG_DIRECTORY; } - int reparse_data_buffer_size = - sizeof REPARSE_DATA_BUFFER + 2 * MAX_PATH * sizeof WCHAR; - REPARSE_DATA_BUFFER* reparse_data_buffer = - reinterpret_cast(malloc(reparse_data_buffer_size)); - reparse_data_buffer->ReparseTag = IO_REPARSE_TAG_MOUNT_POINT; - wcscpy(reparse_data_buffer->MountPointReparseBuffer.PathBuffer, - target.wide()); - wcscpy( - reparse_data_buffer->MountPointReparseBuffer.PathBuffer + target_len + 1, - target.wide()); - reparse_data_buffer->MountPointReparseBuffer.SubstituteNameOffset = 0; - reparse_data_buffer->MountPointReparseBuffer.SubstituteNameLength = - target_len * sizeof WCHAR; - reparse_data_buffer->MountPointReparseBuffer.PrintNameOffset = - (target_len + 1) * sizeof WCHAR; - reparse_data_buffer->MountPointReparseBuffer.PrintNameLength = - target_len * sizeof WCHAR; - reparse_data_buffer->ReparseDataLength = - (target_len + 1) * 2 * sizeof WCHAR + kMountPointHeaderSize; - DWORD dummy_received_bytes; - int result = DeviceIoControl( - dir_handle, FSCTL_SET_REPARSE_POINT, reparse_data_buffer, - reparse_data_buffer->ReparseDataLength + kReparseDataHeaderSize, NULL, 0, - &dummy_received_bytes, NULL); - free(reparse_data_buffer); - if (CloseHandle(dir_handle) == 0) { - return false; + int create_status = CreateSymbolicLinkW(name.wide(), target.wide(), flags); + + // If running on a Windows 10 build older than 14972, an invalid parameter + // error will be returned when trying to use the + // SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE flag. Retry without the flag. + if ((create_status == 0) && (GetLastError() == ERROR_INVALID_PARAMETER)) { + flags &= ~SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE; + create_status = CreateSymbolicLinkW(name.wide(), target.wide(), flags); } - return (result != 0); + + return (create_status != 0); } bool File::Delete(Namespace* namespc, const char* name) { @@ -444,12 +413,18 @@ bool File::DeleteLink(Namespace* namespc, const char* name) { Utf8ToWideScope system_name(name); bool result = false; DWORD attributes = GetFileAttributesW(system_name.wide()); - if ((attributes != INVALID_FILE_ATTRIBUTES) && - (attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) { - // It's a junction(link), delete it. + if ((attributes == INVALID_FILE_ATTRIBUTES) || + ((attributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0)) { + SetLastError(ERROR_NOT_A_REPARSE_POINT); + return false; + } + if ((attributes & FILE_ATTRIBUTE_DIRECTORY) != 0) { + // It's a junction, which is a special type of directory, or a symbolic + // link to a directory. Remove the directory. result = (RemoveDirectoryW(system_name.wide()) != 0); } else { - SetLastError(ERROR_NOT_A_REPARSE_POINT); + // Symbolic link to a file. Remove the file. + result = (DeleteFileW(system_name.wide()) != 0); } return result; } @@ -458,65 +433,59 @@ bool File::Rename(Namespace* namespc, const char* old_path, const char* new_path) { File::Type type = GetType(namespc, old_path, false); - if (type == kIsFile) { - Utf8ToWideScope system_old_path(old_path); - Utf8ToWideScope system_new_path(new_path); - DWORD flags = MOVEFILE_WRITE_THROUGH | MOVEFILE_REPLACE_EXISTING; - int move_status = - MoveFileExW(system_old_path.wide(), system_new_path.wide(), flags); - return (move_status != 0); - } else { + if (type != kIsFile) { SetLastError(ERROR_FILE_NOT_FOUND); + return false; } - return false; + Utf8ToWideScope system_old_path(old_path); + Utf8ToWideScope system_new_path(new_path); + DWORD flags = MOVEFILE_WRITE_THROUGH | MOVEFILE_REPLACE_EXISTING; + int move_status = + MoveFileExW(system_old_path.wide(), system_new_path.wide(), flags); + return (move_status != 0); } bool File::RenameLink(Namespace* namespc, const char* old_path, const char* new_path) { File::Type type = GetType(namespc, old_path, false); - if (type == kIsLink) { - Utf8ToWideScope system_old_path(old_path); - Utf8ToWideScope system_new_path(new_path); - DWORD flags = MOVEFILE_WRITE_THROUGH | MOVEFILE_REPLACE_EXISTING; - // Links on Windows appear as special directories. MoveFileExW's - // MOVEFILE_REPLACE_EXISTING does not allow for replacement of directories, - // so we need to remove it before renaming a link. - if (Directory::Exists(namespc, new_path) == Directory::EXISTS) { - bool result = true; - if (GetType(namespc, new_path, false) == kIsLink) { - result = DeleteLink(namespc, new_path); - } else { - result = Delete(namespc, new_path); - } - // Bail out if the Delete calls fail. - if (!result) { - return false; - } - } - int move_status = - MoveFileExW(system_old_path.wide(), system_new_path.wide(), flags); - return (move_status != 0); - } else { + if (type != kIsLink) { SetLastError(ERROR_FILE_NOT_FOUND); + return false; } - return false; + Utf8ToWideScope system_old_path(old_path); + Utf8ToWideScope system_new_path(new_path); + DWORD flags = MOVEFILE_WRITE_THROUGH | MOVEFILE_REPLACE_EXISTING; + + // Junction links on Windows appear as special directories. MoveFileExW's + // MOVEFILE_REPLACE_EXISTING does not allow for replacement of directories, + // so we need to remove it before renaming a link. This step is only + // necessary for junctions created by the old Link.create implementation. + if ((Directory::Exists(namespc, new_path) == Directory::EXISTS) && + (GetType(namespc, new_path, false) == kIsLink)) { + // Bail out if the DeleteLink call fails. + if (!DeleteLink(namespc, new_path)) { + return false; + } + } + int move_status = + MoveFileExW(system_old_path.wide(), system_new_path.wide(), flags); + return (move_status != 0); } bool File::Copy(Namespace* namespc, const char* old_path, const char* new_path) { File::Type type = GetType(namespc, old_path, false); - if (type == kIsFile) { - 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; - } else { + if (type != kIsFile) { SetLastError(ERROR_FILE_NOT_FOUND); + return false; } - 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; } int64_t File::LengthFromPath(Namespace* namespc, const char* name) { diff --git a/sdk/lib/io/link.dart b/sdk/lib/io/link.dart index b43c13a8106..79f865638da 100644 --- a/sdk/lib/io/link.dart +++ b/sdk/lib/io/link.dart @@ -50,11 +50,11 @@ abstract class Link implements FileSystemEntity { * components are created. The directories in the path of [target] are * not affected, unless they are also in [path]. * - * On the Windows platform, this will only work with directories, and the - * target directory must exist. The link will be created as a Junction. - * Only absolute links will be created, and relative paths to the target - * will be converted to absolute paths by joining them with the path of the - * directory the link is contained in. + * On the Windows platform, this call will create a true symbolic link + * instead of a Junction. In order to create a symbolic link on Windows, Dart + * must be run in Administrator mode or the system must have Developer Mode + * enabled, otherwise a [FileSystemException] will be raised with + * `ERROR_PRIVILEGE_NOT_HELD` set as the errno when this call is made. * * On other platforms, the posix symlink() call is used to make a symbolic * link containing the string [target]. If [target] is a relative path, @@ -71,10 +71,11 @@ abstract class Link implements FileSystemEntity { * non-existing path components are created. The directories in * the path of [target] are not affected, unless they are also in [path]. * - * On the Windows platform, this will only work with directories, and the - * target directory must exist. The link will be created as a Junction. - * Only absolute links will be created, and relative paths to the target - * will be converted to absolute paths. + * On the Windows platform, this call will create a true symbolic link + * instead of a Junction. In order to create a symbolic link on Windows, Dart + * must be run in Administrator mode or the system must have Developer Mode + * enabled, otherwise a [FileSystemException] will be raised with + * `ERROR_PRIVILEGE_NOT_HELD` set as the errno when this call is made. * * On other platforms, the posix symlink() call is used to make a symbolic * link containing the string [target]. If [target] is a relative path, @@ -85,9 +86,6 @@ abstract class Link implements FileSystemEntity { /** * Synchronously updates the link. Calling [updateSync] on a non-existing link * will throw an exception. - * - * On the Windows platform, this will only work with directories, and the - * target directory must exist. */ void updateSync(String target); @@ -95,9 +93,6 @@ abstract class Link implements FileSystemEntity { * Updates the link. Returns a [:Future:] that completes with the * link when it has been updated. Calling [update] on a non-existing link * will complete its returned future with an exception. - * - * On the Windows platform, this will only work with directories, and the - * target directory must exist. */ Future update(String target); @@ -183,9 +178,6 @@ class _Link extends FileSystemEntity implements Link { Link get absolute => new Link.fromRawPath(_rawAbsolutePath); Future create(String target, {bool recursive: false}) { - if (Platform.isWindows) { - target = _makeWindowsLinkTarget(target); - } var result = recursive ? parent.create(recursive: true) : new Future.value(null); return result @@ -204,28 +196,10 @@ class _Link extends FileSystemEntity implements Link { if (recursive) { parent.createSync(recursive: true); } - if (Platform.isWindows) { - target = _makeWindowsLinkTarget(target); - } var result = _File._createLink(_Namespace._namespace, _rawPath, target); throwIfError(result, "Cannot create link", path); } - // Put target into the form "\??\C:\my\target\dir". - String _makeWindowsLinkTarget(String target) { - Uri base = new Uri.file('${Directory.current.path}\\'); - Uri link = new Uri.file(path); - Uri destination = new Uri.file(target); - String result = base.resolveUri(link).resolveUri(destination).toFilePath(); - if (result.length > 3 && result[1] == ':' && result[2] == '\\') { - return '\\??\\$result'; - } else { - throw new FileSystemException( - 'Target $result of Link.create on Windows cannot be converted' - ' to start with a drive letter. Unexpected error.'); - } - } - void updateSync(String target) { // TODO(12414): Replace with atomic update, where supported by platform. // Atomically changing a link can be done by creating the new link, with diff --git a/tests/standalone_2/io/link_test.dart b/tests/standalone_2/io/link_test.dart index afc0f73cf0c..477d7b53de1 100644 --- a/tests/standalone_2/io/link_test.dart +++ b/tests/standalone_2/io/link_test.dart @@ -7,7 +7,6 @@ import "package:expect/expect.dart"; import "package:path/path.dart"; import "dart:async"; import "dart:io"; -import "dart:isolate"; // Test the dart:io Link class. @@ -57,8 +56,8 @@ testCreateSync() { // Test FileSystemEntity.identical on files, directories, and links, // reached by different paths. - Expect - .isTrue(FileSystemEntity.identicalSync(createdDirectly, createdDirectly)); + Expect.isTrue( + FileSystemEntity.identicalSync(createdDirectly, createdDirectly)); Expect.isFalse( FileSystemEntity.identicalSync(createdDirectly, createdThroughLink)); Expect.isTrue(FileSystemEntity.identicalSync( @@ -190,6 +189,38 @@ testRenameSync() { Expect.isFalse(link2.existsSync()); } + testRenameToLink(String base, String target) { + Link link1 = Link(join(base, '1'))..createSync(target); + Link link2 = Link(join(base, '2'))..createSync(target); + Expect.isTrue(link1.existsSync()); + Expect.isTrue(link2.existsSync()); + Link renamed = link1.renameSync(link2.path); + Expect.isFalse(link1.existsSync()); + Expect.isTrue(renamed.existsSync()); + renamed.deleteSync(); + Expect.isFalse(renamed.existsSync()); + } + + testRenameToTarget(String linkName, String target, bool isDirectory) { + Link link = Link(linkName)..createSync(target); + Expect.isTrue(link.existsSync()); + try { + Link renamed = link.renameSync(target); + if (isDirectory) { + Expect.fail('Renaming a link to the name of an existing directory ' + + 'should fail'); + } + Expect.isTrue(renamed.existsSync()); + renamed.deleteSync(); + } on FileSystemException catch (_) { + if (isDirectory) { + return; + } + Expect.fail('Renaming a link to the name of an existing file should ' + + 'not fail'); + } + } + Directory baseDir = Directory.systemTemp.createTempSync('dart_link'); String base = baseDir.path; Directory dir = new Directory(join(base, 'a'))..createSync(); @@ -198,6 +229,11 @@ testRenameSync() { testRename(base, file.path); testRename(base, dir.path); + testRenameToLink(base, file.path); + + testRenameToTarget(join(base, 'fileLink'), file.path, false); + testRenameToTarget(join(base, 'dirLink'), dir.path, true); + baseDir.deleteSync(recursive: true); }