Bug 1313306 - Don't expose os.path.{exists,isdir,isfile} to python configure without an @imports. r=chmanchester draft
authorMike Hommey <mh+mozilla@glandium.org>
Wed, 26 Oct 2016 11:49:58 +0900
changeset 431897 b601edf4c51bfb7fcbb360bcec4635572b5b120b
parent 431349 5ec994cb1be05ee46e2e58d476b2e62c1179b7dc
child 431898 0872ec7059c2bc417d44ac8295780a935c96d7d3
push id34144
push userbmo:mh+mozilla@glandium.org
push dateMon, 31 Oct 2016 22:40:13 +0000
reviewerschmanchester
bugs1313306
milestone52.0a1
Bug 1313306 - Don't expose os.path.{exists,isdir,isfile} to python configure without an @imports. r=chmanchester We want functions without an @imports to not have any side effects, and to not use external resources. So remove the few functions we expose from os.path without @imports('os') that do.
b2g/common.configure
build/moz.configure/android-ndk.configure
build/moz.configure/init.configure
build/moz.configure/old.configure
build/moz.configure/windows.configure
mobile/android/gradle.configure
python/mozbuild/mozbuild/configure/__init__.py
python/mozbuild/mozbuild/test/configure/common.py
python/mozbuild/mozbuild/test/configure/data/moz.configure
python/mozbuild/mozbuild/test/configure/test_configure.py
toolkit/moz.configure
--- a/b2g/common.configure
+++ b/b2g/common.configure
@@ -4,16 +4,17 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 # Truetype fonts for B2G
 # ==============================================================
 option(env='MOZTTDIR', nargs=1, help='Path to truetype fonts for B2G')
 
 @depends('MOZTTDIR')
+@imports('os')
 def mozttdir(value):
     if value:
         path = value[0]
         if not os.path.isdir(path):
             die('MOZTTDIR "%s" is not a valid directory', path)
         return path
 
 set_config('MOZTTDIR', mozttdir)
--- a/build/moz.configure/android-ndk.configure
+++ b/build/moz.configure/android-ndk.configure
@@ -49,16 +49,17 @@ def ndk(value, build_project):
     if value:
         return value[0]
 
 set_config('ANDROID_NDK', ndk)
 add_old_configure_assignment('android_ndk', ndk)
 
 @depends(target, android_version, ndk)
 @checking('for android platform directory')
+@imports('os')
 def android_platform(target, android_version, ndk):
     if target.os != 'Android':
         return
 
     if 'mips' in target.cpu:
         target_dir_name = 'mips'
     elif 'aarch64' == target.cpu:
         target_dir_name = 'arm64'
@@ -92,16 +93,17 @@ def extra_toolchain_flags(platform_dir):
     if not platform_dir:
         return []
     return ['-idirafter',
             os.path.join(platform_dir, 'usr', 'include')]
 
 @depends(target, host, ndk, '--with-android-toolchain',
          '--with-android-gnu-compiler-version')
 @checking('for the Android toolchain directory', lambda x: x or 'not found')
+@imports('os')
 @imports(_from='mozbuild.shellutil', _import='quote')
 def android_toolchain(target, host, ndk, toolchain, gnu_compiler_version):
     if not ndk:
         return
     if toolchain:
         return toolchain[0]
     else:
         if target.cpu == 'arm' and target.endianness == 'little':
--- a/build/moz.configure/init.configure
+++ b/build/moz.configure/init.configure
@@ -7,16 +7,17 @@
 include('util.configure')
 include('checks.configure')
 
 option(env='DIST', nargs=1, help='DIST directory')
 
 # Do not allow objdir == srcdir builds.
 # ==============================================================
 @depends('--help', 'DIST')
+@imports('os')
 def check_build_environment(help, dist):
     topobjdir = os.path.realpath(os.path.abspath('.'))
     topsrcdir = os.path.realpath(os.path.abspath(
         os.path.join(os.path.dirname(__file__), '..', '..')))
 
     if dist:
         dist = normsep(dist[0])
     else:
@@ -628,16 +629,17 @@ def default_project(build_env, help):
 option('--enable-project', nargs=1, default=default_project,
        help='Project to build')
 
 option('--with-external-source-dir', env='EXTERNAL_SOURCE_DIR', nargs=1,
        help='External directory containing additional build files')
 
 @depends('--enable-project', '--with-external-source-dir',
          check_build_environment, '--help')
+@imports('os')
 def include_project_configure(project, external_source_dir, build_env, help):
     if not project:
         die('--enable-project is required.')
 
     base_dir = build_env.topsrcdir
     if external_source_dir:
         base_dir = os.path.join(base_dir, external_source_dir[0])
 
