SCons / scons

SCons - a software construction tool
http://scons.org
MIT License
2.11k stars 319 forks source link

Fix escape function for POSIX systems in SCons. #2766

Open bdbaddog opened 6 years ago

bdbaddog commented 6 years ago

This issue was originally created at: 2011-05-25 02:58:29. This issue was reported by: sesarr. sesarr said at 2011-05-25 02:58:29

Default escape function for POSIX systems in SCons breaks on filenames containing special chars like ( ) $ because it tries to escape them inside quotation marks.


$ echo "\(\)" # Wrong
\(\)

$ echo '()' # OK ()


> 
> 
```diff
diff --git a/scons/scons-2.0.1/scons-local-2.0.1/SCons/Platform/posix.py
b/scons/scons-2.0.1/scons-local-2.0.1/SCons/Platform/posix.py
index 87de9df..aa6fedd 100644
--- a/scons/scons-2.0.1/scons-local-2.0.1/SCons/Platform/posix.py
+++ b/scons/scons-2.0.1/scons-local-2.0.1/SCons/Platform/posix.py
@@ -49,14 +49,7 @@ exitvalmap = {

 def escape(arg):
     "escape shell special characters"
-    slash = '\\'
-    special = '"$()'
-
-    arg = arg.replace(slash, slash+slash)
-    for c in special:
-        arg = arg.replace(c, slash+c)
-
-    return '"' + arg + '"'
+    return "'" + arg.replace("'", "'\''") + "'"

 def exec_system(l, env):
     stat = os.system(' '.join(l))
mwichmann commented 2 years ago

Looks like the code has changed here - the collections of special no longer includes parens, but I think the underlying comment is correct - since single and double quotes differ somewhat for POSIX shells, the escaped string should be forced to be single-quoted, not double-quoted. shlex.quote does this (after using a regex to not-bother if there are no characters that actually need escaping):

    return "'" + s.replace("'", "'\"'\"'") + "'"
bdbaddog commented 2 years ago

@mwichmann Should we switch to use shlex.quote?

mwichmann commented 2 years ago

I probably would - it's not "portable", but since it's only intended for the Platform/posix module, that's not a problem.

mwichmann commented 2 years ago

A quick change - from shlex import quote as escape - doesn't break any of the obvious tests. Update: it breaks one non-obvious one - test/GetBuildFailures/serial.py. In this case the test is expecting oddly quoted things, and now not getting them. I think the test is silly (i.e. it was coded to the implementation, not necessarily to what the result should be), but now I'm a bit worried someone else might expect this too...

mwichmann commented 2 years ago

Note that the problem complained about here (escaping of parens) was probably fixed by ed53a2e548, which referenced a different issue, #2225, so I think we can close this in any case, up to @bdbaddog whether we further switch to using the (presumably) more tested routine from shlex.