Windows: fs.readdir() includes "." and ".." when running over Sharepoint connection

[bpasero]

I see this in Electron 0.34.1 which is using node.js 4.1.1. I have mapped a Sharepoint connection as a drive on Windows 10 and notice that fs.readdir() includes "." and "..". I have never seen this in any other OS or environment. I dont think "." and ".." should be included in the call.

[bnoordhuis]

Here is the code that filters out the "." and ".." directory entries. I speculate that the SharePoint driver returns something that looks like but is not exactly those strings.

What does filename.split('').map(c => c.charCodeAt(0)) print for those entries?

[bpasero]

@bnoordhuis 46 for "." and 46, 46 for "..". I can also see that the same code is running in the patched node version of Electron (https://github.com/atom/node/blob/1445826ca73cc79bc57d503dd11d4ffaf695625c/deps/uv/src/win/fs.c#L890).

While the char code in JS is 46, maybe it is different in fs.c at that point in time actually?

[bpasero]

Actually this reproduces from vanilla node.js 4.1.1 and 5.x:

C:\Users\benjpas\Downloads>node

process.version
'v5.1.0'
require("fs").readdirSync("Z:")
[ '.',
'..',
'Presentations',
........
'Vision',
'Research' ]

[bnoordhuis]

Does this patch help?

diff --git a/deps/uv/src/win/fs.c b/deps/uv/src/win/fs.c
index 4a17573..cade6e3 100644
--- a/deps/uv/src/win/fs.c
+++ b/deps/uv/src/win/fs.c
@@ -921,8 +921,6 @@ void fs__scandir(uv_fs_t* req) {
       if (dirent == NULL)
         goto out_of_memory_error;

-      dirents[dirents_used++] = dirent;
-
       /* Convert file name to UTF-8. */
       if (WideCharToMultiByte(CP_UTF8,
                               0,
@@ -934,6 +932,19 @@ void fs__scandir(uv_fs_t* req) {
                               NULL) == 0)
         goto win32_error;

+      /* Skip over '.' and '..' entries. */
+      if (utf8_len == 1 && dirent->d_name[0] == '.') {
+        uv__free(dirent);
+        continue;
+      }
+      if (utf8_len == 2 && dirent->d_name[0] == '.' &&
+          dirent->d_name[1] == '.') {
+        uv__free(dirent);
+        continue;
+      }
+
+      dirents[dirents_used++] = dirent;
+
       /* Add a null terminator to the filename. */
       dirent->d_name[utf8_len] = '\0';

[bpasero]

@bnoordhuis unfortunately not, can we somehow console.log more information to find out what the string really is from the C code?

[bnoordhuis]

I'd start by adding printf statements to the code in deps/uv/src/win/fs.c.

[bpasero]

@bnoordhuis anything special I have to do to see the output from printf() in my compiled node.exe?

[bnoordhuis]

You have to recompile it after every change but I assume you know that. Apart from that, there's nothing you need to do; the printfs should show up next time you run the binary.

[bpasero]

@bnoordhuis both utf8_len and wchar_len are 2 (for the case of '.') and thats why those checks fail.

[bpasero]

@bnoordhuis so if I take out the length check the filter function works and "." and ".." are not returned.

[bnoordhuis]

wchar_len == 2 for the "." case? What's the value of the second character?

[bpasero]

@bnoordhuis it is the null character, looks like the string we get back is a null-terminated string maybe?

[bnoordhuis]

Can you try this patch?

diff --git a/deps/uv/src/win/fs.c b/deps/uv/src/win/fs.c
index 4a17573..7947c3b 100644
--- a/deps/uv/src/win/fs.c
+++ b/deps/uv/src/win/fs.c
@@ -886,12 +886,26 @@ void fs__scandir(uv_fs_t* req) {
       /* Compute the length of the filename in WCHARs. */
       wchar_len = info->FileNameLength / sizeof info->FileName[0];

-      /* Skip over '.' and '..' entries. */
-      if (wchar_len == 1 && info->FileName[0] == L'.')
+      /* Skip over '.' and '..' entries.  It has been reported that
+       * the SharePoint driver includes the terminating zero byte in
+       * the filename length.
+       */
+      if (wchar_len == 1 && info->FileName[0] == L'.') {
         continue;
-      if (wchar_len == 2 && info->FileName[0] == L'.' &&
-          info->FileName[1] == L'.')
+      }
+
+      if (wchar_len == 2 &&
+          info->FileName[0] == L'.' &&
+          (info->FileName[1] == L'.' || info->FileName[1] == L'\0')) {
         continue;
+      }
+
+      if (wchar_len == 3 &&
+          info->FileName[0] == L'.' &&
+          info->FileName[1] == L'.' &&
+          info->FileName[2] == L'\0') {
+        continue;
+      }

       /* Compute the space required to store the filename as UTF-8. */
       utf8_len = WideCharToMultiByte(

[bpasero]

@bnoordhuis yeah looks like that one works!

[bnoordhuis]

libuv/libuv#636

[bpasero]

Thanks!

[targos]

I didn't know you could close issues with commit in a different repo !
Should we reopen until we get a libuv update ?

[bnoordhuis]

Only if you have commit access to both. I'll reopen. =)