Hi! Please comment on "try 2" of the patch. Will you accept it?
IB> Hi! This should expose this bug: IB> http://bugs.winehq.org/show_bug.cgi?id=19385 ( the 'wine start' IB> launcher does not open MS Office documents that have spaces in their IB> path ). Although the title is misleading, many other tickets are IB> marked as duplicates of it. IB> It's not about spaces. It's about sometimes unneeded parsing of IB> SHELLEXECUTEINFO.lpFile, trying to extract arguments from it. Why? IB> Or maybe some other part of wine relies on this behavior?
IB> Succeeds on 98 se, 2k & xp. IB> 4 tests fail on wine IB> --- IB> dlls/shell32/tests/shlexec.c | 87 ++++++++++++++++++++++++++++++++++++++++++ IB> 1 files changed, 87 insertions(+), 0 deletions(-)
IB> diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c IB> index c673f0a..d575d22 100644 IB> --- a/dlls/shell32/tests/shlexec.c IB> +++ b/dlls/shell32/tests/shlexec.c IB> @@ -797,6 +797,10 @@ static const char* testfiles[]= IB> "%s\test file.sde", IB> "%s\test file.exe", IB> "%s\test2.exe", IB> + "%s\simple.shlexec", IB> + "%s\drawback_file.noassoc", IB> + "%s\drawback_file.noassoc foo.shlexec", IB> + "%s\drawback_nonexist.noassoc foo.shlexec", IB> NULL IB> }; IB> IB> @@ -852,6 +856,88 @@ static filename_tests_t noquotes_tests[]= IB> {NULL, NULL, 0} IB> }; IB> IB> +static void test_lpFile_parsed(void) IB> +{ IB> + /* basename tmpdir */ IB> + const char* shorttmpdir; IB> + IB> + const char *testfile; IB> + char fileA[MAX_PATH]; IB> + IB> + int rc; IB> + int expected; IB> + IB> + HMODULE hkernel32 = GetModuleHandle("kernel32"); IB> + BOOL isreallyXP2k3plus = NULL != GetProcAddress(hkernel32, "AttachConsole"); IB> + IB> + GetTempPathA(sizeof(fileA), fileA); IB> + shorttmpdir = tmpdir + strlen(fileA); IB> + IB> + /* ensure tmpdir is in %TEMP%: GetTempPath() can succeed even if TEMP is undefined */ IB> + SetEnvironmentVariableA("TEMP", fileA); IB> + IB> + #define TEST_LPFILE_PARSED_OK_() (rc==expected || rc>32 && expected>32) IB> + #define TEST_LPFILE_PARSED_OK() ok(TEST_LPFILE_PARSED_OK_(), "expected %s (%d), got %s (%d), lpFile: %s \n", expected==33 ? "success" : "failure", expected, rc > 32 ? "success" : "failure", rc, fileA) IB> + IB> + /* existing "drawback_file.noassoc" prevents finding "drawback_file.noassoc foo.shlexec" on wine */ IB> + testfile = "%s\drawback_file.noassoc foo.shlexec"; IB> + expected = 33; sprintf(fileA, testfile, tmpdir); IB> + rc=shell_execute(NULL, fileA, NULL, NULL); IB> + todo_wine { TEST_LPFILE_PARSED_OK(); } IB> + IB> + /* if quoted, existing "drawback_file.noassoc" not prevents finding "drawback_file.noassoc foo.shlexec" on wine */ IB> + testfile = ""%s\drawback_file.noassoc foo.shlexec""; IB> + expected = 33; sprintf(fileA, testfile, tmpdir); IB> + rc=shell_execute(NULL, fileA, NULL, NULL); IB> + TEST_LPFILE_PARSED_OK(); IB> + IB> + /* error should be 2, not 31 */ IB> + testfile = ""%s\drawback_file.noassoc" foo.shlexec"; IB> + expected = 2; sprintf(fileA, testfile, tmpdir); IB> + rc=shell_execute(NULL, fileA, NULL, NULL); IB> + TEST_LPFILE_PARSED_OK(); IB> + IB> + /* ""command"" not works on wine (and real win9x and w2k) */ IB> + if (isreallyXP2k3plus) { IB> + testfile = """%s\simple.shlexec"""; IB> + expected = 33; sprintf(fileA, testfile, tmpdir); IB> + rc=shell_execute(NULL, fileA, NULL, NULL); IB> + todo_wine { TEST_LPFILE_PARSED_OK(); } IB> + } else { IB> + win_skip("ShellExecute('""command""', ...) only works on XP/2k3 or newer\n"); IB> + } IB> + IB> + /* nonexisting "drawback_nonexist.noassoc" not prevents finding "drawback_nonexist.noassoc foo.shlexec" on wine */ IB> + testfile = "%s\drawback_nonexist.noassoc foo.shlexec"; IB> + expected = 33; sprintf(fileA, testfile, tmpdir); IB> + rc=shell_execute(NULL, fileA, NULL, NULL); IB> + TEST_LPFILE_PARSED_OK(); IB> + IB> + /* is SEE_MASK_DOENVSUBST default flag? Should only be when XP emulates 9x (XP bug or real 95 or ME behavior ?) */ IB> + testfile = "%%TEMP%%\%s\simple.shlexec"; IB> + expected = 2; sprintf(fileA, testfile, shorttmpdir); IB> + rc=shell_execute(NULL, fileA, NULL, NULL); IB> + todo_wine { TEST_LPFILE_PARSED_OK(); } IB> + IB> + /* quoted */ IB> + testfile = ""%%TEMP%%\%s\simple.shlexec""; IB> + expected = 2; sprintf(fileA, testfile, shorttmpdir); IB> + rc=shell_execute(NULL, fileA, NULL, NULL); IB> + todo_wine { TEST_LPFILE_PARSED_OK(); } IB> + IB> + /* test SEE_MASK_DOENVSUBST works */ IB> + testfile = "%%TEMP%%\%s\simple.shlexec"; IB> + expected = 33; sprintf(fileA, testfile, shorttmpdir); IB> + rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI, NULL, fileA, NULL, NULL); IB> + TEST_LPFILE_PARSED_OK(); IB> + IB> + /* quoted */ IB> + testfile = ""%%TEMP%%\%s\simple.shlexec""; IB> + expected = 33; sprintf(fileA, testfile, shorttmpdir); IB> + rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI, NULL, fileA, NULL, NULL); IB> + TEST_LPFILE_PARSED_OK(); IB> +} IB> + IB> static void test_filename(void) IB> { IB> char filename[MAX_PATH]; IB> @@ -1938,6 +2024,7 @@ START_TEST(shlexec) IB> IB> init_test(); IB> IB> + test_lpFile_parsed(); IB> test_filename(); IB> test_find_executable(); IB> test_lnks();
On 02/17/2010 09:51 AM, Ilya Basin wrote:
Hi! Please comment on "try 2" of the patch. Will you accept it? IB> + /* ensure tmpdir is in %TEMP%: GetTempPath() can succeed even if TEMP is undefined */ IB> + SetEnvironmentVariableA("TEMP", fileA);
I'm still not convinced you need this but I think that's minor.
I ran your tests on winetestbot and there are 2 failures for Win95 and NT4:
https://winetestbot.geldorp.nl/JobDetails.pl?Key=733
PV> On 02/17/2010 09:51 AM, Ilya Basin wrote:
Hi! Please comment on "try 2" of the patch. Will you accept it? IB> + /* ensure tmpdir is in %TEMP%: GetTempPath() can succeed even if TEMP is undefined */ IB> + SetEnvironmentVariableA("TEMP", fileA);
PV> I'm still not convinced you need this but I think that's minor.
PV> I ran your tests on winetestbot and there are 2 failures for Win95 and NT4:
PV> https://winetestbot.geldorp.nl/JobDetails.pl?Key=733
Added 2 win_skip's for these failures. Tested on NT4. Don't have 95 though.
--- dlls/shell32/tests/shlexec.c | 98 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 98 insertions(+), 0 deletions(-) mode change 100644 => 100755 dlls/shell32/tests/shlexec.c
diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c old mode 100644 new mode 100755 index c673f0a..150856c --- a/dlls/shell32/tests/shlexec.c +++ b/dlls/shell32/tests/shlexec.c @@ -797,6 +797,10 @@ static const char* testfiles[]= "%s\test file.sde", "%s\test file.exe", "%s\test2.exe", + "%s\simple.shlexec", + "%s\drawback_file.noassoc", + "%s\drawback_file.noassoc foo.shlexec", + "%s\drawback_nonexist.noassoc foo.shlexec", NULL };
@@ -852,6 +856,99 @@ static filename_tests_t noquotes_tests[]= {NULL, NULL, 0} };
+static void test_lpFile_parsed(void) +{ + /* basename tmpdir */ + const char* shorttmpdir; + + const char *testfile; + char fileA[MAX_PATH]; + + int rc; + int expected; + + HMODULE hkernel32 = GetModuleHandle("kernel32"); + BOOL isreallyXP_2k3_plus = NULL != GetProcAddress(hkernel32, "AttachConsole"); + BOOL isreally98_2k_plus = NULL != GetProcAddress(hkernel32, "GetLongPathNameA"); + + GetTempPathA(sizeof(fileA), fileA); + shorttmpdir = tmpdir + strlen(fileA); + + /* ensure tmpdir is in %TEMP%: GetTempPath() can succeed even if TEMP is undefined */ + SetEnvironmentVariableA("TEMP", fileA); + + #define TEST_LPFILE_PARSED_OK_() (rc==expected || rc>32 && expected>32) + #define TEST_LPFILE_PARSED_OK() ok(TEST_LPFILE_PARSED_OK_(), "expected %s (%d), got %s (%d), lpFile: %s \n", expected==33 ? "success" : "failure", expected, rc > 32 ? "success" : "failure", rc, fileA) + + /* existing "drawback_file.noassoc" prevents finding "drawback_file.noassoc foo.shlexec" on wine */ + testfile = "%s\drawback_file.noassoc foo.shlexec"; + expected = 33; sprintf(fileA, testfile, tmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + todo_wine { TEST_LPFILE_PARSED_OK(); } + + /* quoted lpFile not works on real win95 and nt4 */ + if (isreally98_2k_plus) { + /* if quoted, existing "drawback_file.noassoc" not prevents finding "drawback_file.noassoc foo.shlexec" on wine */ + testfile = ""%s\drawback_file.noassoc foo.shlexec""; + expected = 33; sprintf(fileA, testfile, tmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); + } else { + win_skip("ShellExecute('"command"', ...) only works on 98/2k or newer\n"); + } + + /* error should be 2, not 31 */ + testfile = ""%s\drawback_file.noassoc" foo.shlexec"; + expected = 2; sprintf(fileA, testfile, tmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); + + /* ""command"" not works on wine (and real win9x and w2k) */ + if (isreallyXP_2k3_plus) { + testfile = """%s\simple.shlexec"""; + expected = 33; sprintf(fileA, testfile, tmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + todo_wine { TEST_LPFILE_PARSED_OK(); } + } else { + win_skip("ShellExecute('""command""', ...) only works on XP/2k3 or newer\n"); + } + + /* nonexisting "drawback_nonexist.noassoc" not prevents finding "drawback_nonexist.noassoc foo.shlexec" on wine */ + testfile = "%s\drawback_nonexist.noassoc foo.shlexec"; + expected = 33; sprintf(fileA, testfile, tmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); + + /* is SEE_MASK_DOENVSUBST default flag? Should only be when XP emulates 9x (XP bug or real 95 or ME behavior ?) */ + testfile = "%%TEMP%%\%s\simple.shlexec"; + expected = 2; sprintf(fileA, testfile, shorttmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + todo_wine { TEST_LPFILE_PARSED_OK(); } + + /* quoted */ + testfile = ""%%TEMP%%\%s\simple.shlexec""; + expected = 2; sprintf(fileA, testfile, shorttmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + todo_wine { TEST_LPFILE_PARSED_OK(); } + + /* test SEE_MASK_DOENVSUBST works */ + testfile = "%%TEMP%%\%s\simple.shlexec"; + expected = 33; sprintf(fileA, testfile, shorttmpdir); + rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI, NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); + + /* quoted lpFile not works on real win95 and nt4 */ + if (isreally98_2k_plus) { + /* quoted */ + testfile = ""%%TEMP%%\%s\simple.shlexec""; + expected = 33; sprintf(fileA, testfile, shorttmpdir); + rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI, NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); + } else { + win_skip("ShellExecute('"command"', ...) only works on 98/2k or newer\n"); + } +} + static void test_filename(void) { char filename[MAX_PATH]; @@ -1938,6 +2035,7 @@ START_TEST(shlexec)
init_test();
+ test_lpFile_parsed(); test_filename(); test_find_executable(); test_lnks();
-
Ilya Basin -
Paul Vriens