[Python-checkins] cpython: Issue #17222: Raise FileExistsError when py_compile.compile would

[brett.cannon]

http://hg.python.org/cpython/rev/46ef1d2af352
changeset:   84125:46ef1d2af352
parent:      84123:01da7bf11ca1
user:        Brett Cannon <brett at python.org>
date:        Fri Jun 14 18:33:00 2013 -0400
summary:
  Issue #17222: Raise FileExistsError when py_compile.compile would
overwrite a symlink or non-regular file with a regular file.

files:
  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 deletions(-)


diff --git a/Doc/library/py_compile.rst b/Doc/library/py_compile.rst
--- a/Doc/library/py_compile.rst
+++ b/Doc/library/py_compile.rst
@@ -41,6 +41,13 @@
    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 @@
    .. 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
--- a/Doc/whatsnew/3.4.rst
+++ b/Doc/whatsnew/3.4.rst
@@ -272,3 +272,8 @@
 
 * :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
--- a/Lib/py_compile.py
+++ b/Lib/py_compile.py
@@ -7,6 +7,7 @@
 import importlib._bootstrap
 import importlib.machinery
 import os
+import os.path
 import sys
 import traceback
 
@@ -96,12 +97,25 @@
     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('<py_compile>', file)
     source_bytes = loader.get_data(file)
     try:
diff --git a/Lib/test/test_py_compile.py b/Lib/test/test_py_compile.py
--- a/Lib/test/test_py_compile.py
+++ b/Lib/test/test_py_compile.py
@@ -36,6 +36,26 @@
         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
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -1005,6 +1005,9 @@
   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.

-- 
Repository URL: http://hg.python.org/cpython