File APIs inconsistently handle paths ending in `.` on windows in 3.4.3

[jakemac53]

See this gist https://gist.github.com/jakemac53/845b60847dbda53f93aeed8056d5fbd2.

Assuming the file does exist with that name (I think it is invalid, but it is possible to create at least on the command line), on 3.2.0 and 3.4.0 and greater we do consistently always claim the file does not exist. I am not sure if this in intended behavior or not, but at least it is consistent.

However, starting with at least 3.4.0, the first file copy (using the absolute file URI) succeeds, even though exists() returned false.

I can't easily test this beyond 3.4.3 on my windows box, as I am not set up to do custom builds.

[jakemac53]

Discovered when investigating https://github.com/dart-lang/test/issues/2243.

[lrhn]

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.

[aam]

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

[aam]

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 https://github.com/dart-lang/sdk/blob/ca3e67a3b654c492b85b546354619137d1f2f95d/runtime/bin/file_win.cc#L816 , that changes meaning of those (trailing) periods. We could strip trailing periods before adding a prefix

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

[mraleph]

@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?

[jakemac53]

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

[aam]

"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>

[aam]

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>

[mraleph]

While stripping dots would be more "Windows-native", I think we should opt for supporting dots in filepaths instead - because we are multiplatform language.

We already hide some of the WinAPI complexity away from users (e.g. we don't require them to know about \\?\ prefix which is necessary to work with long file names), so I think we should just silently supports trailing dots as well.

We might want to add some warnings in File and Directory documentations that there are certain operating system specific recommendations around path (link to Windows docs) which users better adhere to.

[aam]

https://dart-review.googlesource.com/c/sdk/+/375421 with the fix