Bug 944265 - Do proper quoting of DEFINES, and do the same for ACDEFINES. r=gps
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 28 Nov 2013 17:08:22 +0900
changeset 157925 9af1bfc53b44144d4df673719623004d5646ab99
parent 157924 c5d32149691aa6dbf121c07693a6214b424dedac
child 157926 f4b143a9c6241644470aab2a35e8e8045d0b59ff
push id25726
push usercbook@mozilla.com
push dateThu, 28 Nov 2013 10:47:25 +0000
treeherdermozilla-central@cdca43b7657d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs944265
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 944265 - Do proper quoting of DEFINES, and do the same for ACDEFINES. r=gps
python/mozbuild/mozbuild/backend/configenvironment.py
python/mozbuild/mozbuild/frontend/data.py
python/mozbuild/mozbuild/test/backend/data/defines/moz.build
python/mozbuild/mozbuild/test/backend/test_configenvironment.py
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
python/mozbuild/mozbuild/util.py
--- a/python/mozbuild/mozbuild/backend/configenvironment.py
+++ b/python/mozbuild/mozbuild/backend/configenvironment.py
@@ -13,33 +13,26 @@ from os.path import relpath
 from types import StringTypes
 
 from mozbuild.preprocessor import Preprocessor
 
 from ..util import (
     ensureParentDir,
     FileAvoidWrite,
     ReadOnlyDict,
+    shell_quote,
 )
 
 
 if sys.version_info.major == 2:
     text_type = unicode
 else:
     text_type = str
 
 
-RE_SHELL_ESCAPE = re.compile('''([ \t`#$^&*(){}\\|;'"<>?\[\]])''')
-
-
-def shell_escape(s):
-    """Escape some characters with a backslash, and double dollar signs."""
-    return RE_SHELL_ESCAPE.sub(r'\\\1', str(s)).replace('$', '$$')
-
-
 class BuildConfig(object):
     """Represents the output of configure."""
 
     def __init__(self):
         self.topsrcdir = None
         self.topobjdir = None
         self.defines = {}
         self.non_global_defines = []
@@ -117,17 +110,17 @@ class ConfigEnvironment(object):
 
         self.defines = ReadOnlyDict(defines)
         self.substs = dict(substs)
         self.topsrcdir = topsrcdir
         self.topobjdir = topobjdir
         global_defines = [name for name, value in defines
             if not name in non_global_defines]
         self.substs['ACDEFINES'] = ' '.join(['-D%s=%s' % (name,
-            shell_escape(self.defines[name])) for name in global_defines])
+            shell_quote(self.defines[name]).replace('$', '$$')) for name in global_defines])
         def serialize(obj):
             if isinstance(obj, StringTypes):
                 return obj
             if isinstance(obj, Iterable):
                 return ' '.join(obj)
             raise Exception('Unhandled type %s', type(obj))
         self.substs['ALLSUBSTS'] = '\n'.join(sorted(['%s = %s' % (name,
             serialize(self.substs[name])) for name in self.substs if self.substs[name]]))
--- a/python/mozbuild/mozbuild/frontend/data.py
+++ b/python/mozbuild/mozbuild/frontend/data.py
@@ -15,17 +15,20 @@ contains the code for converting execute
 structures.
 """
 
 from __future__ import unicode_literals
 
 import os
 
 from collections import OrderedDict
-from mozbuild.util import StrictOrderingOnAppendList
+from mozbuild.util import (
+    shell_quote,
+    StrictOrderingOnAppendList,
+)
 from .sandbox_symbols import compute_final_target
 
 
 class TreeMetadata(object):
     """Base class for all data being captured."""
 
     def __init__(self):
         self._ack = False
@@ -177,20 +180,18 @@ class Defines(SandboxDerived):
     def __init__(self, sandbox, defines):
         SandboxDerived.__init__(self, sandbox)
         self.defines = defines
 
     def get_defines(self):
         for define, value in self.defines.iteritems():
             if value is True:
                 defstr = define
-            elif type(value) == int:
-                defstr = '%s=%s' % (define, value)
             else:
-                defstr = '%s=\'%s\'' % (define, value)
+                defstr = '%s=%s' % (define, shell_quote(value))
             yield('-D%s' % defstr)
 
 class Exports(SandboxDerived):
     """Sandbox container object for EXPORTS, which is a HierarchicalStringList.
 
     We need an object derived from SandboxDerived for use in the backend, so
     this object fills that role. It just has a reference to the underlying
     HierarchicalStringList, which is created when parsing EXPORTS.
--- a/python/mozbuild/mozbuild/test/backend/data/defines/moz.build
+++ b/python/mozbuild/mozbuild/test/backend/data/defines/moz.build
@@ -1,13 +1,13 @@
 # Any copyright is dedicated to the Public Domain.
 # http://creativecommons.org/publicdomain/zero/1.0/
 
 value = 'xyz'
 DEFINES = {
     'FOO': True,
 }
 
