# Suggestion for fixing Bug #40928

- Thread: https://externals.io/message/30468
- Original archive: https://news-web.php.net/php.internals/30468
- Started by Tzachi Tager on 2007-07-05T22:29:31+00:00
- Messages: 5
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #30468 — Tzachi Tager — 2007-07-05T22:29:31+00:00

URL: https://externals.io/message/30468#30468

Hi,
I was looking at Bug #40928 - `escapeshellarg()` does not quote percent
(%) correctly for cmd.exe.
This bug seems to be because `escapeshellarg()` in  Windows replaces '%'
and '"' with spaces, while assuming there isn't a real escaping method
for command line in Windows. Therefore I'm guessing no one really use
 `escapeshellarg()` or `escapeshellcmd()` on Windows. And in order to change
this  I suggest to use the command line escaping that does exists
(although looking a bit ugly), as you can see for example here:
http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs
/en-us/ntcmds_shelloverview.mspx?mfr=true , quoting: "You can use most
characters as variable values, including white space. If you use the
special characters <, >, |, &, or ^, you must precede them with the
escape character (^) or quotation marks." - So all special characters
will be replaced with "^<char>".
So this is the diff file that I suggest to use- it for sure fix the
above bug and may improve windows `escapeshellcmd()`:

cvs diff -- exec.c (in directory C:\php-src\ext\standard\)
Index: exec.c
===================================================================
RCS file: /repository/php-src/ext/standard/exec.c,v
retrieving revision 1.113.2.3.2.1
diff -r1.113.2.3.2.1 exec.c
275d274
< 			case '\'':
276a276,277
> 			case '\'':
> #endif
281a283
> #ifndef PHP_WIN32
282a285,289
> #else
> 					cmd[y++] = '"';
> 					cmd[y++] = '^';
> 					cmd[y++] = str[x];
> #endif				
286,287d292
< #endif
< 			case '#': /* This is character-set independent
*/
289,290d293
< 			case ';':
< 			case '`':
292,294d294
< 			case '*':
< 			case '?':
< 			case '~':
299a300,313
> #ifdef PHP_WIN32
> 			case '%':
> 				cmd[y++] = '"';
> 				cmd[y++] = '^';
> 				cmd[y++] = str[x];
> 				cmd[y++] = '"';		
> 				break;
> #endif
> 			case '#': /* This is character-set independent
*/
> 			case ';':
> 			case '`': 
> 			case '*':
> 			case '?':
> 			case '~':	
305d318
< 			case '\\':
309,310c322
< 			/* since Windows does not allow us to escape
these chars, just remove them */
< 			case '%':
---
> 				/* since Windows does not allow us to
escape these chars, just remove them */
313a326
> 			case '\\':
347d359
< 		case '%':

Comments will be greatly appreciated.
All the best,
Tzachi Tager.

---

## #30483 — Frode E. Moe — 2007-07-06T06:42:19+00:00

URL: https://externals.io/message/30468#30483 · In reply to: #30468

> Hi,
> I was looking at Bug #40928 - `escapeshellarg()` does not quote percent
> (%) correctly for cmd.exe.
> This bug seems to be because `escapeshellarg()` in  Windows replaces '%'
> and '"' with spaces, while assuming there isn't a real escaping method
> for command line in Windows. Therefore I'm guessing no one really use
> `escapeshellarg()` or `escapeshellcmd()` on Windows. And in order to change
> this  I suggest to use the command line escaping that does exists
> (although looking a bit ugly), as you can see for example here:
> http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs
> /en-us/ntcmds_shelloverview.mspx?mfr=true , quoting: "You can use most
> characters as variable values, including white space. If you use the
> special characters <, >, |, &, or ^, you must precede them with the
> escape character (^) or quotation marks." - So all special characters
> will be replaced with "^<char>".
> So this is the diff file that I suggest to use- it for sure fix the
> above bug and may improve windows `escapeshellcmd()`:

Hi, I'm the guy who reported the bug originally.

When I read your post now, I just realized that maybe there should be a
different set of escaping functions for win32 ("escapewin32arg" or
"escapecmdarg"?), so that the behaviour of `escapeshellarg()` does not change 
across platforms. (What if you want to dynamically generate a downloadable 
unix shell script, for example.)

Your patch was a bit difficult to read (too little context and not in
unidiff format), so I'll leave the commenting for those more familiar 
with the C source.

Thanks for working on the problem, anyway!

---

## #30563 — Tzachi Tager — 2007-07-06T19:14:13+00:00

URL: https://externals.io/message/30468#30563 · In reply to: #30483

Sorry guys, 
I've added below the unified and more readable diff.
Frode, about your comment I generally agree, it would be much easier to implement it differently for Windows as a separate code. In this diff I tried my best to avoid hurting Unix functionality.

Still comments are welcome.

Tzachi Tager.


