From 3b4499c5c7718a2aca8558b857dfe02cc4a80cd9 Mon Sep 17 00:00:00 2001 From: Brian Curtin Date: Tue, 28 Dec 2010 14:31:47 +0000 Subject: [PATCH] Fix #9333. The symlink function is always available now, raising OSError when the user doesn't hold the symbolic link privilege rather than hiding it. --- Lib/test/support.py | 22 +++++++++++++++++++++- Lib/test/test_glob.py | 7 +++---- Lib/test/test_httpservers.py | 2 +- Lib/test/test_os.py | 6 +++--- Lib/test/test_platform.py | 3 +-- Lib/test/test_posixpath.py | 2 +- Lib/test/test_shutil.py | 12 ++++-------- Lib/test/test_sysconfig.py | 5 ++--- Lib/test/test_tarfile.py | 6 ++---- Misc/NEWS | 4 ++++ Modules/posixmodule.c | 32 +++++++++++++------------------- 11 files changed, 55 insertions(+), 46 deletions(-) diff --git a/Lib/test/support.py b/Lib/test/support.py index 27efd37afc7..142a87ea4eb 100644 --- a/Lib/test/support.py +++ b/Lib/test/support.py @@ -43,7 +43,7 @@ __all__ = [ "run_unittest", "run_doctest", "threading_setup", "threading_cleanup", "reap_children", "cpython_only", "check_impl_detail", "get_attribute", "swap_item", "swap_attr", "requires_IEEE_754", - "TestHandler", "Matcher"] + "TestHandler", "Matcher", "can_symlink", "skip_unless_symlink"] class Error(Exception): @@ -1412,3 +1412,23 @@ class Matcher(object): else: result = dv.find(v) >= 0 return result + + +_can_symlink = None +def can_symlink(): + global _can_symlink + if _can_symlink is not None: + return _can_symlink + try: + os.symlink(TESTFN, TESTFN + "can_symlink") + can = True + except OSError: + can = False + _can_symlink = can + return can + +def skip_unless_symlink(test): + """Skip decorator for tests that require functional symlink""" + ok = can_symlink() + msg = "Requires functional symlink implementation" + return test if ok else unittest.skip(msg)(test) diff --git a/Lib/test/test_glob.py b/Lib/test/test_glob.py index f1e1c036243..1560a6bbf0b 100644 --- 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 +from test.support import run_unittest, TESTFN, skip_unless_symlink, can_symlink 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 hasattr(os, "symlink"): + if can_symlink(): os.symlink(self.norm('broken'), self.norm('sym1')) os.symlink(self.norm('broken'), self.norm('sym2')) @@ -98,8 +98,7 @@ class GlobTests(unittest.TestCase): # either of these results are reasonable self.assertIn(res[0], [self.tempdir, self.tempdir + os.sep]) - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @skip_unless_symlink 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 19d3d176387..e42038aadc3 100644 --- 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 hasattr(os, "symlink"): + if support.can_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 9cfd7b8ffb9..497d8096c1f 100644 --- 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 hasattr(os, "symlink"): + if support.can_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 hasattr(os, "symlink"): + if support.can_symlink(): # Walk, following symlinks. for root, dirs, files in os.walk(walk_path, followlinks=True): if root == link_path: @@ -1149,7 +1149,7 @@ class Win32KillTests(unittest.TestCase): @unittest.skipUnless(sys.platform == "win32", "Win32 specific tests") -@unittest.skipUnless(hasattr(os, "symlink"), "Requires symlink implementation") +@support.skip_unless_symlink 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 7264c576e57..7dd7eef2fed 100644 --- a/Lib/test/test_platform.py +++ b/Lib/test/test_platform.py @@ -10,8 +10,7 @@ class PlatformTest(unittest.TestCase): def test_architecture(self): res = platform.architecture() - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @support.skip_unless_symlink 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 40425956616..f045b0bf4a9 100644 --- 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 hasattr(os, "symlink"): + if support.can_symlink(): os.symlink(support.TESTFN + "1", support.TESTFN + "2") self.assertIs(posixpath.islink(support.TESTFN + "2"), True) os.remove(support.TESTFN + "1") diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index d9e96785b2c..30d9e07e35d 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -291,8 +291,7 @@ class TestShutil(unittest.TestCase): finally: shutil.rmtree(TESTFN, ignore_errors=True) - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @support.skip_unless_symlink def test_dont_copy_file_onto_symlink_to_itself(self): # bug 851123. os.mkdir(TESTFN) @@ -312,8 +311,7 @@ class TestShutil(unittest.TestCase): finally: shutil.rmtree(TESTFN, ignore_errors=True) - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @support.skip_unless_symlink def test_rmtree_on_symlink(self): # bug 1669. os.mkdir(TESTFN) @@ -338,8 +336,7 @@ class TestShutil(unittest.TestCase): finally: os.remove(TESTFN) - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @support.skip_unless_symlink def test_copytree_named_pipe(self): os.mkdir(TESTFN) try: @@ -375,8 +372,7 @@ class TestShutil(unittest.TestCase): shutil.copytree(src_dir, dst_dir, copy_function=_copy) self.assertEqual(len(copied), 2) - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @support.skip_unless_symlink 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 d532ddfe517..193b5f00c4d 100644 --- 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) + captured_stdout, skip_unless_symlink) import sysconfig from sysconfig import (get_paths, get_platform, get_config_vars, @@ -245,8 +245,7 @@ class TestSysConfig(unittest.TestCase): 'posix_home', 'posix_prefix', 'posix_user') self.assertEqual(get_scheme_names(), wanted) - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @skip_unless_symlink 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 9d844640c8c..ff02c693330 100644 --- a/Lib/test/test_tarfile.py +++ b/Lib/test/test_tarfile.py @@ -322,8 +322,7 @@ class MiscReadTest(CommonReadTest): @unittest.skipUnless(hasattr(os, "link"), "Missing hardlink implementation") - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @support.skip_unless_symlink def test_extract_hardlink(self): # Test hardlink extraction (e.g. bug #857297). tar = tarfile.open(tarname, errorlevel=1, encoding="iso8859-1") @@ -841,8 +840,7 @@ class WriteTest(WriteTestBase): os.remove(target) os.remove(link) - @unittest.skipUnless(hasattr(os, "symlink"), - "Missing symlink implementation") + @support.skip_unless_symlink 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 e16cdb83a8b..674ae64c203 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -18,6 +18,10 @@ Core and Builtins Library ------- +- Issue 9333: os.symlink now available regardless of user privileges. + The function now raises OSError on Windows >=6.0 when the user is unable + to create symbolic links. XP and 2003 still raise NotImplementedError. + - Issue #10783: struct.pack() doesn't encode implicitly unicode to UTF-8 anymore. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 6f13776cce0..0b7f3f09bc7 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -279,6 +279,7 @@ extern int lstat(const char *, struct stat *); #include /* for UNLEN */ #ifdef SE_CREATE_SYMBOLIC_LINK_NAME /* Available starting with Vista */ #define HAVE_SYMLINK +static int win32_can_symlink = 0; #endif #endif /* _MSC_VER */ @@ -5243,6 +5244,10 @@ win_symlink(PyObject *self, PyObject *args, PyObject *kwargs) if (!PyArg_ParseTupleAndKeywords(args, kwargs, "OO|i:symlink", kwlist, &src, &dest, &target_is_directory)) return NULL; + + if (win32_can_symlink == 0) + return PyErr_Format(PyExc_OSError, "symbolic link privilege not held"); + if (!convert_to_unicode(&src)) { return NULL; } if (!convert_to_unicode(&dest)) { Py_DECREF(src); @@ -7801,7 +7806,7 @@ static PyMethodDef posix_methods[] = { {"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, + {"symlink", (PyCFunction)win_symlink, METH_VARARGS | METH_KEYWORDS, win_symlink__doc__}, #endif /* defined(HAVE_SYMLINK) && defined(MS_WINDOWS) */ #ifdef HAVE_SYSTEM @@ -8118,7 +8123,7 @@ static int insertvalues(PyObject *module) #endif #if defined(HAVE_SYMLINK) && defined(MS_WINDOWS) -void +static int enable_symlink() { HANDLE tok; @@ -8127,10 +8132,10 @@ enable_symlink() int meth_idx = 0; if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &tok)) - return; + return 0; if (!LookupPrivilegeValue(NULL, SE_CREATE_SYMBOLIC_LINK_NAME, &luid)) - return; + return 0; tok_priv.PrivilegeCount = 1; tok_priv.Privileges[0].Luid = luid; @@ -8139,21 +8144,10 @@ enable_symlink() if (!AdjustTokenPrivileges(tok, FALSE, &tok_priv, sizeof(TOKEN_PRIVILEGES), (PTOKEN_PRIVILEGES) NULL, (PDWORD) NULL)) - return; + return 0; - 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; - } + /* ERROR_NOT_ALL_ASSIGNED returned when the privilege can't be assigned. */ + return GetLastError() == ERROR_NOT_ALL_ASSIGNED ? 0 : 1; } #endif /* defined(HAVE_SYMLINK) && defined(MS_WINDOWS) */ @@ -8419,7 +8413,7 @@ INITFUNC(void) PyObject *m, *v; #if defined(HAVE_SYMLINK) && defined(MS_WINDOWS) - enable_symlink(); + win32_can_symlink = enable_symlink(); #endif m = PyModule_Create(&posixmodule);