[PATCH v3 0/2] MR3133: msi: handle newlines within field elements in MsiDatabaseImportA/MsiDatabaseExportA
Address the issues around bug 54532 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54532 Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- v3: msi: Convert newlines to alternate representation when exporting https://gitlab.winehq.org/wine/wine/-/merge_requests/3133
From: David Kahurani <k.kahurani(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54532 Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/msi/database.c | 19 ++++++++++++++----- dlls/msi/tests/db.c | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/dlls/msi/database.c b/dlls/msi/database.c index 20c74c4d742..548a66c22ab 100644 --- a/dlls/msi/database.c +++ b/dlls/msi/database.c @@ -377,10 +377,11 @@ done: return wdata; } -static void parse_line(WCHAR **line, WCHAR ***entries, DWORD *num_entries, DWORD *len) +static UINT parse_line(WCHAR **line, WCHAR ***entries, DWORD *num_entries, DWORD *len) { LPWSTR ptr = *line, save; DWORD i, count = 1, chars_left = *len; + UINT res = ERROR_SUCCESS; *entries = NULL; @@ -395,9 +396,13 @@ static void parse_line(WCHAR **line, WCHAR ***entries, DWORD *num_entries, DWORD chars_left--; } + /* catch erroneous lines, albeit later on */ + if (*num_entries && *num_entries != count) + return ERROR_FUNCTION_FAILED; + *entries = malloc(count * sizeof(WCHAR *)); if (!*entries) - return; + return ERROR_OUTOFMEMORY;; /* store pointers into the data */ chars_left = *len; @@ -442,8 +447,10 @@ static void parse_line(WCHAR **line, WCHAR ***entries, DWORD *num_entries, DWORD /* move to the next line if there's more, else EOF */ *line = ptr; *len = chars_left; - if (num_entries) + if (!*num_entries) *num_entries = count; + + return res; } static WCHAR *build_createsql_prelude(const WCHAR *table) @@ -732,7 +739,7 @@ done: static UINT MSI_DatabaseImport(MSIDATABASE *db, LPCWSTR folder, LPCWSTR file) { UINT r; - DWORD len, i, num_labels, num_types, num_columns, num_records = 0; + DWORD len, i, num_labels = 0, num_types = 0, num_columns = 0, num_records = 0; WCHAR **columns, **types, **labels, *path, *ptr, *data, ***records = NULL, ***temp_records; TRACE("%p %s %s\n", db, debugstr_w(folder), debugstr_w(file) ); @@ -784,7 +791,9 @@ static UINT MSI_DatabaseImport(MSIDATABASE *db, LPCWSTR folder, LPCWSTR file) /* read in the table records */ while (len) { - parse_line( &ptr, &records[num_records], NULL, &len ); + r = parse_line( &ptr, &records[num_records], &num_columns, &len ); + if (r != ERROR_SUCCESS) + goto done; num_records++; temp_records = realloc(records, (num_records + 1) * sizeof(WCHAR **)); diff --git a/dlls/msi/tests/db.c b/dlls/msi/tests/db.c index 59dcf87a47c..29b8d45e25a 100644 --- a/dlls/msi/tests/db.c +++ b/dlls/msi/tests/db.c @@ -8584,6 +8584,12 @@ static void test_embedded_nulls(void) "s72\tL0\n" "Control\tDialog\n" "LicenseAgreementDlg\ttext\x11\x19text\0text"; + /* newlines have alternate representation in idt files */ + static const char control_table2[] = + "Dialog\tText\n" + "s72\tL0\n" + "Control\tDialog\n" + "LicenseAgreementDlg\ttext\x11\x19te\nxt\0text"; UINT r; DWORD sz; MSIHANDLE hdb, hrec; @@ -8610,6 +8616,18 @@ static void test_embedded_nulls(void) MsiCloseHandle( hrec ); MsiCloseHandle( hdb ); DeleteFileA( msifile ); + + r = MsiOpenDatabaseW( msifileW, MSIDBOPEN_CREATE, &hdb ); + ok( r == ERROR_SUCCESS, "failed to open database %u\n", r ); + + GetCurrentDirectoryA( MAX_PATH, CURR_DIR ); + write_file( "temp_file", control_table2, sizeof(control_table2) ); + r = MsiDatabaseImportA( hdb, CURR_DIR, "temp_file" ); + ok( r == ERROR_FUNCTION_FAILED, "failed to import table %u\n", r ); + DeleteFileA( "temp_file" ); + + MsiCloseHandle( hdb ); + DeleteFileA( msifile ); } static void test_select_column_names(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3133
From: David Kahurani <k.kahurani(a)gmail.com> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/msi/database.c | 20 +++++++++++++++++--- dlls/msi/tests/db.c | 12 ++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/dlls/msi/database.c b/dlls/msi/database.c index 548a66c22ab..8c99ef677e1 100644 --- a/dlls/msi/database.c +++ b/dlls/msi/database.c @@ -890,7 +890,7 @@ end: static UINT export_field( HANDLE handle, MSIRECORD *row, UINT field ) { - char *buffer; + char *buffer, *tmp; BOOL ret; DWORD sz = 0x100; UINT r; @@ -902,8 +902,6 @@ static UINT export_field( HANDLE handle, MSIRECORD *row, UINT field ) r = MSI_RecordGetStringA( row, field, buffer, &sz ); if (r == ERROR_MORE_DATA) { - char *tmp; - sz++; /* leave room for NULL terminator */ tmp = realloc( buffer, sz ); if (!tmp) @@ -926,6 +924,22 @@ static UINT export_field( HANDLE handle, MSIRECORD *row, UINT field ) return r; } + tmp = buffer; + while( *tmp ) + { + if (*tmp == '\r' && *( tmp + 1 ) == '\n') + { + *tmp++ = '\x11'; + *tmp++ = '\x19'; + continue; + } + + if (*tmp == '\n') + *tmp = '\x19'; + + tmp++; + } + ret = WriteFile( handle, buffer, sz, &sz, NULL ); free( buffer ); return ret ? ERROR_SUCCESS : ERROR_FUNCTION_FAILED; diff --git a/dlls/msi/tests/db.c b/dlls/msi/tests/db.c index 29b8d45e25a..22fe92688ad 100644 --- a/dlls/msi/tests/db.c +++ b/dlls/msi/tests/db.c @@ -8584,12 +8584,18 @@ static void test_embedded_nulls(void) "s72\tL0\n" "Control\tDialog\n" "LicenseAgreementDlg\ttext\x11\x19text\0text"; + static const char export_expected[] = + "Dialog\tText\r\n" + "s72\tL0\r\n" + "Control\tDialog\r\n" + "LicenseAgreementDlg\ttext\x11\x19text\x19text"; /* newlines have alternate representation in idt files */ static const char control_table2[] = "Dialog\tText\n" "s72\tL0\n" "Control\tDialog\n" "LicenseAgreementDlg\ttext\x11\x19te\nxt\0text"; + char data[1024]; UINT r; DWORD sz; MSIHANDLE hdb, hrec; @@ -8613,6 +8619,12 @@ static void test_embedded_nulls(void) ok( r == ERROR_SUCCESS, "failed to get string %u\n", r ); ok( !memcmp( "text\r\ntext\ntext", buffer, sizeof("text\r\ntext\ntext") - 1 ), "wrong buffer contents \"%s\"\n", buffer ); + r = MsiDatabaseExportA( hdb, "Control", CURR_DIR, "temp_file1"); + ok( r == ERROR_SUCCESS, "failed to export table %u\n", r ); + read_file_data( "temp_file1", data ); + ok( !memcmp( data, export_expected, sizeof(export_expected) - 1), "expected: \"%s\" got: \"%s\"\n", export_expected, data ); + DeleteFileA( "temp_file1" ); + MsiCloseHandle( hrec ); MsiCloseHandle( hdb ); DeleteFileA( msifile ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3133
Hans Leidekker (@hans) commented about dlls/msi/database.c:
return wdata; }
-static void parse_line(WCHAR **line, WCHAR ***entries, DWORD *num_entries, DWORD *len) +static UINT parse_line(WCHAR **line, WCHAR ***entries, DWORD *num_entries, DWORD *len) { LPWSTR ptr = *line, save; DWORD i, count = 1, chars_left = *len; + UINT res = ERROR_SUCCESS;
You don't need this variable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3133#note_36581
Hans Leidekker (@hans) commented about dlls/msi/database.c:
chars_left--; }
+ /* catch erroneous lines, albeit later on */ + if (*num_entries && *num_entries != count) + return ERROR_FUNCTION_FAILED;
Something like /* make sure this line has the same number of entries as the previous one */ would be clearer IMO. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3133#note_36582
Hans Leidekker (@hans) commented about dlls/msi/database.c:
return r; }
+ tmp = buffer; + while( *tmp ) + { + if (*tmp == '\r' && *( tmp + 1 ) == '\n')
Please use a new variable such as 'ptr' instead of reusing 'tmp'. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3133#note_36583
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Hans Leidekker (＠hans)