cvs diff -u -- exec.c (in directory C:\php-src\ext\standard\)
Index: exec.c
===================================================================
RCS file: /repository/php-src/ext/standard/exec.c,v
retrieving revision 1.113.2.3.2.1
diff -u -r1.113.2.3.2.1 exec.c
--- exec.c	1 Jan 2007 09:36:08 -0000	1.113.2.3.2.1
+++ exec.c	28 Jun 2007 14:15:42 -0000
@@ -272,45 +272,58 @@
 	for (x = 0, y = 0; x < l; x++) {
 		switch (str[x]) {
 			case '"':
-			case '\'':
 #ifndef PHP_WIN32
+			case '\'':
+#endif
 				if (!p && (p = memchr(str + x + 1, str[x], l - x - 1))) {
 					/* noop */
 				} else if (p && *p == str[x]) {
 					p = NULL;
 				} else {
+#ifndef PHP_WIN32
 					cmd[y++] = '\\';
+#else
+					cmd[y++] = '"';
+					cmd[y++] = '^';
+					cmd[y++] = str[x];
+#endif				
 				}
 				cmd[y++] = str[x];
 				break;
-#endif
-			case '#': /* This is character-set independent */
 			case '&':
-			case ';':
-			case '`':
 			case '|':
-			case '*':
-			case '?':
-			case '~':
 			case '<':
 			case '>':
 			case '^':
 			case '(':
 			case ')':
+#ifdef PHP_WIN32
+			case '%':
+				cmd[y++] = '"';
+				cmd[y++] = '^';
+				cmd[y++] = str[x];
+				cmd[y++] = '"';		
+				break;
+#endif
+			case '#': /* This is character-set independent */
+			case ';':
+			case '`': 
+			case '*':
+			case '?':
+			case '~':	
 			case '[':
 			case ']':
 			case '{':
 			case '}':
 			case '$':
-			case '\\':
 			case '\x0A': /* excluding these two */
 			case '\xFF':
 #ifdef PHP_WIN32
-			/* since Windows does not allow us to escape these chars, just remove them */
-			case '%':
+				/* since Windows does not allow us to escape these chars, just remove them */
 				cmd[y++] = ' ';
 				break;
 #endif
+			case '\\':
 				cmd[y++] = '\\';
 				/* fall-through */
 			default:
@@ -344,7 +357,6 @@
 		switch (str[x]) {
 #ifdef PHP_WIN32
 		case '"':
-		case '%':
 			cmd[y++] = ' ';
 			break;
 #else


-----Original Message-----
From: Frode E. Moe [mailto:frode@CoreTrek.no] 
Sent: ו 06 יולי 2007 09:42
To: Tzachi Tager
Cc: internals@lists.php.net
Subject: Re: [PHP-DEV] Suggestion for fixing Bug #40928


> Hi,
> I was looking at Bug #40928 - `escapeshellarg()` does not quote percent
> (%) correctly for cmd.exe.
> This bug seems to be because `escapeshellarg()` in  Windows replaces '%'
> and '"' with spaces, while assuming there isn't a real escaping method
> for command line in Windows. Therefore I'm guessing no one really use
> `escapeshellarg()` or `escapeshellcmd()` on Windows. And in order to change
> this  I suggest to use the command line escaping that does exists
> (although looking a bit ugly), as you can see for example here:
> http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs
> /en-us/ntcmds_shelloverview.mspx?mfr=true , quoting: "You can use most
> characters as variable values, including white space. If you use the
> special characters <, >, |, &, or ^, you must precede them with the
> escape character (^) or quotation marks." - So all special characters
> will be replaced with "^<char>".
> So this is the diff file that I suggest to use- it for sure fix the
> above bug and may improve windows `escapeshellcmd()`:

Hi, I'm the guy who reported the bug originally.

When I read your post now, I just realized that maybe there should be a
different set of escaping functions for win32 ("escapewin32arg" or
"escapecmdarg"?), so that the behaviour of `escapeshellarg()` does not change 
across platforms. (What if you want to dynamically generate a downloadable 
unix shell script, for example.)

Your patch was a bit difficult to read (too little context and not in
unidiff format), so I'll leave the commenting for those more familiar 
with the C source.

Thanks for working on the problem, anyway!

---

## #30749 — Stanislav Malyshev — 2007-07-10T20:51:04+00:00

URL: https://externals.io/message/30468#30749 · In reply to: #30468

> /en-us/ntcmds_shelloverview.mspx?mfr=true , quoting: "You can use most
> characters as variable values, including white space. If you use the
> special characters <, >, |, &, or ^, you must precede them with the
> escape character (^) or quotation marks." - So all special characters
> will be replaced with "^<char>".

I think you are not entirely correct here. What the quote means is that 
if you want foo&bar, you should do either "foo&bar" or foo^&bar - not 
replace & with "^&".
-- 
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com

---

## #30775 — Tzachi Tager — 2007-07-11T08:55:51+00:00

URL: https://externals.io/message/30468#30775 · In reply to: #30749

The reason I'm doing this is because it seems that Windows parser looks for an odd number of '"' before the escaping, otherwise the escaping character is included in the outcome, for example:

string(25) "md "test ^& echo ^%foo^%""  ==> will output a directory called "test ^& echo ^%foo^%"

While:
string(31) "md "test "^&" echo "^%"foo"^%""" ==> will output a directory called "test & echo %foo%"

(I changed the php code and ran again, these are not the runs of the cmd.exe which does the escaping internally).
Which is what I wanted in the first place (and I know it looks ugly...).

Tzachi.

-----Original Message-----
From: Stanislav Malyshev [mailto:stas@zend.com] 
Sent: ג 10 יולי 2007 23:51
To: Tzachi Tager
Cc: internals@lists.php.net
Subject: Re: [PHP-DEV] Suggestion for fixing Bug #40928

> /en-us/ntcmds_shelloverview.mspx?mfr=true , quoting: "You can use most
> characters as variable values, including white space. If you use the
> special characters <, >, |, &, or ^, you must precede them with the
> escape character (^) or quotation marks." - So all special characters
> will be replaced with "^<char>".

I think you are not entirely correct here. What the quote means is that 
if you want foo&bar, you should do either "foo&bar" or foo^&bar - not 
replace & with "^&".
-- 
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com