[PATCH v4 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> -- v4: msi: Convert newlines to alternate representation when exporting msi: Reject shorter/longer lines in MsiDatabaseImportA 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 | 21 ++++++++++++++++----- dlls/msi/tests/db.c | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/dlls/msi/database.c b/dlls/msi/database.c index 20c74c4d742..9e9356dfd92 100644 --- a/dlls/msi/database.c +++ b/dlls/msi/database.c @@ -377,7 +377,7 @@ 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; @@ -395,9 +395,16 @@ static void parse_line(WCHAR **line, WCHAR ***entries, DWORD *num_entries, DWORD chars_left--; } + /* + * make sure this line has the same number of entries as there are columns + * which are indicated by the first database line. + */ + 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 +449,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 ERROR_SUCCESS; } static WCHAR *build_createsql_prelude(const WCHAR *table) @@ -732,7 +741,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 +793,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> 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 | 18 +++++++++++++++++- dlls/msi/tests/db.c | 12 ++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/dlls/msi/database.c b/dlls/msi/database.c index 9e9356dfd92..786f1e3ad65 100644 --- a/dlls/msi/database.c +++ b/dlls/msi/database.c @@ -892,7 +892,7 @@ end: static UINT export_field( HANDLE handle, MSIRECORD *row, UINT field ) { - char *buffer; + char *buffer, *ptr; BOOL ret; DWORD sz = 0x100; UINT r; @@ -928,6 +928,22 @@ static UINT export_field( HANDLE handle, MSIRECORD *row, UINT field ) return r; } + ptr = buffer; + while( *ptr ) + { + if (*ptr == '\r' && *( ptr + 1 ) == '\n') + { + *ptr++ = '\x11'; + *ptr++ = '\x19'; + continue; + } + + if (*ptr == '\n') + *ptr = '\x19'; + + ptr++; + } + 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:
chars_left--; }
+ /* + * make sure this line has the same number of entries as there are columns + * which are indicated by the first database line.
Much clearer now, thanks. I think the word 'database' is superfluous here though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3133#note_36624
Hans Leidekker (@hans) commented about dlls/msi/database.c:
chars_left--; }
+ /* + * make sure this line has the same number of entries as there are columns + * which are indicated by the first database line. + */ + if (*num_entries && *num_entries != count) + return ERROR_FUNCTION_FAILED; + *entries = malloc(count * sizeof(WCHAR *)); if (!*entries) - return; + return ERROR_OUTOFMEMORY;;
Extra semi-colon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3133#note_36625
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Hans Leidekker (＠hans)