--- a/build/moz.configure/old.configure
+++ b/build/moz.configure/old.configure
@@ -10,16 +10,17 @@ def encoded_open(path, mode):
     encoding = 'mbcs' if sys.platform == 'win32' else 'utf-8'
     return codecs.open(path, mode, encoding)
 
 
 option(env='AUTOCONF', nargs=1, help='Path to autoconf 2.13')
 
 @depends(mozconfig, 'AUTOCONF')
 @checking('for autoconf')
+@imports('os')
 @imports('re')
 def autoconf(mozconfig, autoconf):
     mozconfig_autoconf = None
     if mozconfig['path']:
         make_extra = mozconfig['make_extra']
         if make_extra:
             for assignment in make_extra:
                 m = re.match('(?:export\s+)?AUTOCONF\s*:?=\s*(.+)$',
@@ -59,30 +60,31 @@ set_config('AUTOCONF', autoconf)
 
 
 @depends('OLD_CONFIGURE', mozconfig, autoconf, check_build_environment, shell,
          old_configure_assignments, build_project)
 @imports(_from='__builtin__', _import='open')
 @imports(_from='__builtin__', _import='print')
 @imports('glob')
 @imports('itertools')
+@imports('os')
 @imports('subprocess')
 # Import getmtime without overwriting the sandbox os.path.
 @imports(_from='os.path', _import='getmtime')
 @imports(_from='mozbuild.shellutil', _import='quote')
 def prepare_configure(old_configure, mozconfig, autoconf, build_env, shell,
                       old_configure_assignments, build_project):
     # os.path.abspath in the sandbox will ensure forward slashes on Windows,
     # which is actually necessary because this path actually ends up literally
     # as $0, and backslashes there breaks autoconf's detection of the source
     # directory.
     old_configure = os.path.abspath(old_configure[0])
     if build_project == 'js':
         old_configure_dir = os.path.dirname(old_configure)
-        if not old_configure_dir.endswith('/js/src'):
+        if not old_configure_dir.replace(os.sep, '/').endswith('/js/src'):
             old_configure = os.path.join(old_configure_dir, 'js', 'src',
                                          os.path.basename(old_configure))
 
     refresh = True
     if os.path.exists(old_configure):
         mtime = getmtime(old_configure)
         aclocal = os.path.join(build_env.topsrcdir, 'build', 'autoconf',
                                '*.m4')
--- a/build/moz.configure/windows.configure
+++ b/build/moz.configure/windows.configure
@@ -260,16 +260,17 @@ def vc_path(c_compiler):
         result = next
         if p.lower() == 'bin':
             break
     return result
 
 
 @depends_win(vc_path)
 @checking('for the Debug Interface Access SDK', lambda x: x or 'not found')
+@imports('os')
 def dia_sdk_dir(vc_path):
     if vc_path:
         path = os.path.join(os.path.dirname(vc_path), 'DIA SDK')
         if os.path.isdir(path):
             return path
 
 
 @depends_win(vc_path, valid_windows_sdk_dir, valid_ucrt_sdk_dir, dia_sdk_dir)
--- a/mobile/android/gradle.configure
+++ b/mobile/android/gradle.configure
@@ -16,16 +16,17 @@ option('--with-gradle', nargs='?',
 def with_gradle(value):
     if value:
         return True
 
 set_config('MOZ_BUILD_MOBILE_ANDROID_WITH_GRADLE', with_gradle)
 
 
 @depends('--with-gradle', check_build_environment)
+@imports('os')
 def gradle(value, build_env):
     gradle = value[0] if len(value) else \
         os.path.join(build_env.topsrcdir, 'gradlew')
 
     # TODO: verify that $GRADLE is executable.
     if not os.path.isfile(gradle):
         die('GRADLE must be executable: %s', gradle)
 
--- a/python/mozbuild/mozbuild/configure/__init__.py
+++ b/python/mozbuild/mozbuild/configure/__init__.py
@@ -193,19 +193,18 @@ class ConfigureSandbox(dict):
         for b in ('None', 'False', 'True', 'int', 'bool', 'any', 'all', 'len',
                   'list', 'tuple', 'set', 'dict', 'isinstance', 'getattr',
                   'hasattr', 'enumerate', 'range', 'zip')
     }, __import__=forbidden_import, str=unicode)
 
     # Expose a limited set of functions from os.path
     OS = ReadOnlyNamespace(path=ReadOnlyNamespace(**{
         k: getattr(mozpath, k, getattr(os.path, k))
-        for k in ('abspath', 'basename', 'dirname', 'exists', 'isabs', 'isdir',
-                  'isfile', 'join', 'normcase', 'normpath', 'realpath',
-                  'relpath')
+        for k in ('abspath', 'basename', 'dirname', 'isabs', 'join',
+                  'normcase', 'normpath', 'realpath', 'relpath')
     }))
 
     def __init__(self, config, environ=os.environ, argv=sys.argv,
                  stdout=sys.stdout, stderr=sys.stderr, logger=None):
         dict.__setitem__(self, '__builtins__', self.BUILTINS)
 
         self._paths = []
         self._all_paths = set()
