win: fs: fix FILE_FLAG_NO_BUFFERING for writes

On Windows, `fs__open()` maps `UV_FS_O_DIRECT` to
`FILE_FLAG_NO_BUFFERING`.

When `access` is only `FILE_GENERIC_READ` this succeeds, but when
`access` is `FILE_GENERIC_WRITE` this returns an error:

```
0x00000057, ERROR_INVALID_PARAMETER, The parameter is incorrect.
```

The reason is that `FILE_GENERIC_WRITE` includes `FILE_APPEND_DATA`,
but `FILE_APPEND_DATA` and `FILE_FLAG_NO_BUFFERING` are mutually
exclusive:

```
FILE_GENERIC_WRITE = STANDARD_RIGHTS_WRITE |
                     FILE_WRITE_DATA |
                     FILE_WRITE_ATTRIBUTES |
                     FILE_WRITE_EA |
                     FILE_APPEND_DATA |
                     SYNCHRONIZE
```

This incompatibility between access and attribute flags does not appear
to be documented by Microsoft for `FILE_FLAG_NO_BUFFERING` but it is
indirectly documented under [NtCreateFile](https://bit.ly/2rm5wRT):

```
FILE_NO_INTERMEDIATE_BUFFERING
The file cannot be cached or buffered in a driver's internal buffers.
This flag is incompatible with the DesiredAccess FILE_APPEND_DATA flag.
```

The solution is to remove `FILE_APPEND_DATA` from the access flags when
`FILE_FLAG_NO_BUFFERING` is set. Note that this does not prevent
appends, since `FILE_GENERIC_WRITE` also includes `FILE_WRITE_DATA`,
which in turn allows appends.

PR-URL: libuv#2102
Reviewed-By: Bartosz Sosnowski <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

src/win/fs.c

@@ -510,6 +510,33 @@ void fs__open(uv_fs_t* req) {
   }
 
   if (flags & UV_FS_O_DIRECT) {
+    /*
+     * FILE_APPEND_DATA and FILE_FLAG_NO_BUFFERING are mutually exclusive.
+     * Windows returns 87, ERROR_INVALID_PARAMETER if these are combined.
+     *
+     * FILE_APPEND_DATA is included in FILE_GENERIC_WRITE:
+     *
+     * FILE_GENERIC_WRITE = STANDARD_RIGHTS_WRITE |
+     *                      FILE_WRITE_DATA |
+     *                      FILE_WRITE_ATTRIBUTES |
+     *                      FILE_WRITE_EA |
+     *                      FILE_APPEND_DATA |
+     *                      SYNCHRONIZE
+     *
+     * Note: Appends are also permitted by FILE_WRITE_DATA.
+     *
+     * In order for direct writes and direct appends to succeed, we therefore
+     * exclude FILE_APPEND_DATA if FILE_WRITE_DATA is specified, and otherwise
+     * fail if the user's sole permission is a direct append, since this
+     * particular combination is invalid.
+     */
+    if (access & FILE_APPEND_DATA) {
+      if (access & FILE_WRITE_DATA) {
+        access &= ~FILE_APPEND_DATA;
+      } else {
+        goto einval;
+      }
+    }
     attributes |= FILE_FLAG_NO_BUFFERING;
   }
 

test/test-fs.c

@@ -3600,6 +3600,53 @@ TEST_IMPL(fs_exclusive_sharing_mode) {
 }
 #endif
 
+#ifdef _WIN32
+TEST_IMPL(fs_file_flag_no_buffering) {
+  int r;
+
+  /* Setup. */
+  unlink("test_file");
+
+  ASSERT(UV_FS_O_APPEND > 0);
+  ASSERT(UV_FS_O_CREAT > 0);
+  ASSERT(UV_FS_O_DIRECT > 0);
+  ASSERT(UV_FS_O_RDWR > 0);
+
+  /* FILE_APPEND_DATA must be excluded from FILE_GENERIC_WRITE: */
+  r = uv_fs_open(NULL,
+                 &open_req1,
+                 "test_file",
+                 UV_FS_O_RDWR | UV_FS_O_CREAT | UV_FS_O_DIRECT,
+                 S_IWUSR | S_IRUSR,
+                 NULL);
+  ASSERT(r >= 0);
+  ASSERT(open_req1.result >= 0);
+  uv_fs_req_cleanup(&open_req1);
+
+  r = uv_fs_close(NULL, &close_req, open_req1.result, NULL);
+  ASSERT(r == 0);
+  ASSERT(close_req.result == 0);
+  uv_fs_req_cleanup(&close_req);
+
+  /* FILE_APPEND_DATA and FILE_FLAG_NO_BUFFERING are mutually exclusive: */
+  r = uv_fs_open(NULL,
+                 &open_req2,
+                 "test_file",
+                 UV_FS_O_APPEND | UV_FS_O_DIRECT,
+                 S_IWUSR | S_IRUSR,
+                 NULL);
+  ASSERT(r == UV_EINVAL);
+  ASSERT(open_req2.result == UV_EINVAL);
+  uv_fs_req_cleanup(&open_req2);
+
+  /* Cleanup */
+  unlink("test_file");
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
+#endif
+
 #ifdef _WIN32
 int call_icacls(const char* command, ...) {
     char icacls_command[1024];

test/test-list.h

@@ -349,6 +349,7 @@ TEST_DECLARE   (fs_null_req)
 TEST_DECLARE   (fs_read_dir)
 #ifdef _WIN32
 TEST_DECLARE   (fs_exclusive_sharing_mode)
+TEST_DECLARE   (fs_file_flag_no_buffering)
 TEST_DECLARE   (fs_open_readonly_acl)
 TEST_DECLARE   (fs_fchmod_archive_readonly)
 #endif
@@ -905,6 +906,7 @@ TASK_LIST_START
   TEST_ENTRY  (fs_read_dir)
 #ifdef _WIN32
   TEST_ENTRY  (fs_exclusive_sharing_mode)
+  TEST_ENTRY  (fs_file_flag_no_buffering)
   TEST_ENTRY  (fs_open_readonly_acl)
   TEST_ENTRY  (fs_fchmod_archive_readonly)
 #endif