Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Platform.resolvedExecutable gives invalid path for network UNC #52309

Closed
timsneath opened this issue May 8, 2023 · 4 comments
Closed

Platform.resolvedExecutable gives invalid path for network UNC #52309

timsneath opened this issue May 8, 2023 · 4 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. os-windows

Comments

@timsneath
Copy link
Contributor

timsneath commented May 8, 2023

See fluttercommunity/plus_plugins#1483 (comment) for more information.

In Platform::ResolveExecutablePath, if Windows returns a UNC such as \\?\UNC\server\share\folder\file.exe from GetModuleFileNameW, this will become stripped to UNC\server\share\folder\file.exe, which is not a valid Windows file path.

I believe this is due to some incorrect URL editing here:

sdk/runtime/bin/file_win.cc

Lines 1060 to 1065 in 6e18b08

// Remove leading \\?\ if possible, unless input used it.
int offset = 0;
if ((result_size > 4) && (wcsncmp(path.get(), L"\\\\?\\", 4) == 0) &&
(strncmp(pathname, "\\\\?\\", 4) != 0)) {
offset = 4;
}

Documentation on valid file path formats on Windows can be found here:
https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats

/cc @bkonyi @aam

@timsneath
Copy link
Contributor Author

It's not clear why we remove \\?\ at all? Is it out of a fear that it's not a valid file syntax?

To this untrained PM eye, it seems dangerous to be editing paths like this by hand if at all possible.

@aam
Copy link
Contributor

aam commented May 8, 2023

I've tried reproducing this in dart on Windows, but things seem to work as expected:

PS C:\src\d1\sdk> type r.dart
import 'dart:io';

main() {
  print(Platform.executable);
}
PS C:\src\d1\sdk> \\aam0\c$\src\d1\sdk\out\ReleaseX64\dart.exe r.dart
\\aam0\c$\src\d1\sdk\out\ReleaseX64\dart.exe

@timsneath
Copy link
Contributor Author

timsneath commented May 8, 2023

OK, I'll trade you for a broken one :)

import 'dart:io';

main() {
  print(Platform.executable);
  print(Platform.resolvedExecutable);
}
[C:\src\scratch] \\.\UNC\MICA\Flutter\bin\cache\dart-sdk\bin\dart.exe io.dart
\\.\UNC\MICA\Flutter\bin\cache\dart-sdk\bin\dart.exe
UNC\MICA\flutter\bin\cache\dart-sdk\bin\dart.exe

the former is a valid file path; the latter is not.

(This is a machine named MICA where I've shared C:\src\flutter as \\MICA\Flutter.)

@aam
Copy link
Contributor

aam commented May 8, 2023

Ah, I see, thanks. Platform.resolvedExecutable gets malformed when we attempt to canonicalize Windows path and remove \\?\, which was thought to be used specifically to prefix long-paths, letting Windows to bypass MAX_PATH limitation.
UNC paths were not considered, but somehow they should be.

@aam aam added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. os-windows labels May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. os-windows
Projects
None yet
Development

No branches or pull requests

2 participants