[PATCH 1/2] setupapi/tests: Add a test for SetupGetStringField with strings with embedded \0.

[Dmitry Timoshkov]

Signed-off-by: Dmitry Timoshkov 
---
 dlls/setupapi/tests/parser.c | 69 +++++++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 9 deletions(-)

diff --git a/dlls/setupapi/tests/parser.c b/dlls/setupapi/tests/parser.c
index 06c674aa16..4f7736473c 100644
--- a/dlls/setupapi/tests/parser.c
+++ b/dlls/setupapi/tests/parser.c
@@ -71,13 +71,14 @@ static const char tmpfilename[] = ".\tmp.inf";
                     "verybig=" A1200 "\n"

/* create a new file with specified contents and open it */
-static HINF test_file_contents( const char *data, UINT *err_line )
+static HINF test_file_contents( const char *data, int size, UINT *err_line )
 {
     DWORD res;
     HANDLE handle = CreateFileA( tmpfilename, GENERIC_READ|GENERIC_WRITE,
                                  FILE_SHARE_READ|FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, 0, 0 );
     if (handle == INVALID_HANDLE_VALUE) return 0;
-    if (!WriteFile( handle, data, strlen(data), &res, NULL )) trace( "write error\n" );
+    if (size < 0) size = strlen(data);
+    if (!WriteFile( handle, data, size, &res, NULL )) trace( "write error\n" );
     CloseHandle( handle );
     return SetupOpenInfFileA( tmpfilename, 0, INF_STYLE_WIN4, err_line );
 }
@@ -159,7 +160,7 @@ static void test_invalid_files(void)
     {
         SetLastError( 0xdeadbeef );
         err_line = 0xdeadbeef;
-        hinf = test_file_contents( invalid_files[i].data, &err_line );
+        hinf = test_file_contents( invalid_files[i].data, -1, &err_line );
         err = GetLastError();
         trace( "hinf=%p err=0x%x line=%d\n", hinf, err, err_line );
         if (invalid_files[i].error)  /* should fail */
@@ -233,7 +234,7 @@ static void test_section_names(void)
     for (i = 0; i < ARRAY_SIZE(section_names); i++)
     {
         SetLastError( 0xdeadbeef );
-        hinf = test_file_contents( section_names[i].data, &err_line );
+        hinf = test_file_contents( section_names[i].data, -1, &err_line );
         ok( hinf != INVALID_HANDLE_VALUE, "line %u: open failed err %u\n", i, GetLastError() );
         if (hinf == INVALID_HANDLE_VALUE) continue;

@@ -273,7 +274,7 @@ static void test_enum_sections(void)
         return;
     }

-    hinf = test_file_contents( contents, &err );
+    hinf = test_file_contents( contents, -1, &err );
     ok( hinf != NULL, "Expected valid INF file\n" );

for (index = 0; ; index++)
@@ -424,7 +425,7 @@ static void test_key_names(void)
         strcpy( buffer, STD_HEADER "[Test]\n" );
         strcat( buffer, key_names[i].data );
         SetLastError( 0xdeadbeef );
-        hinf = test_file_contents( buffer, &err_line );
+        hinf = test_file_contents( buffer, -1, &err_line );
         ok( hinf != INVALID_HANDLE_VALUE, "line %u: open failed err %u\n", i, GetLastError() );
         if (hinf == INVALID_HANDLE_VALUE) continue;

@@ -560,7 +561,7 @@ static void test_pSetupGetField(void)
         unicode = FALSE;
     }

-    hinf = test_file_contents( contents, &err );
+    hinf = test_file_contents( contents, -1, &err );
     ok( hinf != NULL, "Expected valid INF file\n" );

ret = SetupFindFirstLineA( hinf, "FileBranchInfo", NULL, &context );
@@ -651,7 +652,7 @@ static void test_SetupGetIntField(void)
         strcat( buffer, keys[i].key );
         strcat( buffer, "=" );
         strcat( buffer, keys[i].fields );
-        hinf = test_file_contents( buffer, &err);
+        hinf = test_file_contents( buffer, -1, &err);
         ok( hinf != NULL, "Expected valid INF file\n" );

SetupFindFirstLineA( hinf, "TestSection", keys[i].key, &context );
@@ -695,7 +696,7 @@ static void test_GLE(void)
     int bufsize = MAX_INF_STRING_LENGTH;
     DWORD retsize;

-    hinf = test_file_contents( inf, &err );
+    hinf = test_file_contents( inf, -1, &err );
     ok( hinf != NULL, "Expected valid INF file\n" );

SetLastError(0xdeadbeef);
@@ -791,6 +792,55 @@ static void test_GLE(void)
     SetupCloseInfFile( hinf );
 }

