Fix #9333. Expose os.symlink on Windows only when usable.

In order to create symlinks on Windows, SeCreateSymbolicLinkPrivilege
is an account privilege that is required to be held by the user. Not only
must the privilege be enabled for the account, the activated privileges for
the currently running application must be adjusted to enable the requested
privilege.

Rather than exposing an additional function to be called prior to the user's
first os.symlink call, we handle the AdjustTokenPrivileges Windows API call
internally and only expose os.symlink when the privilege escalation was
successful.

Due to the change of only exposing os.symlink when it's available, we can
go back to the original test skipping methods of checking via `hasattr`.

diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index 46a6f6e906a92e3e57508def4117185b12c61e39..d8b87fd99086b1d089b70c6e123c28652c913d97 100644 (file)
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -1392,7 +1392,15 @@ Files and Directories
 
    Symbolic link support was introduced in Windows 6.0 (Vista).  :func:`symlink`
    will raise a :exc:`NotImplementedError` on Windows versions earlier than 6.0.
-   The *SeCreateSymbolicLinkPrivilege* is required in order to create symlinks.
+
+   .. note::
+
+      The *SeCreateSymbolicLinkPrivilege* is required in order to create
+      symlinks, so the function is only available when the privilege is held.
+      This privilege is not typically granted to regular users but is available
+      to accounts which can escalate privileges to the administrator level.
+      Either obtaining the privilege or running your application as an
+      administrator are ways to successfully create symlinks.
 
    Availability: Unix, Windows.
 

diff --git a/Lib/test/support.py b/Lib/test/support.py
index fb43f3dbc5ed658a7c3fb837955121e6d49c0f53..535e2bec5b426f596d1df491170543a72288a0f0 100644 (file)
--- a/Lib/test/support.py
+++ b/Lib/test/support.py
@@ -42,7 +42,7 @@ __all__ = [
     "set_memlimit", "bigmemtest", "bigaddrspacetest", "BasicTestRunner",
     "run_unittest", "run_doctest", "threading_setup", "threading_cleanup",
     "reap_children", "cpython_only", "check_impl_detail", "get_attribute",
-    "swap_item", "swap_attr", "can_symlink", "skip_unless_symlink"]
+    "swap_item", "swap_attr"]
 
 
 class Error(Exception):
@@ -1256,27 +1256,6 @@ def reap_children():
             except:
                 break
 