-DEFINES['BAZ'] = '"abcd"'
+DEFINES['BAZ'] = '"ab\'cd"'
 DEFINES.update({
     'BAR': 7,
     'VALUE': value
 })
--- a/python/mozbuild/mozbuild/test/backend/test_configenvironment.py
+++ b/python/mozbuild/mozbuild/test/backend/test_configenvironment.py
@@ -33,33 +33,33 @@ class ConfigEnvironment(ConfigStatus.Con
 
 class TestEnvironment(unittest.TestCase):
     def test_auto_substs(self):
         '''Test the automatically set values of ACDEFINES, ALLDEFINES,
         ALLSUBSTS and ALLEMPTYSUBSTS.
         '''
         env = ConfigEnvironment('.', '.',
                   defines = [ ('foo', 'bar'), ('baz', 'qux 42'),
-                              ('abc', 'def'), ('extra', 'foobar') ],
+                              ('abc', "d'e'f"), ('extra', 'foobar') ],
                   non_global_defines = ['extra', 'ignore'],
                   substs = [ ('FOO', 'bar'), ('FOOBAR', ''), ('ABC', 'def'),
                              ('bar', 'baz qux'), ('zzz', '"abc def"'),
                              ('qux', '') ])
         # non_global_defines should be filtered out in ACDEFINES and
         # ALLDEFINES.
         # Original order of the defines need to be respected in ACDEFINES
-        self.assertEqual(env.substs['ACDEFINES'], '''-Dfoo=bar -Dbaz=qux\ 42 -Dabc=def''')
+        self.assertEqual(env.substs['ACDEFINES'], """-Dfoo='bar' -Dbaz='qux 42' -Dabc='d'\\''e'\\''f'""")
         # ALLDEFINES, on the other hand, needs to be sorted
-        self.assertEqual(env.substs['ALLDEFINES'], '''#define abc def
+        self.assertEqual(env.substs['ALLDEFINES'], '''#define abc d'e'f
 #define baz qux 42
 #define foo bar''')
         # Likewise for ALLSUBSTS, which also mustn't contain ALLDEFINES
         # but contain ACDEFINES
         self.assertEqual(env.substs['ALLSUBSTS'], '''ABC = def
-ACDEFINES = -Dfoo=bar -Dbaz=qux\ 42 -Dabc=def
+ACDEFINES = -Dfoo='bar' -Dbaz='qux 42' -Dabc='d'\\''e'\\''f'
 FOO = bar
 bar = baz qux
 zzz = "abc def"''')
         # ALLEMPTYSUBSTS contains all substs with no value.
         self.assertEqual(env.substs['ALLEMPTYSUBSTS'], '''FOOBAR =
 qux =''')
 
     def test_config_file(self):
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -470,17 +470,17 @@ class TestRecursiveMakeBackend(BackendTe
         env = self._consume('defines', RecursiveMakeBackend)
 
         backend_path = os.path.join(env.topobjdir, 'backend.mk')
         lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
 
         var = 'DEFINES'
         defines = [val for val in lines if val.startswith(var)]
 
-        expected = ['DEFINES += -DFOO -DBAZ=\'"abcd"\' -DBAR=7 -DVALUE=\'xyz\'']
+        expected = ['DEFINES += -DFOO -DBAZ=\'"ab\'\\\'\'cd"\' -DBAR=7 -DVALUE=\'xyz\'']
         self.assertEqual(defines, expected)
 
     def test_local_includes(self):
         """Test that LOCAL_INCLUDES are written to backend.mk correctly."""
         env = self._consume('local_includes', RecursiveMakeBackend)
 
         backend_path = os.path.join(env.topobjdir, 'backend.mk')
         lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
--- a/python/mozbuild/mozbuild/util.py
+++ b/python/mozbuild/mozbuild/util.py
@@ -485,8 +485,23 @@ class PushbackIter(object):
 
     def next(self):
         if self.pushed_back:
             return self.pushed_back.pop()
         return self.it.next()
 
     def pushback(self, item):
         self.pushed_back.append(item)
+
+
+def shell_quote(s):
+    '''Given a string, returns a version enclosed with single quotes for use
+    in a shell command line.
+
+    As a special case, if given an int, returns a string containing the int,
+    not enclosed in quotes.
+    '''
+    if type(s) == int:
+        return '%d' % s
+    # Single quoted strings can contain any characters unescaped except the
+    # single quote itself, which can't even be escaped, so the string needs to
+    # be closed, an escaped single quote added, and reopened.
+    return "'%s'" % s.replace("'", "'\\''")