File APIs inconsistently handle paths ending in `.` on windows in 3.4.3 about sdk HOT 8 OPEN

Discovered when investigating dart-lang/test#2243.

from sdk.

Seems to be the absolute path which breaks copy (and copySync) for me. The following shows the same error:

import "dart:io";
void main() {
    var test = File(r'test.');
    try {
      test.copySync(r'copyTest.txt');
      print("Success");
    } catch (e) {
      print("$e");
    }
    test = test.absolute;
    try {
      test.copySync(r'copyTest.txt'); // Fails.
      print("Success");
    } catch (e) {
      print("$e");
    }
}

Also works with .\test. and ..\testdata\test., so it's not any preceding path, only an absolute one, that breaks it.

from sdk.

https://dart-review.googlesource.com/c/sdk/+/356680 is the culprit

from sdk.

Per https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file?redirectedfrom=MSDN#win32-file-namespaces on Windows you can't have (trailing) periods in filenames unless you prefix the filename with '\\?\'.
Since we do prefix filenames with '\?' before doing a copy

diff --git a/runtime/bin/file_win.cc b/runtime/bin/file_win.cc
index 66cadbedf1b..bf2c0c058e1 100644
--- a/runtime/bin/file_win.cc
+++ b/runtime/bin/file_win.cc
@@ -408,10 +408,14 @@ static std::unique_ptr<wchar_t[]> ToWinAPIPath(const char* utf8_path,
     return absolute_path;
   }

-  // Add prefix and replace forward slashes with backward slashes.
+  // Add prefix, remove trailing periods, replace forward slashes with
+  // backward slashes.
   auto result =
       std::make_unique<wchar_t[]>(kLongPathPrefixLength + path_length + 1);
   wcsncpy(result.get(), kLongPathPrefix, kLongPathPrefixLength);
+  if (is_file) {
+    while (path_length > 0 && absolute_path[path_length - 1] == '.') {
+      --path_length;
+    }
+  }
   for (int i = 0; i < path_length; i++) {
     result.get()[kLongPathPrefixLength + i] =
         absolute_path[i] == L'/' ? L'\\' : absolute_path[i];

cc @mraleph

from sdk.

@aam I don't think the proposed change is enough. Rules around trailing dots apply to all path components, not just the trailing one. Consider path test.\test. for example.

I think we have a choice to make:

1. We could add better support for path where components include trailing dots - this would require rewriting ToWinAPIFilePath to avoid using GetFullPathNameW which currently eats those dots and then forcing \\?\ prefix if any components contain ..
2. We can silently apply Win32 path normalization rules in ToWinAPIFilePath and strip trailing dots from all path components (so that test.\test. becomes test\test). Maybe just using PathCchCanonicalizeEx would do the trick?
3. We could do a breaking change and make dart:io throw an error when path contains a component with a trailing dot.

I am in doubt that we actually want option 2 - because it silently leads to a very surprising behavior, especially when people are trying to write portable code. I think we should go for either 1 or 3. I am leaning towards 1.

Any opinions?

from sdk.

I would be fine with either option 1 or 3, I would not be a fan of option 2, that seems very surprising.

from sdk.

"Do not end a file or directory name with a space or a period. Although the underlying file system may support such names, the Windows shell and user interface does not." (https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file)
It is true in Windows 11 - you are not able to cd/dir into such a directory and you can't do anything with it in Windows Explorer.
Given that, it's not clear whether writing our own canonicalizer that instead of using GetFullPathNameW is worth it.

Also at present, if user wants to they can explicitly add '\?' prefix to a path/filename with trailing periods and file operations will work.
But then forward slashes won't be accepted anymore, user have to specify backslashes, meaning no canonicalization to such filenames.

import "dart:io";
void main() {
    var test = File(r'\\?\c:\src\d2\sdk\test.');
    test.writeAsStringSync('kuka');
    try {
      test.copySync(r'copyTest.txt');
      print("Success");
    } catch (e) {
      print("$e");
    }
    test = test.absolute;
    print(test.path);
    try {
      test.copySync(r'copyTest.txt'); // Fails.
      print("Success");
    } catch (e) {
      print("$e");
    }
}

PS C:\src\d2\sdk> out\ReleaseX64\dart d1.dart
Success
\\?\c:\src\d2\sdk\test.
Success
PS C:\src\d2\sdk>

from sdk.

Also note that Windows CLI shell as well as Windows Explorer does strip trailing periods from filenames automatically:

PS C:\src\d2\sdk> dir a
Get-ChildItem: Cannot find path 'C:\src\d2\sdk\a' because it does not exist.
PS C:\src\d2\sdk> copy .\d1.dart a....
PS C:\src\d2\sdk> dir a

    Directory: C:\src\d2\sdk

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a---           6/13/2024  1:17 PM            420 a

PS C:\src\d2\sdk>

from sdk.