-try:
-    from .symlink_support import enable_symlink_privilege
-except:
-    enable_symlink_privilege = lambda: True
-
-def can_symlink():
-    """It's no longer sufficient to test for the presence of symlink in the
-    os module - on Windows XP and earlier, os.symlink exists but a
-    NotImplementedError is thrown.
-    """
-    has_symlink = hasattr(os, 'symlink')
-    is_old_windows = sys.platform == "win32" and sys.getwindowsversion().major < 6
-    has_privilege = False if is_old_windows else enable_symlink_privilege()
-    return has_symlink and (not is_old_windows) and has_privilege
-
-def skip_unless_symlink(test):
-    """Skip decorator for tests that require functional symlink"""
-    selector = can_symlink()
-    msg = "Requires functional symlink implementation"
-    return [unittest.skip(msg)(test), test][selector]
-
 @contextlib.contextmanager
 def swap_attr(obj, attr, new_val):
     """Temporary swap out an attribute with a new object.

diff --git a/Lib/test/test_glob.py b/Lib/test/test_glob.py
index 1560a6bbf0b1b45895f006caf428a5ceb9a441fa..f1e1c03624387e636f548dfc82d33943a7bc209d 100644 (file)
--- a/Lib/test/test_glob.py
+++ b/Lib/test/test_glob.py
@@ -1,5 +1,5 @@
 import unittest
-from test.support import run_unittest, TESTFN, skip_unless_symlink, can_symlink
+from test.support import run_unittest, TESTFN
 import glob
 import os
 import shutil
@@ -25,7 +25,7 @@ class GlobTests(unittest.TestCase):
         self.mktemp('ZZZ')
         self.mktemp('a', 'bcd', 'EF')
         self.mktemp('a', 'bcd', 'efg', 'ha')
-        if can_symlink():
+        if hasattr(os, "symlink"):
             os.symlink(self.norm('broken'), self.norm('sym1'))
             os.symlink(self.norm('broken'), self.norm('sym2'))
 
@@ -98,7 +98,8 @@ class GlobTests(unittest.TestCase):
         # either of these results are reasonable
         self.assertIn(res[0], [self.tempdir, self.tempdir + os.sep])
 
-    @skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_glob_broken_symlinks(self):
         eq = self.assertSequencesEqual_noorder
         eq(self.glob('sym*'), [self.norm('sym1'), self.norm('sym2')])

diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py
index 86ce91287bc5459780bbdc3b2ecebc245dc55755..b03637cd722665bdae6c5a0af427da420eb0f801 100644 (file)
--- a/Lib/test/test_httpservers.py
+++ b/Lib/test/test_httpservers.py
@@ -304,7 +304,7 @@ class CGIHTTPServerTestCase(BaseTestCase):
 
         # The shebang line should be pure ASCII: use symlink if possible.
         # See issue #7668.
-        if support.can_symlink():
+        if hasattr(os, "symlink"):
             self.pythonexe = os.path.join(self.parent_dir, 'python')
             os.symlink(sys.executable, self.pythonexe)
         else:

diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index e27dd7a63d4ca185de492ecdbb05c922f2cb2940..fc6084b05e3df7e3b4f77214b9b82fdf88c7c2e9 100644 (file)
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -541,7 +541,7 @@ class WalkTests(unittest.TestCase):
             f = open(path, "w")
             f.write("I'm " + path + " and proud of it.  Blame test_os.\n")
             f.close()
-        if support.can_symlink():
+        if hasattr(os, "symlink"):
             os.symlink(os.path.abspath(t2_path), link_path)
             sub2_tree = (sub2_path, ["link"], ["tmp3"])
         else:
@@ -585,7 +585,7 @@ class WalkTests(unittest.TestCase):
         self.assertEqual(all[flipped + 1], (sub1_path, ["SUB11"], ["tmp2"]))
         self.assertEqual(all[2 - 2 * flipped], sub2_tree)
 
-        if support.can_symlink():
+        if hasattr(os, "symlink"):
             # Walk, following symlinks.
             for root, dirs, files in os.walk(walk_path, followlinks=True):
                 if root == link_path:
@@ -1146,14 +1146,8 @@ class Win32KillTests(unittest.TestCase):
         self._kill_with_event(signal.CTRL_BREAK_EVENT, "CTRL_BREAK_EVENT")
 
 
-def skipUnlessWindows6(test):
-    if (hasattr(sys, 'getwindowsversion')
-        and sys.getwindowsversion().major >= 6):
-        return test
-    return unittest.skip("Requires Windows Vista or later")(test)
-
 @unittest.skipUnless(sys.platform == "win32", "Win32 specific tests")
-@support.skip_unless_symlink
+@unittest.skipUnless(hasattr(os, "symlink"), "Requires symlink implementation")
 class Win32SymlinkTests(unittest.TestCase):
     filelink = 'filelinktest'
     filelink_target = os.path.abspath(__file__)

diff --git a/Lib/test/test_platform.py b/Lib/test/test_platform.py
index 7dd7eef2fedb1d4bb96cf99a0f34c394a996d3ce..7264c576e572992d2728f1dd49834d44e2bdb493 100644 (file)
--- a/Lib/test/test_platform.py
+++ b/Lib/test/test_platform.py
@@ -10,7 +10,8 @@ class PlatformTest(unittest.TestCase):
     def test_architecture(self):
         res = platform.architecture()
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_architecture_via_symlink(self): # issue3762
         # On Windows, the EXE needs to know where pythonXY.dll is at so we have
         # to add the directory to the path.

diff --git a/Lib/test/test_posixpath.py b/Lib/test/test_posixpath.py
index 8cac0df779149e06827c5673b38be04fe5783ae9..40425956616c29d05263089de6d52901e3b37406 100644 (file)
--- a/Lib/test/test_posixpath.py
+++ b/Lib/test/test_posixpath.py
@@ -155,7 +155,7 @@ class PosixPathTest(unittest.TestCase):
             f.write(b"foo")
             f.close()
             self.assertIs(posixpath.islink(support.TESTFN + "1"), False)
-            if support.can_symlink():
+            if hasattr(os, "symlink"):
                 os.symlink(support.TESTFN + "1", support.TESTFN + "2")
                 self.assertIs(posixpath.islink(support.TESTFN + "2"), True)
                 os.remove(support.TESTFN + "1")
@@ -180,7 +180,8 @@ class PosixPathTest(unittest.TestCase):
     @unittest.skipIf(
         sys.platform.startswith('win'),
         "posixpath.samefile does not work on links in Windows")
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_samefile_on_links(self):
         test_fn1 = support.TESTFN + "1"
         test_fn2 = support.TESTFN + "2"
@@ -204,7 +205,8 @@ class PosixPathTest(unittest.TestCase):
     @unittest.skipIf(
         sys.platform.startswith('win'),
         "posixpath.samestat does not work on links in Windows")
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_samestat_on_links(self):
         test_fn1 = support.TESTFN + "1"
         test_fn2 = support.TESTFN + "2"
@@ -273,7 +275,8 @@ class PosixPathTest(unittest.TestCase):
         self.assertEqual(posixpath.normpath(b"///..//./foo/.//bar"),
                          b"/foo/bar")
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     @skip_if_ABSTFN_contains_backslash
     def test_realpath_basic(self):
         # Basic operation.
@@ -283,7 +286,8 @@ class PosixPathTest(unittest.TestCase):
         finally:
             support.unlink(ABSTFN)
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     @skip_if_ABSTFN_contains_backslash
     def test_realpath_symlink_loops(self):
         # Bug #930024, return the path unchanged if we get into an infinite
@@ -307,7 +311,8 @@ class PosixPathTest(unittest.TestCase):
             support.unlink(ABSTFN+"1")
             support.unlink(ABSTFN+"2")
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     @skip_if_ABSTFN_contains_backslash
     def test_realpath_resolve_parents(self):
         # We also need to resolve any symlinks in the parents of a relative
@@ -328,7 +333,8 @@ class PosixPathTest(unittest.TestCase):
             safe_rmdir(ABSTFN + "/y")
             safe_rmdir(ABSTFN)
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     @skip_if_ABSTFN_contains_backslash
     def test_realpath_resolve_before_normalizing(self):
         # Bug #990669: Symbolic links should be resolved before we
@@ -358,7 +364,8 @@ class PosixPathTest(unittest.TestCase):
             safe_rmdir(ABSTFN + "/k")
             safe_rmdir(ABSTFN)
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     @skip_if_ABSTFN_contains_backslash
     def test_realpath_resolve_first(self):
         # Bug #1213894: The first component of the path, if not absolute,

diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index d80acfaf8f22e029a188f379f8ef27cfe0438cb2..a5497e5186f4ca7d0c53ccaef3c3ddf932ce9739 100644 (file)
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -271,7 +271,8 @@ class TestShutil(unittest.TestCase):
             shutil.rmtree(src_dir)
             shutil.rmtree(os.path.dirname(dst_dir))
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_dont_copy_file_onto_link_to_itself(self):
         # bug 851123.
         os.mkdir(TESTFN)
@@ -303,7 +304,8 @@ class TestShutil(unittest.TestCase):
             except OSError:
                 pass
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_rmtree_on_symlink(self):
         # bug 1669.
         os.mkdir(TESTFN)
@@ -328,26 +330,27 @@ class TestShutil(unittest.TestCase):
             finally:
                 os.remove(TESTFN)
 
-    @unittest.skipUnless(hasattr(os, 'mkfifo'), 'requires os.mkfifo')
-    def test_copytree_named_pipe(self):
-        os.mkdir(TESTFN)
-        try:
-            subdir = os.path.join(TESTFN, "subdir")
-            os.mkdir(subdir)
-            pipe = os.path.join(subdir, "mypipe")
-            os.mkfifo(pipe)
+        @unittest.skipUnless(hasattr(os, "symlink"),
+                             "Missing symlink implementation")
+        def test_copytree_named_pipe(self):
+            os.mkdir(TESTFN)
             try:
-                shutil.copytree(TESTFN, TESTFN2)
-            except shutil.Error as e:
-                errors = e.args[0]
-                self.assertEqual(len(errors), 1)
-                src, dst, error_msg = errors[0]
-                self.assertEqual("`%s` is a named pipe" % pipe, error_msg)
-            else:
-                self.fail("shutil.Error should have been raised")
-        finally:
-            shutil.rmtree(TESTFN, ignore_errors=True)
-            shutil.rmtree(TESTFN2, ignore_errors=True)
+                subdir = os.path.join(TESTFN, "subdir")
+                os.mkdir(subdir)
+                pipe = os.path.join(subdir, "mypipe")
+                os.mkfifo(pipe)
+                try:
+                    shutil.copytree(TESTFN, TESTFN2)
+                except shutil.Error as e:
+                    errors = e.args[0]
+                    self.assertEqual(len(errors), 1)
+                    src, dst, error_msg = errors[0]
+                    self.assertEqual("`%s` is a named pipe" % pipe, error_msg)
+                else:
+                    self.fail("shutil.Error should have been raised")
+            finally:
+                shutil.rmtree(TESTFN, ignore_errors=True)
+                shutil.rmtree(TESTFN2, ignore_errors=True)
 
     def test_copytree_special_func(self):
 
@@ -364,7 +367,8 @@ class TestShutil(unittest.TestCase):
         shutil.copytree(src_dir, dst_dir, copy_function=_copy)
         self.assertEqual(len(copied), 2)
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_copytree_dangling_symlinks(self):
 
         # a dangling symlink raises an error at the end

diff --git a/Lib/test/test_sysconfig.py b/Lib/test/test_sysconfig.py
index 193b5f00c4db091eeae72d4d6b5796116004a1b5..d532ddfe51774bc6c4ef8186f21f1cd3c7b6d6d1 100644 (file)
--- a/Lib/test/test_sysconfig.py
+++ b/Lib/test/test_sysconfig.py
@@ -12,7 +12,7 @@ import shutil
 from copy import copy, deepcopy
 
 from test.support import (run_unittest, TESTFN, unlink, get_attribute,
-                          captured_stdout, skip_unless_symlink)
+                          captured_stdout)
 
 import sysconfig
 from sysconfig import (get_paths, get_platform, get_config_vars,
@@ -245,7 +245,8 @@ class TestSysConfig(unittest.TestCase):
                   'posix_home', 'posix_prefix', 'posix_user')
         self.assertEqual(get_scheme_names(), wanted)
 
-    @skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_symlink(self):
         # On Windows, the EXE needs to know where pythonXY.dll is at so we have
         # to add the directory to the path.

diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py
index ff02c693330d2c0a1d0febb3b7630cac96299101..9d844640c8cc55219376ec7c507d6be2fa570bb8 100644 (file)
--- a/Lib/test/test_tarfile.py
+++ b/Lib/test/test_tarfile.py
@@ -322,7 +322,8 @@ class MiscReadTest(CommonReadTest):
 
     @unittest.skipUnless(hasattr(os, "link"),
                          "Missing hardlink implementation")
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_extract_hardlink(self):
         # Test hardlink extraction (e.g. bug #857297).
         tar = tarfile.open(tarname, errorlevel=1, encoding="iso8859-1")
@@ -840,7 +841,8 @@ class WriteTest(WriteTestBase):
                 os.remove(target)
                 os.remove(link)
 
-    @support.skip_unless_symlink
+    @unittest.skipUnless(hasattr(os, "symlink"),
+                         "Missing symlink implementation")
     def test_symlink_size(self):
         path = os.path.join(TEMPDIR, "symlink")
         os.symlink("link_target", path)

diff --git a/Misc/NEWS b/Misc/NEWS
index 27965f1bad2706b0cc0e88bf8f476f2accf0604f..aeb1f143cc85347f7493b0ae1d12542e5f2c6389 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@ What's New in Python 3.2 Beta 1?
 Core and Builtins
 -----------------
 
+- Issue #9333: Expose os.symlink only when the SeCreateSymbolicLinkPrivilege
+  is held by the user's account, i.e., when the function can actually be used.
+
 - Issue #7475: Added transform() and untransform() methods to both bytes
   and string types.  They can be used to access those codecs providing
   bytes-to-bytes and string-to-string mappings.

diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 7267eca1bac0078e103607b425e20981ec985075..7419862879dc2015423d48419ee93236ab86a40b 100644 (file)
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -277,6 +277,9 @@ extern int lstat(const char *, struct stat *);
 #include <windows.h>
 #include <shellapi.h>   /* for ShellExecute() */
 #include <lmcons.h>     /* for UNLEN */
+#ifdef SE_CREATE_SYMBOLIC_LINK_NAME /* Available starting with Vista */
+#define HAVE_SYMLINK
+#endif
 #endif /* _MSC_VER */
 
 #if defined(PYCC_VACPP) && defined(PYOS_OS2)
@@ -5091,7 +5094,7 @@ posix_readlink(PyObject *self, PyObject *args)
 #endif /* HAVE_READLINK */
 
 
-#ifdef HAVE_SYMLINK
+#if defined(HAVE_SYMLINK) && !defined(MS_WINDOWS)
 PyDoc_STRVAR(posix_symlink__doc__,
 "symlink(src, dst)\n\n\
 Create a symbolic link pointing to src named dst.");
@@ -5179,7 +5182,7 @@ win_readlink(PyObject *self, PyObject *args)
 
 #endif /* !defined(HAVE_READLINK) && defined(MS_WINDOWS) */
 
-#if !defined(HAVE_SYMLINK) && defined(MS_WINDOWS)
+#if defined(HAVE_SYMLINK) && defined(MS_WINDOWS)
 
 /* Grab CreateSymbolicLinkW dynamically from kernel32 */
 static int has_CreateSymbolicLinkW = 0;
@@ -5257,7 +5260,7 @@ win_symlink(PyObject *self, PyObject *args, PyObject *kwargs)
     Py_INCREF(Py_None);
     return Py_None;
 }
-#endif /* !defined(HAVE_SYMLINK) && defined(MS_WINDOWS) */
+#endif /* defined(HAVE_SYMLINK) && defined(MS_WINDOWS) */
 
 #ifdef HAVE_TIMES
 #if defined(PYCC_VACPP) && defined(PYOS_OS2)
@@ -7779,13 +7782,13 @@ static PyMethodDef posix_methods[] = {
     {"rmdir",           posix_rmdir, METH_VARARGS, posix_rmdir__doc__},
     {"stat",            posix_stat, METH_VARARGS, posix_stat__doc__},
     {"stat_float_times", stat_float_times, METH_VARARGS, stat_float_times__doc__},
-#ifdef HAVE_SYMLINK
+#if defined(HAVE_SYMLINK) && !defined(MS_WINDOWS)
     {"symlink",         posix_symlink, METH_VARARGS, posix_symlink__doc__},
 #endif /* HAVE_SYMLINK */
-#if !defined(HAVE_SYMLINK) && defined(MS_WINDOWS)
-    {"symlink", (PyCFunction)win_symlink, METH_VARARGS | METH_KEYWORDS,
-                win_symlink__doc__},
-#endif /* !defined(HAVE_SYMLINK) && defined(MS_WINDOWS) */
+#if defined(HAVE_SYMLINK) && defined(MS_WINDOWS)
+    {"_symlink", (PyCFunction)win_symlink, METH_VARARGS | METH_KEYWORDS,
+                 win_symlink__doc__},
+#endif /* defined(HAVE_SYMLINK) && defined(MS_WINDOWS) */
 #ifdef HAVE_SYSTEM
     {"system",          posix_system, METH_VARARGS, posix_system__doc__},
 #endif
@@ -8099,6 +8102,46 @@ static int insertvalues(PyObject *module)
 }
 #endif
 
+#if defined(HAVE_SYMLINK) && defined(MS_WINDOWS)
+void
+enable_symlink()
+{
+    HANDLE tok;
+    TOKEN_PRIVILEGES tok_priv;
+    LUID luid;
+    int meth_idx = 0;
+
+    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &tok))
+        return;
+
+    if (!LookupPrivilegeValue(NULL, SE_CREATE_SYMBOLIC_LINK_NAME, &luid))
+        return;
+
+    tok_priv.PrivilegeCount = 1;
+    tok_priv.Privileges[0].Luid = luid;
+    tok_priv.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
+
+    if (!AdjustTokenPrivileges(tok, FALSE, &tok_priv,
+                               sizeof(TOKEN_PRIVILEGES),
+                               (PTOKEN_PRIVILEGES) NULL, (PDWORD) NULL))
+        return;
+
+    if(GetLastError() == ERROR_NOT_ALL_ASSIGNED) {
+        /* We couldn't acquire the necessary privilege, so leave the
+           method hidden for this user. */
+        return;
+    } else {
+        /* We've successfully acquired the symlink privilege so rename
+           the method to it's proper "os.symlink" name. */
+        while(posix_methods[meth_idx].ml_meth != (PyCFunction)win_symlink)
+            meth_idx++;
+        posix_methods[meth_idx].ml_name = "symlink";
+
+        return;
+    }
+}
+#endif /* defined(HAVE_SYMLINK) && defined(MS_WINDOWS) */
+
 static int
 all_ins(PyObject *d)
 {
@@ -8360,6 +8403,10 @@ INITFUNC(void)
 {
     PyObject *m, *v;
 
+#if defined(HAVE_SYMLINK) && defined(MS_WINDOWS)
+    enable_symlink();
+#endif
+
     m = PyModule_Create(&posixmodule);
     if (m == NULL)
         return NULL;