diff -r 4a6b8f86b081 Lib/test/test_os.py --- a/Lib/test/test_os.py Mon Apr 15 23:14:55 2013 -0700 +++ b/Lib/test/test_os.py Mon Apr 22 12:09:27 2013 -0700 @@ -24,6 +24,8 @@ import stat import locale import codecs +import decimal +import fractions try: import threading except ImportError: @@ -870,6 +872,16 @@ os.makedirs(path, mode=mode, exist_ok=True) os.umask(old_mask) + def test_chown_uid_gid_arguments_must_be_index(self): + stat = os.stat(support.TESTFN) + uid = stat.st_uid + gid = stat.st_gid + for value in (-1.0, -1j, decimal.Decimal(-1), fractions.Fraction(-2, 2)): + self.assertRaises(TypeError, os.chown, support.TESTFN, value, gid) + self.assertRaises(TypeError, os.chown, support.TESTFN, uid, value) + self.assertIsNone(os.chown(support.TESTFN, uid, gid)) + self.assertIsNone(os.chown(support.TESTFN, -1, -1)) + def test_exist_ok_s_isgid_directory(self): path = os.path.join(support.TESTFN, 'dir1') S_ISGID = stat.S_ISGID diff -r 4a6b8f86b081 Modules/posixmodule.c --- a/Modules/posixmodule.c Mon Apr 15 23:14:55 2013 -0700 +++ b/Modules/posixmodule.c Mon Apr 22 12:09:27 2013 -0700 @@ -405,108 +405,221 @@ int _Py_Uid_Converter(PyObject *obj, void *p) { + uid_t uid; + PyObject *index; int overflow; long result; - if (PyFloat_Check(obj)) { - PyErr_SetString(PyExc_TypeError, - "integer argument expected, got float"); + unsigned long uresult; + + index = PyNumber_Index(obj); + if (index == NULL) { + PyErr_Format(PyExc_TypeError, + "uid should be integer, not %.200s", + Py_TYPE(obj)->tp_name); return 0; } - result = PyLong_AsLongAndOverflow(obj, &overflow); + + /* + * Handling uid_t is complicated for two reasons: + * * Although uid_t is (always?) unsigned, it still + * accepts -1. + * * We don't know its size in advance--it may be + * bigger than an int, or it may be smaller than + * a long. + * + * So a bit of defensive programming is in order. + * Start with interpreting the value passed + * in as a signed long and see if it works. + */ + + result = PyLong_AsLongAndOverflow(index, &overflow); + + if (!overflow) { + uid = (uid_t)result; + + if (result == -1) { + if (PyErr_Occurred()) + goto fail; + /* It's a legitimate -1, we're done. */ + goto success; + } + + /* Any other negative number is disallowed. */ + if (result < 0) + goto underflow; + + /* Ensure the value wasn't truncated. */ + if (sizeof(uid_t) < sizeof(long) && + (long)uid != result) + goto underflow; + goto success; + } + if (overflow < 0) - goto OverflowDown; - if (!overflow && result == -1) { - /* error or -1 */ - if (PyErr_Occurred()) - return 0; - *(uid_t *)p = (uid_t)-1; - } - else { - /* unsigned uid_t */ - unsigned long uresult; - if (overflow > 0) { - uresult = PyLong_AsUnsignedLong(obj); - if (PyErr_Occurred()) { - if (PyErr_ExceptionMatches(PyExc_OverflowError)) - goto OverflowUp; - return 0; - } - if ((uid_t)uresult == (uid_t)-1) - goto OverflowUp; - } else { - if (result < 0) - goto OverflowDown; - uresult = result; - } - if (sizeof(uid_t) < sizeof(long) && - (unsigned long)(uid_t)uresult != uresult) - goto OverflowUp; - *(uid_t *)p = (uid_t)uresult; - } + goto underflow; + + /* + * Okay, the value overflowed a signed long. If it + * fits in an *unsigned* long, it may still be okay, + * as uid_t may be unsigned long on this platform. + */ + uresult = PyLong_AsUnsignedLong(index); + if (PyErr_Occurred()) { + if (PyErr_ExceptionMatches(PyExc_OverflowError)) + goto overflow; + goto fail; + } + + uid = (uid_t)uresult; + + /* + * If uid reads as a -1, we've gotten ULONG_MAX. + * Throw an overflow so we don't accidentally read + * it as a -1. (We already handled a real -1 with + * PyLong_AsLongAndOverflow() above.) + * + * (Note: this code is arguably wrong if uid_t is + * unsigned. The POSIX standard makes no statement + * on whether uid_t is signed or unsigned, but in + * practice it's always signed.) + */ + if (uid == (uid_t)-1) + goto overflow; + + /* Ensure the value wasn't truncated. */ + if (sizeof(uid_t) < sizeof(long) && + (unsigned long)uid != uresult) + goto overflow; + /* fallthrough */ + +success: + Py_DECREF(index); + *(uid_t *)p = uid; return 1; -OverflowDown: +underflow: PyErr_SetString(PyExc_OverflowError, - "user id is less than minimum"); - return 0; - -OverflowUp: + "uid is less than minimum"); + goto fail; + +overflow: PyErr_SetString(PyExc_OverflowError, - "user id is greater than maximum"); + "uid is greater than maximum"); + /* fallthrough */ + +fail: + Py_DECREF(index); return 0; } int _Py_Gid_Converter(PyObject *obj, void *p) { + gid_t gid; + PyObject *index; int overflow; long result; - if (PyFloat_Check(obj)) { - PyErr_SetString(PyExc_TypeError, - "integer argument expected, got float"); + unsigned long uresult; + + index = PyNumber_Index(obj); + if (index == NULL) { + PyErr_Format(PyExc_TypeError, + "gid should be integer, not %.200s", + Py_TYPE(obj)->tp_name); return 0; } - result = PyLong_AsLongAndOverflow(obj, &overflow); + + /* + * Handling gid_t is complicated for two reasons: + * * Although gid_t is (always?) unsigned, it still + * accepts -1. + * * We don't know its size in advance--it may be + * bigger than an int, or it may be smaller than + * a long. + * + * So a bit of defensive programming is in order. + * Start with interpreting the value passed + * in as a signed long and see if it works. + */ + + result = PyLong_AsLongAndOverflow(index, &overflow); + + if (!overflow) { + gid = (gid_t)result; + + if (result == -1) { + if (PyErr_Occurred()) + goto fail; + /* It's a legitimate -1, we're done. */ + goto success; + } + + /* Any other negative number is disallowed. */ + if (result < 0) { + goto underflow; + } + + /* Ensure the value wasn't truncated. */ + if (sizeof(gid_t) < sizeof(long) && + (long)gid != result) + goto underflow; + goto success; + } + if (overflow < 0) - goto OverflowDown; - if (!overflow && result == -1) { - /* error or -1 */ - if (PyErr_Occurred()) - return 0; - *(gid_t *)p = (gid_t)-1; - } - else { - /* unsigned gid_t */ - unsigned long uresult; - if (overflow > 0) { - uresult = PyLong_AsUnsignedLong(obj); - if (PyErr_Occurred()) { - if (PyErr_ExceptionMatches(PyExc_OverflowError)) - goto OverflowUp; - return 0; - } - if ((gid_t)uresult == (gid_t)-1) - goto OverflowUp; - } else { - if (result < 0) - goto OverflowDown; - uresult = result; - } - if (sizeof(gid_t) < sizeof(long) && - (unsigned long)(gid_t)uresult != uresult) - goto OverflowUp; - *(gid_t *)p = (gid_t)uresult; - } + goto underflow; + + /* + * Okay, the value overflowed a signed long. If it + * fits in an *unsigned* long, it may still be okay, + * as gid_t may be unsigned long on this platform. + */ + uresult = PyLong_AsUnsignedLong(index); + if (PyErr_Occurred()) { + if (PyErr_ExceptionMatches(PyExc_OverflowError)) + goto overflow; + goto fail; + } + + gid = (gid_t)uresult; + + /* + * If gid reads as a -1, we've gotten ULONG_MAX. + * Throw an overflow so we don't accidentally read + * it as a -1. (We already handled a real -1 with + * PyLong_AsLongAndOverflow() above.) + * + * (Note: this code is arguably wrong if gid_t is + * unsigned. The POSIX standard makes no statement + * on whether gid_t is signed or unsigned, but in + * practice it's always signed.) + */ + if (gid == (gid_t)-1) + goto overflow; + + /* Ensure the value wasn't truncated. */ + if (sizeof(gid_t) < sizeof(long) && + (unsigned long)gid != uresult) + goto overflow; + /* fallthrough */ + +success: + Py_DECREF(index); + *(gid_t *)p = gid; return 1; -OverflowDown: +underflow: PyErr_SetString(PyExc_OverflowError, - "group id is less than minimum"); - return 0; - -OverflowUp: + "gid is less than minimum"); + goto fail; + +overflow: PyErr_SetString(PyExc_OverflowError, - "group id is greater than maximum"); + "gid is greater than maximum"); + /* fallthrough */ + +fail: + Py_DECREF(index); return 0; } #endif /* MS_WINDOWS */ @@ -529,25 +642,29 @@ _fd_converter(PyObject *o, int *p, const char *allowed) { int overflow; - long long_value = PyLong_AsLongAndOverflow(o, &overflow); - if (PyFloat_Check(o) || - (long_value == -1 && !overflow && PyErr_Occurred())) { - PyErr_Clear(); + long long_value; + + PyObject *index = PyNumber_Index(o); + if (index == NULL) { PyErr_Format(PyExc_TypeError, - "argument should be %s, not %.200s", - allowed, Py_TYPE(o)->tp_name); + "argument should be %s, not %.200s", + allowed, Py_TYPE(o)->tp_name); return 0; } + + long_value = PyLong_AsLongAndOverflow(index, &overflow); + Py_DECREF(index); if (overflow > 0 || long_value > INT_MAX) { PyErr_SetString(PyExc_OverflowError, - "signed integer is greater than maximum"); + "fd is greater than maximum"); return 0; } if (overflow < 0 || long_value < INT_MIN) { PyErr_SetString(PyExc_OverflowError, - "signed integer is less than minimum"); + "fd is less than minimum"); return 0; } + *p = (int)long_value; return 1; }