+static void test_SetupGetStringField(void)
+{
+    static const char infdata[] =
+        "[Version]\n"
+        "Signature="$CHICAGO$"\n"
+        "[MSIinstaller]\n"
+        "Name1 = "%COMPANY_NAME%.Admin\InstallerData\%PRODUCT_NAME%\%VersionString%_%ProductCode1%\Sources\%SETUP_NAME%"\n"
+        "Name2 = "%COMPANY_NAME%.Admin\InstallerData\%PRODUCT_NAME%\%VersionString%_%ProductCode2%\Sources\%SETUP_NAME%"\n"
+        "Name3 = "%COMPANY_NAME%.Admin\InstallerData\%PRODUCT_NAME%\%VersionString%_%ProductCode3%\Sources\%SETUP_NAME%"\n"
+        "[Strings]\n"
+        "VersionString=1.2.3.45678\0\n"
+        "ProductCode1={11111111-1234-5678-1234-567812345678}\0\n"
+        "ProductCode2={22222222-1234-5678-1234\0-567812345678}\0\n"
+        "ProductCode3={33333333-1234-5678-1234-567812345678}  \0\n"
+        "COMPANY_NAME = "Company Name"\n"
+        "PRODUCT_NAME = "Product Name"\n"
+        "SETUP_NAME = "Setup.msi"\n";
+    HINF hinf;
+    UINT err, ret;
+    INFCONTEXT context;
+    char buf[MAX_INF_STRING_LENGTH];
+
+    hinf = test_file_contents(infdata, sizeof(infdata), &err);
+    ok(hinf != NULL, "Expected valid INF file\n");
+
+    ret = SetupFindFirstLineA(hinf, "MSIinstaller", "Name1", &context);
+    ok(ret, "SetupFindFirstLine error %u\n", GetLastError());
+    ret = SetupGetStringFieldA(&context, 1, buf, sizeof(buf), NULL);
+    ok(ret, "SetupGetStringField error %u\n", GetLastError());
+todo_wine
+    ok(!strcmp(buf, "Company Name.Admin\InstallerData\Product Name\1.2.3.45678_{11111111-1234-5678-1234-567812345678}\Sources\Setup.msi"), "got %s\n", buf);
+
+    ret = SetupFindFirstLineA(hinf, "MSIinstaller", "Name2", &context);
+    ok(ret, "SetupFindFirstLine error %u\n", GetLastError());
+    ret = SetupGetStringFieldA(&context, 1, buf, sizeof(buf), NULL);
+    ok(ret, "SetupGetStringField error %u\n", GetLastError());
+todo_wine
+    ok(!strcmp(buf, "Company Name.Admin\InstallerData\Product Name\1.2.3.45678_{22222222-1234-5678-1234 -567812345678}\Sources\Setup.msi"), "got %s\n", buf);
+
+    ret = SetupFindFirstLineA(hinf, "MSIinstaller", "Name3", &context);
+    ok(ret, "SetupFindFirstLine error %u\n", GetLastError());
+    ret = SetupGetStringFieldA(&context, 1, buf, sizeof(buf), NULL);
+    ok(ret, "SetupGetStringField error %u\n", GetLastError());
+todo_wine
+    ok(!strcmp(buf, "Company Name.Admin\InstallerData\Product Name\1.2.3.45678_{33333333-1234-5678-1234-567812345678}\Sources\Setup.msi"), "got %s\n", buf);
+
+    SetupCloseInfFile(hinf);
+}
+
 START_TEST(parser)
 {
     init_function_pointers();
@@ -801,6 +851,7 @@ START_TEST(parser)
     test_close_inf_file();
     test_pSetupGetField();
     test_SetupGetIntField();
+    test_SetupGetStringField();
     test_GLE();
     DeleteFileA( tmpfilename );
 }

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> Signed-off-by: Dmitry Timoshkov
>
> dlls/setupapi/tests/parser.c | 69 +++++++++++++++++++++++++++++++-----
>  1 file changed, 60 insertions(+), 9 deletions(-)

Please update the existing parser tests instead. And if null chars need
special handling this would need to be done as part of the parser state
machine.

[Dmitry Timoshkov]

Alexandre Julliard  wrote:
> Dmitry Timoshkov  writes:
> > Signed-off-by: Dmitry Timoshkov
> >
> > dlls/setupapi/tests/parser.c | 69 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 60 insertions(+), 9 deletions(-)
>
> Please update the existing parser tests instead.

Unfortunately existing tests depend on test data being NUL terminated,
and even the test that presumably is supposed to test embedded NUL
(test_key_names() with "abcd=ef\x0gh") fails to do so because of strcpy/
strcat used to create test file contents. That's why I decided to go
a simpler way and just create new test from scratch.
> And if null chars need
> special handling this would need to be done as part of the parser state
> machine.

Thanks.

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> Alexandre Julliard  wrote:
> > Dmitry Timoshkov  writes:
> > > Signed-off-by: Dmitry Timoshkov
> > >
> > > dlls/setupapi/tests/parser.c | 69 +++++++++++++++++++++++++++++++-----
> > >  1 file changed, 60 insertions(+), 9 deletions(-)
> >
> > Please update the existing parser tests instead.
>
> Unfortunately existing tests depend on test data being NUL terminated,
> and even the test that presumably is supposed to test embedded NUL
> (test_key_names() with "abcd=ef\x0gh") fails to do so because of strcpy/
> strcat used to create test file contents. That's why I decided to go
> a simpler way and just create new test from scratch.

Yes, that test is currently broken. It would be better to fix it and
then extend it, than to leave it broken and create a whole new function
just for this.

[Dmitry Timoshkov]

Alexandre Julliard  wrote:
> > > > dlls/setupapi/tests/parser.c | 69 +++++++++++++++++++++++++++++++-----
> > > >  1 file changed, 60 insertions(+), 9 deletions(-)
> > >
> > > Please update the existing parser tests instead.
> >
> > Unfortunately existing tests depend on test data being NUL terminated,
> > and even the test that presumably is supposed to test embedded NUL
> > (test_key_names() with "abcd=ef\x0gh") fails to do so because of strcpy/
> > strcat used to create test file contents. That's why I decided to go
> > a simpler way and just create new test from scratch.
>
> Yes, that test is currently broken. It would be better to fix it and
> then extend it, than to leave it broken and create a whole new function
> just for this.

Since I'm interested to see the patch with the fix accepted, then if there's
nothing wrong with the fix I'd appreciate if it could be commited. Then I'd
take a look at the fixing the tests and adding some new ones a bit later.