Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:30563
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.162 as permitted sender)
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="windows-1255"
Content-Transfer-Encoding: quoted-printable
Date: Fri, 6 Jul 2007 22:14:13 +0300
Message-ID: <06B0D32C7A96544490D18AF653D6BDE501075E42@il-ex1.zend.net>
Thread-Topic: [PHP-DEV] Suggestion for fixing Bug #40928
Thread-Index: Ace/mTIU1SjqSMQwQV6tj5yurMnPeQAOYz4w
References: <06B0D32C7A96544490D18AF653D6BDE501075DCE@il-ex1.zend.net> <20070706064219.GA4515@fearless.intra.coretrek.com>
To: "Frode E. Moe" <frode@CoreTrek.no>
Cc: <internals@lists.php.net>
Subject: RE: [PHP-DEV] Suggestion for fixing Bug #40928
From: tzachi@zend.com ("Tzachi Tager")

Sorry guys,=20
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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
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 =3D 0, y =3D 0; x < l; x++) {
 		switch (str[x]) {
 			case '"':
-			case '\'':
 #ifndef PHP_WIN32
+			case '\'':
+#endif
 				if (!p && (p =3D memchr(str + x + 1, str[x], l - x - 1))) {
 					/* noop */
 				} else if (p && *p =3D=3D str[x]) {
 					p =3D NULL;
 				} else {
+#ifndef PHP_WIN32
 					cmd[y++] =3D '\\';
+#else
+					cmd[y++] =3D '"';
+					cmd[y++] =3D '^';
+					cmd[y++] =3D str[x];
+#endif			=09
 				}
 				cmd[y++] =3D 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++] =3D '"';
+				cmd[y++] =3D '^';
+				cmd[y++] =3D str[x];
+				cmd[y++] =3D '"';	=09
+				break;
+#endif
+			case '#': /* This is character-set independent */
+			case ';':
+			case '`':=20
+			case '*':
+			case '?':
+			case '~':=09
 			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++] =3D ' ';
 				break;
 #endif
+			case '\\':
 				cmd[y++] =3D '\\';
 				/* fall-through */
 			default:
@@ -344,7 +357,6 @@
 		switch (str[x]) {
 #ifdef PHP_WIN32
 		case '"':
-		case '%':
 			cmd[y++] =3D ' ';
 			break;
 #else


-----Original Message-----
From: Frode E. Moe [mailto:frode@CoreTrek.no]=20
Sent: =E5 06 =E9=E5=EC=E9 2007 09:42
To: Tzachi Tager
Cc: internals@lists.php.net
Subject: Re: [PHP-DEV] Suggestion for fixing Bug #40928

On Fri, Jul 06, 2007 at 01:29:31 +0300, Tzachi Tager wrote:
> 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=3Dtrue , 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=20
across platforms. (What if you want to dynamically generate a =
downloadable=20
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=20
with the C source.

Thanks for working on the problem, anyway!