From 33915eba7c8293eab4962345fbbb1e5d193295ed Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 14 Jun 2013 18:33:00 -0400 Subject: Issue #17222: Raise FileExistsError when py_compile.compile would overwrite a symlink or non-regular file with a regular file. --- Doc/library/py_compile.rst | 11 ++++++++++- Doc/whatsnew/3.4.rst | 5 +++++ Lib/py_compile.py | 14 ++++++++++++++ Lib/test/test_py_compile.py | 20 ++++++++++++++++++++ Misc/NEWS | 3 +++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/Doc/library/py_compile.rst b/Doc/library/py_compile.rst index 0c8c99d..bae8450 100644 --- a/Doc/library/py_compile.rst +++ b/Doc/library/py_compile.rst @@ -41,6 +41,13 @@ byte-code cache files in the directory containing the source code. is raised. This function returns the path to byte-compiled file, i.e. whatever *cfile* value was used. + If the path that *cfile* becomes (either explicitly specified or computed) + is a symlink or non-regular file, :exc:`FileExistsError` will be raised. + This is to act as a warning that import will turn those paths into regular + files if it is allowed to write byte-compiled files to those paths. This is + a side-effect of import using file renaming to place the final byte-compiled + file into place to prevent concurrent file writing issues. + *optimize* controls the optimization level and is passed to the built-in :func:`compile` function. The default of ``-1`` selects the optimization level of the current interpreter. @@ -53,7 +60,9 @@ byte-code cache files in the directory containing the source code. .. versionchanged:: 3.4 Changed code to use :mod:`importlib` for the byte-code cache file writing. This means file creation/writing semantics now match what :mod:`importlib` - does, e.g. permissions, write-and-move semantics, etc. + does, e.g. permissions, write-and-move semantics, etc. Also added the + caveat that :exc:`FileExistsError` is raised if *cfile* is a symlink or + non-regular file. .. function:: main(args=None) diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst index 1054c68..90b10e9 100644 --- a/Doc/whatsnew/3.4.rst +++ b/Doc/whatsnew/3.4.rst @@ -272,3 +272,8 @@ that may require changes to your code. * :c:func:`PyErr_SetImportError` now sets :exc:`TypeError` when its **msg** argument is not set. Previously only ``NULL`` was returned. + +* :func:`py_compile.compile` now raises :exc:`FileExistsError` if the file path + it would write to is a symlink or a non-regular file. This is to act as a + warning that import will overwrite those files with a regular file regardless + of what type of file path they were originally. diff --git a/Lib/py_compile.py b/Lib/py_compile.py index 701e8ac..cee35a5 100644 --- a/Lib/py_compile.py +++ b/Lib/py_compile.py @@ -7,6 +7,7 @@ import imp import importlib._bootstrap import importlib.machinery import os +import os.path import sys import traceback @@ -96,12 +97,25 @@ def compile(file, cfile=None, dfile=None, doraise=False, optimize=-1): See compileall.py for a script/module that uses this module to byte-compile all installed files (or all files in selected directories). + + Do note that FileExistsError is raised if cfile ends up pointing at a + non-regular file or symlink. Because the compilation uses a file renaming, + the resulting file would be regular and thus not the same type of file as + it was previously. """ if cfile is None: if optimize >= 0: cfile = imp.cache_from_source(file, debug_override=not optimize) else: cfile = imp.cache_from_source(file) + if os.path.islink(cfile): + msg = ('{} is a symlink and will be changed into a regular file if ' + 'import writes a byte-compiled file to it') + raise FileExistsError(msg.format(file, cfile)) + elif os.path.exists(cfile) and not os.path.isfile(cfile): + msg = ('{} is a non-regular file and will be changed into a regular ' + 'one if import writes a byte-compiled file to it') + raise FileExistsError(msg.format(file, cfile)) loader = importlib.machinery.SourceFileLoader('', file) source_bytes = loader.get_data(file) try: diff --git a/Lib/test/test_py_compile.py b/Lib/test/test_py_compile.py index e179554..54dc596 100644 --- a/Lib/test/test_py_compile.py +++ b/Lib/test/test_py_compile.py @@ -36,6 +36,26 @@ class PyCompileTests(unittest.TestCase): self.assertTrue(os.path.exists(self.pyc_path)) self.assertFalse(os.path.exists(self.cache_path)) + def test_do_not_overwrite_symlinks(self): + # In the face of a cfile argument being a symlink, bail out. + # Issue #17222 + try: + os.symlink(self.pyc_path + '.actual', self.pyc_path) + except OSError: + self.skipTest('need to be able to create a symlink for a file') + else: + assert os.path.islink(self.pyc_path) + with self.assertRaises(FileExistsError): + py_compile.compile(self.source_path, self.pyc_path) + + @unittest.skipIf(not os.path.exists(os.devnull) or os.path.isfile(os.devnull), + 'requires os.devnull and for it to be a non-regular file') + def test_do_not_overwrite_nonregular_files(self): + # In the face of a cfile argument being a non-regular file, bail out. + # Issue #17222 + with self.assertRaises(FileExistsError): + py_compile.compile(self.source_path, os.devnull) + def test_cache_path(self): py_compile.compile(self.source_path) self.assertTrue(os.path.exists(self.cache_path)) diff --git a/Misc/NEWS b/Misc/NEWS index 1fcb9c9..fb1fedf 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -1005,6 +1005,9 @@ Library Python file. Patch by Ben Morgan. - Have py_compile use importlib as much as possible to avoid code duplication. + Code now raises FileExistsError if the file path to be used for the + byte-compiled file is a symlink or non-regular file as a warning that import + will not keep the file path type if it writes to that path. - Issue #180022: Have site.addpackage() consider already known paths even when none are explicitly passed in. Bug report and fix by Kirill. -- cgit v0.12