-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
Windows: fs.readdir() includes "." and ".." when running over Sharepoint connection #4002
Comments
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 |
@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? |
Actually this reproduces from vanilla node.js 4.1.1 and 5.x: C:\Users\benjpas\Downloads>node
|
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';
|
@bnoordhuis unfortunately not, can we somehow console.log more information to find out what the string really is from the C code? |
I'd start by adding printf statements to the code in |
@bnoordhuis anything special I have to do to see the output from printf() in my compiled node.exe? |
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. |
@bnoordhuis both utf8_len and wchar_len are 2 (for the case of '.') and thats why those checks fail. |
@bnoordhuis so if I take out the length check the filter function works and "." and ".." are not returned. |
wchar_len == 2 for the "." case? What's the value of the second character? |
@bnoordhuis it is the null character, looks like the string we get back is a null-terminated string maybe? |
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( |
@bnoordhuis yeah looks like that one works! |
It has been reported that for SharePoint connections mapped as a drive, uv_fs_scandir() returns "." and ".." entries when the expectation is that they should be filtered out. After some investigation it looks like the driver returns ".\0" and "..\0" for those entries, that is, it includes the zero byte in the filename length. Rewrite the filter to catch those entries as well. Fixes: nodejs/node#4002
Thanks! |
It has been reported that for SharePoint connections mapped as a drive, uv_fs_scandir() returns "." and ".." entries when the expectation is that they should be filtered out. After some investigation it looks like the driver returns ".\0" and "..\0" for those entries, that is, it includes the zero byte in the filename length. Rewrite the filter to catch those entries as well. Fixes: nodejs/node#4002
I didn't know you could close issues with commit in a different repo ! |
Only if you have commit access to both. I'll reopen. =) |
It has been reported that for SharePoint connections mapped as a drive, uv_fs_scandir() returns "." and ".." entries when the expectation is that they should be filtered out. After some investigation it looks like the driver returns ".\0" and "..\0" for those entries, that is, it includes the zero byte in the filename length. Rewrite the filter to catch those entries as well. Fixes: nodejs/node#4002 PR-URL: libuv#636 Reviewed-By: Alexis Campailla <orangemocha@nodejs.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Fixes: nodejs#4002 Fixes: nodejs#5384 Fixes: nodejs#6563 Refs: nodejs#2680 (comment) PR-URL: nodejs#6796 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Fixes: #4002 Fixes: #5384 Fixes: #6563 Refs: #2680 (comment) PR-URL: #6796 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Fixes: nodejs#4002 Fixes: nodejs#5384 Fixes: nodejs#6563 Refs: nodejs#2680 (comment) PR-URL: nodejs#6796 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Fixes: #4002 Fixes: #5384 Fixes: #6563 Refs: #2680 (comment) PR-URL: #6796 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Fixes: #4002 Fixes: #5384 Fixes: #6563 Refs: #2680 (comment) PR-URL: #6796 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Fixes: #4002 Fixes: #5384 Fixes: #6563 Refs: #2680 (comment) PR-URL: #6796 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Fixes: #4002 Fixes: #5384 Fixes: #6563 Refs: #2680 (comment) PR-URL: #6796 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Fixes: #4002 Fixes: #5384 Fixes: #6563 Refs: #2680 (comment) PR-URL: #6796 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Myles Borins <myles.borins@gmail.com>
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.
The text was updated successfully, but these errors were encountered: