Skip to content

Commit

Permalink
Stricter validation for popen mode argument on Windows
Browse files Browse the repository at this point in the history
Context: The ext/standard/tests/file/popen_pclose_error-win32.phpt
test often fails under parallel testing, because the "is not recognized
as an internal or external command" message doesn't actually have a
guaranteed position in the output.

While looking into this, I noticed that this test on Windows tests
something very different (invalid comand) than on Linux (invalid mode).
Here I'm adjusting the Windows popen implementation so it immediately
fails on a `rw` mode, just like it does on Linux.
  • Loading branch information
nikic committed Feb 22, 2019
1 parent 427ebce commit 914c1ec
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 127 deletions.
14 changes: 7 additions & 7 deletions TSRM/tsrm_win32.c
Expand Up @@ -468,17 +468,17 @@ TSRM_API FILE *popen_ex(const char *command, const char *type, const char *cwd,
return NULL;
}

/*The following two checks can be removed once we drop XP support */
type_len = (int)strlen(type);
if (type_len <1 || type_len > 2) {
if (type_len < 1 || type_len > 2) {
return NULL;
}

for (i=0; i < type_len; i++) {
if (!(*ptype == 'r' || *ptype == 'w' || *ptype == 'b' || *ptype == 't')) {
return NULL;
}
ptype++;
if (ptype[0] != 'r' && ptype[0] != 'w') {
return NULL;
}

if (type_len > 1 && (ptype[1] != 'b' && ptype[1] != 't')) {
return NULL;
}

cmd = (char*)malloc(strlen(command)+strlen(TWG(comspec))+sizeof(" /c ")+2);
Expand Down
61 changes: 0 additions & 61 deletions ext/standard/tests/file/popen_pclose_error-win32-debug.phpt

This file was deleted.

57 changes: 0 additions & 57 deletions ext/standard/tests/file/popen_pclose_error-win32.phpt

This file was deleted.

4 changes: 2 additions & 2 deletions ext/standard/tests/file/popen_pclose_error.phpt
Expand Up @@ -2,8 +2,8 @@
Test popen() and pclose function: error conditions
--SKIPIF--
<?php
if(substr(PHP_OS, 0, 3) == 'WIN' || strtoupper( substr(PHP_OS, 0, 3) ) == 'SUN')
die("skip Not Valid for Windows & Sun Solaris");
if (strtoupper( substr(PHP_OS, 0, 3) ) == 'SUN')
die("skip Not Valid for Sun Solaris");
?>
--FILE--
<?php
Expand Down

0 comments on commit 914c1ec

Please sign in to comment.