[PATCH v3 0/2] MR898: scrrun: Fix bugs in FileSystemObject text-file handling

[Kevin Puetz (＠puetzk)]

Fix bugs in FileSystemObject text-file handling

OpenTextFile(...,ForWriting,True) should either create a new file,
or open and truncate an existing one

OpenTextFile(...,ForAppending,?,True) should write a BOM
if appending to an existing-but-empty file

--
  v3: scrrun: Fix bugs in FileSystemObject text-file handling
      scrrun/tests: Introduce a test_file_contents helper

https://gitlab.winehq.org/wine/wine/-/merge_requests/898

[PATCH v3 1/2] scrrun/tests: Introduce a test_file_contents helper

[Kevin Puetz]

From: Kevin Puetz

---
 dlls/scrrun/tests/filesystem.c | 97 ++++++++++++++--------------------
 1 file changed, 41 insertions(+), 56 deletions(-)

diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c
index f2e23e4011c..41d7f8b5500 100644
--- a/dlls/scrrun/tests/filesystem.c
+++ b/dlls/scrrun/tests/filesystem.c
@@ -62,6 +62,32 @@ static inline void get_temp_path(const WCHAR *prefix, WCHAR *path)
     DeleteFileW(path);
 }

+#define test_file_contents(a, b, c) _test_file_contents(a, b, c, __LINE__)
+static void _test_file_contents(LPCWSTR pathW, DWORD expected_size, const void * expected_contents, int line)
+{
+    HANDLE file;
+    BOOL ret;
+    DWORD actual_size;
+
+    file = CreateFileW(pathW, GENERIC_READ, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+    ok_(__FILE__,line) (file != INVALID_HANDLE_VALUE, "got %p\n", file);
+
+    actual_size = GetFileSize(file,NULL);
+    ok_(__FILE__,line) (actual_size == expected_size, "expected file %s size %ld, got %ld\n", wine_dbgstr_w(pathW), expected_size, actual_size);
+
+    if((expected_contents != NULL) && (actual_size > 0))
+    {
+	DWORD r;
+        void *actual_contents = malloc(actual_size);
+        ret = ReadFile(file, actual_contents, actual_size, &r, NULL);
+        ok_(__FILE__,line) (ret && r == actual_size, "Read %ld, got %d, error %ld.\n", r, ret, GetLastError());
+        ok_(__FILE__,line) (!memcmp(expected_contents, actual_contents, expected_size), "expected file %s contents %s, got %s\n", wine_dbgstr_w(pathW), wine_dbgstr_an(expected_contents,expected_size), wine_dbgstr_an(actual_contents,r));
+        free(actual_contents);
+    }
+
+    CloseHandle(file);
+}
+
 static IDrive *get_fixed_drive(void)
 {
     IDriveCollection *drives;
@@ -1507,13 +1533,12 @@ static void get_temp_filepath(const WCHAR *filename, WCHAR *path, WCHAR *dir)

static void test_CreateTextFile(void)
 {
-    WCHAR pathW[MAX_PATH], dirW[MAX_PATH], buffW[10];
+    WCHAR pathW[MAX_PATH], dirW[MAX_PATH];
     ITextStream *stream;
     BSTR nameW, str;
     HANDLE file;
     HRESULT hr;
     BOOL ret;
-    DWORD r;

get_temp_filepath(testfileW, pathW, dirW);

@@ -1563,15 +1588,7 @@ static void test_CreateTextFile(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ITextStream_Release(stream);

-    /* check contents */
-    file = CreateFileW(pathW, GENERIC_READ, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
-    ok(file != INVALID_HANDLE_VALUE, "got %p\n", file);
-    r = 0;
-    ret = ReadFile(file, buffW, sizeof(buffW), &r, NULL);
-    ok(ret && r == 2, "Read %ld, got %d, error %ld.\n", r, ret, GetLastError());
-
-    ok( buffW[0] == 0xfeff, "got %04x\n", buffW[0] );
-    CloseHandle(file);
+    test_file_contents(pathW,2,L"\ufeff");

DeleteFileW(nameW);
     RemoveDirectoryW(dirW);
@@ -1580,14 +1597,13 @@ static void test_CreateTextFile(void)

static void test_FolderCreateTextFile(void)
 {
-    WCHAR pathW[MAX_PATH], dirW[MAX_PATH], buffW[10], buff2W[10];
+    WCHAR pathW[MAX_PATH], dirW[MAX_PATH];
     ITextStream *stream;
     BSTR nameW, str;
     HANDLE file;
     IFolder *folder;
     HRESULT hr;
     BOOL ret;
-    DWORD r;

get_temp_filepath(testfileW, pathW, dirW);
     nameW = SysAllocString(L"foo.txt");
@@ -1640,18 +1656,7 @@ static void test_FolderCreateTextFile(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ITextStream_Release(stream);

-    /* check contents */
-    file = CreateFileW(pathW, GENERIC_READ, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
-    ok(file != INVALID_HANDLE_VALUE, "got %p\n", file);
-    r = 0;
-    ret = ReadFile(file, buffW, sizeof(buffW), &r, NULL);
-    ok(ret && r == 2, "Read %ld, got %d, error %ld.\n", r, ret, GetLastError());
-    buffW[r/sizeof(WCHAR)] = 0;
-
-    buff2W[0] = 0xfeff;
-    buff2W[1] = 0;
-    ok(!lstrcmpW(buff2W, buffW), "got %s, expected %s\n", wine_dbgstr_w(buffW), wine_dbgstr_w(buff2W));
-    CloseHandle(file);
+    test_file_contents(pathW,2,L"\ufeff");

DeleteFileW(pathW);
     RemoveDirectoryW(dirW);
@@ -1661,11 +1666,10 @@ static void test_FolderCreateTextFile(void)
 static void test_WriteLine(void)
 {
     WCHAR pathW[MAX_PATH], dirW[MAX_PATH];
-    WCHAR buffW[MAX_PATH], buff2W[MAX_PATH];
-    char buffA[MAX_PATH];
+    WCHAR ExpectedW[MAX_PATH];
+    char ExpectedA[MAX_PATH];
     ITextStream *stream;
-    DWORD r, len;
-    HANDLE file;
+    DWORD len;
     BSTR nameW;
     HRESULT hr;
     BOOL ret;
@@ -1684,19 +1688,10 @@ static void test_WriteLine(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ITextStream_Release(stream);

-    /* check contents */
-    file = CreateFileW(pathW, GENERIC_READ, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
-    ok(file != INVALID_HANDLE_VALUE, "got %p\n", file);
-    r = 0;
-    ret = ReadFile(file, buffA, sizeof(buffA), &r, NULL);
-    ok(ret && r, "Read %ld, got %d, error %ld.\n", r, ret, GetLastError());
-
-    len = MultiByteToWideChar(CP_ACP, 0, buffA, r, buffW, ARRAY_SIZE(buffW));
-    buffW[len] = 0;
-    lstrcpyW(buff2W, nameW);
-    lstrcatW(buff2W, L"\r\n");
-    ok(!lstrcmpW(buff2W, buffW), "got %s, expected %s\n", wine_dbgstr_w(buffW), wine_dbgstr_w(buff2W));
-    CloseHandle(file);
+    len = WideCharToMultiByte(CP_ACP, 0, nameW, -1, ExpectedA, ARRAY_SIZE(ExpectedA)-2, NULL, NULL);
+    ok(len > 0, "got %ld", len);
+    lstrcatA(ExpectedA, "\r\n");
+    test_file_contents(pathW,lstrlenA(ExpectedA),ExpectedA);
     DeleteFileW(nameW);

/* same for unicode file */
@@ -1707,20 +1702,10 @@ static void test_WriteLine(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ITextStream_Release(stream);

-    /* check contents */
-    file = CreateFileW(pathW, GENERIC_READ, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
-    ok(file != INVALID_HANDLE_VALUE, "got %p\n", file);
-    r = 0;
-    ret = ReadFile(file, buffW, sizeof(buffW), &r, NULL);
-    ok(ret && r, "Read %ld, got %d, error %ld.\n", r, ret, GetLastError());
-    buffW[r/sizeof(WCHAR)] = 0;
-
-    buff2W[0] = 0xfeff;
-    buff2W[1] = 0;
-    lstrcatW(buff2W, nameW);
-    lstrcatW(buff2W, L"\r\n");
-    ok(!lstrcmpW(buff2W, buffW), "got %s, expected %s\n", wine_dbgstr_w(buffW), wine_dbgstr_w(buff2W));
-    CloseHandle(file);
+    lstrcpyW(ExpectedW, L"\ufeff");
+    lstrcatW(ExpectedW, nameW);
+    lstrcatW(ExpectedW, L"\r\n");
+    test_file_contents(pathW,lstrlenW(ExpectedW)*sizeof(WCHAR),ExpectedW);
     DeleteFileW(nameW);

RemoveDirectoryW(dirW);

[PATCH v3 2/2] scrrun: Fix bugs in FileSystemObject text-file handling

[Kevin Puetz]

From: Kevin Puetz

OpenTextFile(...,ForWriting,True) should either create a new file,
or open and truncate an existing one

OpenTextFile(...,ForAppending,?,True) should write a BOM
if appending to an existing-but-empty file
---
 dlls/scrrun/filesystem.c       | 23 ++++++++++++++++++++---
 dlls/scrrun/tests/filesystem.c | 26 ++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c
index cfeb5bb1532..e53355267db 100644
--- a/dlls/scrrun/filesystem.c
+++ b/dlls/scrrun/filesystem.c
@@ -897,7 +897,7 @@ static HRESULT create_textstream(const WCHAR *filename, DWORD disposition, IOMod
     {
         stream->unicode = format == TristateTrue;
         /* Write Unicode BOM */
-        if (stream->unicode && (disposition == CREATE_ALWAYS || disposition == CREATE_NEW)) {
+        if (stream->unicode && (disposition == CREATE_ALWAYS || disposition == CREATE_NEW || disposition == TRUNCATE_EXISTING)) {
             DWORD written = 0;
             BOOL ret = WriteFile(stream->file, &utf16bom, sizeof(utf16bom), &written, NULL);
             if (!ret || written != sizeof(utf16bom)) {
@@ -939,7 +939,20 @@ static HRESULT create_textstream(const WCHAR *filename, DWORD disposition, IOMod

stream->eof = read != sizeof(buf);
         }
-        else SetFilePointer(stream->file, 0, 0, FILE_END);
+        else
+        {
+            LONG filePosHigh = 0;
+            DWORD filePosLow = SetFilePointer(stream->file, 0, &filePosHigh, FILE_END);
+            if(stream->unicode && filePosHigh == 0 && filePosLow == 0) {
+                /* unicode ForAppending to an empty file, write BOM */
+                DWORD written = 0;
+                BOOL ret = WriteFile(stream->file, &utf16bom, sizeof(utf16bom), &written, NULL);
+                if (!ret || written != sizeof(utf16bom)) {
+                    ITextStream_Release(&stream->ITextStream_iface);
+                    return create_error(GetLastError());
+                }
+            }
+        }
     }

init_classinfo(&CLSID_TextStream, (IUnknown *)&stream->ITextStream_iface, &stream->classinfo);
@@ -4049,7 +4062,11 @@ static HRESULT WINAPI filesys_OpenTextFile(IFileSystem3 *iface, BSTR filename,

TRACE("(%p)->(%s %d %d %d %p)\n", iface, debugstr_w(filename), mode, create, format, stream);

-    disposition = create == VARIANT_TRUE ? OPEN_ALWAYS : OPEN_EXISTING;
+    if(mode == ForWriting) {
+        disposition = create == VARIANT_TRUE ? CREATE_ALWAYS : TRUNCATE_EXISTING;
+    } else {
+        disposition = create == VARIANT_TRUE ? OPEN_ALWAYS : OPEN_EXISTING;
+    }
     return create_textstream(filename, disposition, mode, format, stream);
 }

diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c
index 41d7f8b5500..e53bd092ce9 100644
--- a/dlls/scrrun/tests/filesystem.c
+++ b/dlls/scrrun/tests/filesystem.c
@@ -263,6 +263,7 @@ static void test_createfolder(void)

static void test_textstream(void)
 {
+    WCHAR ExpectedW[10];
     ITextStream *stream;
     VARIANT_BOOL b;
     DWORD written;
@@ -378,6 +379,31 @@ todo_wine {
     ok(ret && written == sizeof(testfileW), "got %d\n", ret);
     CloseHandle(file);

+    /* opening a non-empty file (from above) for writing should truncate it */
+    hr = IFileSystem3_OpenTextFile(fs3, name, ForWriting, VARIANT_FALSE, TristateFalse, &stream);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ITextStream_Release(stream);
+
+    test_file_contents(testfileW,0,"");
+
+    /* appending to an empty file file and specifying unicode should immediately write a BOM */
+    hr = IFileSystem3_OpenTextFile(fs3, name, ForAppending, VARIANT_FALSE, TristateTrue, &stream);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ITextStream_Release(stream);
+
+    test_file_contents(testfileW,2,L"\ufeff");
+
+    /* appending to a file that contains a BOM should detect unicode mode, but not write a second BOM */
+    hr = IFileSystem3_OpenTextFile(fs3, name, ForAppending, VARIANT_FALSE, TristateUseDefault, &stream);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = ITextStream_Write(stream, name);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ITextStream_Release(stream);
+
+    lstrcpyW(ExpectedW, L"\ufeff");
+    lstrcatW(ExpectedW, name);
+    test_file_contents(testfileW,lstrlenW(ExpectedW)*sizeof(WCHAR),ExpectedW);
+
     hr = IFileSystem3_OpenTextFile(fs3, name, ForReading, VARIANT_FALSE, TristateFalse, &stream);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     b = 10;