--- a/python/mozbuild/mozbuild/test/configure/common.py
+++ b/python/mozbuild/mozbuild/test/configure/common.py
@@ -81,21 +81,23 @@ class ConfigureTestSandbox(ConfigureSand
         if 'CONFIG_SHELL' not in environ:
             environ['CONFIG_SHELL'] = mozpath.abspath('/bin/sh')
             self._subprocess_paths[environ['CONFIG_SHELL']] = self.shell
             paths.append(environ['CONFIG_SHELL'])
         self._environ = copy.copy(environ)
 
         vfs = ConfigureTestVFS(paths)
 
-        self.OS = ReadOnlyNamespace(path=ReadOnlyNamespace(**{
-            k: v if k not in ('exists', 'isfile')
-            else getattr(vfs, k)
-            for k, v in ConfigureSandbox.OS.path.__dict__.iteritems()
-        }))
+        os_path = {
+            k: getattr(vfs, k) for k in dir(vfs) if not k.startswith('_')
+        }
+
+        os_path.update(self.OS.path.__dict__)
+
+        self.imported_os = ReadOnlyNamespace(path=ReadOnlyNamespace(**os_path))
 
         super(ConfigureTestSandbox, self).__init__(config, environ, *args,
                                                    **kwargs)
 
     def _get_one_import(self, what):
         if what == 'which.which':
             return self.which
 
@@ -166,17 +168,17 @@ class ConfigureTestSandbox(ConfigureSand
     def GetShortPathNameW(self, path_in, path_out, length):
         path_out.value = path_in
         return length
 
     def which(self, command, path=None):
         for parent in (path or self._search_path):
             c = mozpath.abspath(mozpath.join(parent, command))
             for candidate in (c, ensure_exe_extension(c)):
-                if self.OS.path.exists(candidate):
+                if self.imported_os.path.exists(candidate):
                     return candidate
         raise WhichError()
 
     def Popen(self, args, stdin=None, stdout=None, stderr=None, **kargs):
         try:
             program = self.which(args[0])
         except WhichError:
             raise OSError(errno.ENOENT, 'File not found')
--- a/python/mozbuild/mozbuild/test/configure/data/moz.configure
+++ b/python/mozbuild/mozbuild/test/configure/data/moz.configure
@@ -147,19 +147,19 @@ include(include_path)
 # The order of the decorators matter: @imports needs to appear after other
 # decorators.
 option('--with-imports', nargs='?', help='Imports')
 
 # A limited set of functions from os.path are exposed by default.
 @depends('--with-imports')
 def with_imports(value):
     if len(value):
-        return os.path.isfile(value[0])
+        return hasattr(os.path, 'abspath')
 
-set_config('IS_FILE', with_imports)
+set_config('HAS_ABSPATH', with_imports)
 
 # It is still possible to import the full set from os.path.
 # It is also possible to cherry-pick builtins.
 @depends('--with-imports')
 @imports('os.path')
 def with_imports(value):
     if len(value):
         return hasattr(os.path, 'getatime')
--- a/python/mozbuild/mozbuild/test/configure/test_configure.py
+++ b/python/mozbuild/mozbuild/test/configure/test_configure.py
@@ -361,23 +361,18 @@ class TestConfigure(unittest.TestCase):
             foo()'''),
             sandbox
         )
 
         self.assertEquals(len(imports), 1)
 
     def test_os_path(self):
         config = self.get_config(['--with-imports=%s' % __file__])
-        self.assertIn('IS_FILE', config)
-        self.assertEquals(config['IS_FILE'], True)
-
-        config = self.get_config(['--with-imports=%s.no-exist' % __file__])
-        self.assertIn('IS_FILE', config)
-        self.assertEquals(config['IS_FILE'], False)
-
+        self.assertIn('HAS_ABSPATH', config)
+        self.assertEquals(config['HAS_ABSPATH'], True)
         self.assertIn('HAS_GETATIME', config)
         self.assertEquals(config['HAS_GETATIME'], True)
         self.assertIn('HAS_GETATIME2', config)
         self.assertEquals(config['HAS_GETATIME2'], False)
 
     def test_template_call(self):
         config = self.get_config(env={'CC': 'gcc'})
         self.assertIn('TEMPLATE_VALUE', config)
--- a/toolkit/moz.configure
+++ b/toolkit/moz.configure
@@ -87,16 +87,17 @@ include('../js/moz.configure')
 
 
 # L10N
 # ==============================================================
 option('--with-l10n-base', nargs=1, env='L10NBASEDIR',
        help='Path to l10n repositories')
 
 @depends('--with-l10n-base')
+@imports('os')
 def l10n_base(value):
     if value:
         path = value[0]
         if not os.path.isdir(path):
             die("Invalid value --with-l10n-base, %s doesn't exist", path)
         return os.path.realpath(os.path.abspath(path))
 
 set_config('L10NBASEDIR', l10n_base)