Issue #23908: os functions, open() and the io.FileIO constructor now reject

unicode paths with embedded null character on Windows instead of silently
truncate them.

diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py
index 8fdad14880c791365c8f14f1dd09c9c118247dab..634a7641ebe60f90050307c6344a38f9157bbcd3 100644 (file)
--- a/Lib/test/test_fileio.py
+++ b/Lib/test/test_fileio.py
@@ -361,6 +361,11 @@ class OtherFileTests(unittest.TestCase):
         finally:
             os.unlink(TESTFN)
 
+    def testConstructorHandlesNULChars(self):
+        fn_with_NUL = 'foo\0bar'
+        self.assertRaises(TypeError, _FileIO, fn_with_NUL, 'w')
+        self.assertRaises(TypeError, _FileIO, fn_with_NUL.encode('ascii'), 'w')
+
     def testInvalidFd(self):
         self.assertRaises(ValueError, _FileIO, -10)
         self.assertRaises(OSError, _FileIO, make_bad_fd())

diff --git a/Lib/test/test_getargs2.py b/Lib/test/test_getargs2.py
index 2b2fa607149fd06ce483e0ed8eb3425715934d51..e0efcaae9c271ee0ca6b70036bbe1cff5c83211e 100644 (file)
--- a/Lib/test/test_getargs2.py
+++ b/Lib/test/test_getargs2.py
@@ -779,7 +779,7 @@ class Unicode_TestCase(unittest.TestCase):
     def test_u(self):
         from _testcapi import getargs_u
         self.assertEqual(getargs_u(u'abc\xe9'), u'abc\xe9')
-        self.assertEqual(getargs_u(u'nul:\0'), u'nul:')
+        self.assertRaises(TypeError, getargs_u, u'nul:\0')
         self.assertRaises(TypeError, getargs_u, 'bytes')
         self.assertRaises(TypeError, getargs_u, bytearray('bytearray'))
         self.assertRaises(TypeError, getargs_u, memoryview('memoryview'))

diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index e26ffba22966cdd81e81c2a4ddd642e59577c47a..3d4da46113927bab843592f951e2762b08dae960 100644 (file)
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -349,6 +349,15 @@ class IOTest(unittest.TestCase):
             self.assertRaises(IOError, fp.write, "blah")
             self.assertRaises(IOError, fp.writelines, ["blah\n"])
 
+    def test_open_handles_NUL_chars(self):
+        fn_with_NUL = 'foo\0bar'
+        self.assertRaises(TypeError, self.open, fn_with_NUL, 'w')
+
+        bytes_fn = fn_with_NUL.encode('ascii')
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            self.assertRaises(TypeError, self.open, bytes_fn, 'w')
+
     def test_raw_file_io(self):
         with self.open(support.TESTFN, "wb", buffering=0) as f:
             self.assertEqual(f.readable(), False)

diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py
index 9810c77bc833917c9e4f31e9a7adbb9ca04725aa..7478234a9e041ee4cd985b3ce7279fb7f81b05f6 100644 (file)
--- a/Lib/test/test_posix.py
+++ b/Lib/test/test_posix.py
@@ -577,6 +577,44 @@ class PosixTester(unittest.TestCase):
                 set([int(x) for x in groups.split()]),
                 set(posix.getgroups() + [posix.getegid()]))
 
+    @test_support.requires_unicode
+    def test_path_with_null_unicode(self):
+        fn = test_support.TESTFN_UNICODE
+        fn_with_NUL = fn + u'\0'
+        self.addCleanup(test_support.unlink, fn)
+        test_support.unlink(fn)
+        fd = None
+        try:
+            with self.assertRaises(TypeError):
+                fd = os.open(fn_with_NUL, os.O_WRONLY | os.O_CREAT) # raises
+        finally:
+            if fd is not None:
+                os.close(fd)
+        self.assertFalse(os.path.exists(fn))
+        self.assertRaises(TypeError, os.mkdir, fn_with_NUL)
+        self.assertFalse(os.path.exists(fn))
+        open(fn, 'wb').close()
+        self.assertRaises(TypeError, os.stat, fn_with_NUL)
+
+    def test_path_with_null_byte(self):
+        fn = test_support.TESTFN
+        fn_with_NUL = fn + '\0'
+        self.addCleanup(test_support.unlink, fn)
+        test_support.unlink(fn)
+        fd = None
+        try:
+            with self.assertRaises(TypeError):
+                fd = os.open(fn_with_NUL, os.O_WRONLY | os.O_CREAT) # raises
+        finally:
+            if fd is not None:
+                os.close(fd)
+        self.assertFalse(os.path.exists(fn))
+        self.assertRaises(TypeError, os.mkdir, fn_with_NUL)
+        self.assertFalse(os.path.exists(fn))
+        open(fn, 'wb').close()
+        self.assertRaises(TypeError, os.stat, fn_with_NUL)
+
+
 class PosixGroupsTester(unittest.TestCase):
 
     def setUp(self):

diff --git a/Misc/NEWS b/Misc/NEWS
index 503df9969de03071bf8a0a2fdcadf9a9d7f030cf..fbfacca749e913198b2b42eda945d388191d8cd4 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,10 @@ What's New in Python 2.7.13?
 Core and Builtins
 -----------------
 
+- Issue #23908: os functions, open() and the io.FileIO constructor now reject
+  unicode paths with embedded null character on Windows instead of silently
+  truncate them.
+
 Library
 -------
 

diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c
index 74b23051bdd36304dfb65d455997b915d56df32a..0678ef8e35409171fabf3e48d7e6c4ec86d0625f 100644 (file)
--- a/Modules/_io/fileio.c
+++ b/Modules/_io/fileio.c
@@ -224,8 +224,13 @@ fileio_init(PyObject *oself, PyObject *args, PyObject *kwds)
     }
 
 #ifdef MS_WINDOWS
-    if (PyUnicode_Check(nameobj))
+    if (PyUnicode_Check(nameobj)) {
         widename = PyUnicode_AS_UNICODE(nameobj);
+        if (wcslen(widename) != (size_t)PyUnicode_GET_SIZE(nameobj)) {
+            PyErr_SetString(PyExc_TypeError, "embedded NUL character");
+            return -1;
+        }
+    }
     if (widename == NULL)
 #endif
     if (fd < 0)
@@ -234,6 +239,10 @@ fileio_init(PyObject *oself, PyObject *args, PyObject *kwds)
             Py_ssize_t namelen;
             if (PyObject_AsCharBuffer(nameobj, &name, &namelen) < 0)
                 return -1;
+            if (strlen(name) != (size_t)namelen) {
+                PyErr_SetString(PyExc_TypeError, "embedded NUL character");
+                return -1;
+            }
         }
         else {
             PyObject *u = PyUnicode_FromObject(nameobj);
@@ -252,6 +261,10 @@ fileio_init(PyObject *oself, PyObject *args, PyObject *kwds)
                 goto error;
             }
             name = PyBytes_AS_STRING(stringobj);
+            if (strlen(name) != (size_t)PyBytes_GET_SIZE(stringobj)) {
+                PyErr_SetString(PyExc_TypeError, "embedded NUL character");
+                goto error;
+            }
         }
     }
 

diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 4fc3ef75a12ca6a89461605b064075699af6987c..df3c32671453f9c2da2e59e1d26eecc23a627c39 100644 (file)
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -1646,13 +1646,9 @@ posix_do_stat(PyObject *self, PyObject *args,
     PyObject *result;
 
 #ifdef MS_WINDOWS
-    PyUnicodeObject *po;
-    if (PyArg_ParseTuple(args, wformat, &po)) {
-        Py_UNICODE *wpath = PyUnicode_AS_UNICODE(po);
-
+    Py_UNICODE *wpath;
+    if (PyArg_ParseTuple(args, wformat, &wpath)) {
         Py_BEGIN_ALLOW_THREADS
-            /* PyUnicode_AS_UNICODE result OK without
-               thread lock as it is a simple dereference. */
         res = wstatfunc(wpath, &st);
         Py_END_ALLOW_THREADS
 
@@ -1706,12 +1702,10 @@ posix_access(PyObject *self, PyObject *args)
 
 #ifdef MS_WINDOWS
     DWORD attr;
-    PyUnicodeObject *po;
-    if (PyArg_ParseTuple(args, "Ui:access", &po, &mode)) {
+    Py_UNICODE *wpath;
+    if (PyArg_ParseTuple(args, "ui:access", &wpath, &mode)) {
         Py_BEGIN_ALLOW_THREADS
-        /* PyUnicode_AS_UNICODE OK without thread lock as
-           it is a simple dereference. */
-        attr = GetFileAttributesW(PyUnicode_AS_UNICODE(po));
+        attr = GetFileAttributesW(wpath);
         Py_END_ALLOW_THREADS
         goto finish;
     }
@@ -1858,23 +1852,22 @@ posix_chmod(PyObject *self, PyObject *args)
     int res;
 #ifdef MS_WINDOWS
     DWORD attr;
-    PyUnicodeObject *po;
-    if (PyArg_ParseTuple(args, "Ui|:chmod", &po, &i)) {
+    Py_UNICODE *wpath;
+    if (PyArg_ParseTuple(args, "ui|:chmod", &wpath, &i)) {
         Py_BEGIN_ALLOW_THREADS
-        attr = GetFileAttributesW(PyUnicode_AS_UNICODE(po));
+        attr = GetFileAttributesW(wpath);
         if (attr != 0xFFFFFFFF) {
             if (i & _S_IWRITE)
                 attr &= ~FILE_ATTRIBUTE_READONLY;
             else
                 attr |= FILE_ATTRIBUTE_READONLY;
-            res = SetFileAttributesW(PyUnicode_AS_UNICODE(po), attr);
+            res = SetFileAttributesW(wpath, attr);
         }
         else
             res = 0;
         Py_END_ALLOW_THREADS
         if (!res)
-            return win32_error_unicode("chmod",
-                                       PyUnicode_AS_UNICODE(po));
+            return win32_error_unicode("chmod", wpath);
         Py_INCREF(Py_None);
         return Py_None;
     }
@@ -2300,18 +2293,18 @@ posix_listdir(PyObject *self, PyObject *args)
     char *bufptr = namebuf;
     Py_ssize_t len = sizeof(namebuf)-5; /* only claim to have space for MAX_PATH */
 
-    PyObject *po;
-    if (PyArg_ParseTuple(args, "U:listdir", &po)) {
+    Py_UNICODE *wpath;
+    if (PyArg_ParseTuple(args, "u:listdir", &wpath)) {
         WIN32_FIND_DATAW wFileData;
         Py_UNICODE *wnamebuf;
         /* Overallocate for \\*.*\0 */
-        len = PyUnicode_GET_SIZE(po);
+        len = wcslen(wpath);
         wnamebuf = malloc((len + 5) * sizeof(wchar_t));
         if (!wnamebuf) {
             PyErr_NoMemory();
             return NULL;
         }
-        wcscpy(wnamebuf, PyUnicode_AS_UNICODE(po));
+        wcscpy(wnamebuf, wpath);
         if (len > 0) {
             Py_UNICODE wch = wnamebuf[len-1];
             if (wch != L'/' && wch != L'\\' && wch != L':')
@@ -2615,9 +2608,8 @@ posix__getfullpathname(PyObject *self, PyObject *args)
     char outbuf[MAX_PATH*2];
     char *temp;
 
-    PyUnicodeObject *po;
-    if (PyArg_ParseTuple(args, "U|:_getfullpathname", &po)) {
-        Py_UNICODE *wpath = PyUnicode_AS_UNICODE(po);
+    Py_UNICODE *wpath;
+    if (PyArg_ParseTuple(args, "u|:_getfullpathname", &wpath)) {
         Py_UNICODE woutbuf[MAX_PATH*2], *woutbufp = woutbuf;
         Py_UNICODE *wtemp;
         DWORD result;
@@ -2670,15 +2662,13 @@ posix_mkdir(PyObject *self, PyObject *args)
     int mode = 0777;
 
 #ifdef MS_WINDOWS
-    PyUnicodeObject *po;
-    if (PyArg_ParseTuple(args, "U|i:mkdir", &po, &mode)) {
+    Py_UNICODE *wpath;
+    if (PyArg_ParseTuple(args, "u|i:mkdir", &wpath, &mode)) {
         Py_BEGIN_ALLOW_THREADS
-        /* PyUnicode_AS_UNICODE OK without thread lock as
-           it is a simple dereference. */
-        res = CreateDirectoryW(PyUnicode_AS_UNICODE(po), NULL);
+        res = CreateDirectoryW(wpath, NULL);
         Py_END_ALLOW_THREADS
         if (!res)
-            return win32_error_unicode("mkdir", PyUnicode_AS_UNICODE(po));
+            return win32_error_unicode("mkdir", wpath);
         Py_INCREF(Py_None);
         return Py_None;
     }
@@ -2689,8 +2679,6 @@ posix_mkdir(PyObject *self, PyObject *args)
                           Py_FileSystemDefaultEncoding, &path, &mode))
         return NULL;
     Py_BEGIN_ALLOW_THREADS
-    /* PyUnicode_AS_UNICODE OK without thread lock as
-       it is a simple dereference. */
     res = CreateDirectoryA(path, NULL);
     Py_END_ALLOW_THREADS
     if (!res) {
@@ -2833,7 +2821,7 @@ static PyObject *
 posix_stat(PyObject *self, PyObject *args)
 {
 #ifdef MS_WINDOWS
-    return posix_do_stat(self, args, "et:stat", STAT, "U:stat", win32_wstat);
+    return posix_do_stat(self, args, "et:stat", STAT, "u:stat", win32_wstat);
 #else
     return posix_do_stat(self, args, "et:stat", STAT, NULL, NULL);
 #endif
@@ -2969,7 +2957,6 @@ posix_utime(PyObject *self, PyObject *args)
 {
 #ifdef MS_WINDOWS
     PyObject *arg;
-    PyUnicodeObject *obwpath;
     wchar_t *wpath = NULL;
     char *apath = NULL;
     HANDLE hFile;
@@ -2978,8 +2965,7 @@ posix_utime(PyObject *self, PyObject *args)
     FILETIME atime, mtime;
     PyObject *result = NULL;
 
-    if (PyArg_ParseTuple(args, "UO|:utime", &obwpath, &arg)) {
-        wpath = PyUnicode_AS_UNICODE(obwpath);
+    if (PyArg_ParseTuple(args, "uO|:utime", &wpath, &arg)) {
         Py_BEGIN_ALLOW_THREADS
         hFile = CreateFileW(wpath, FILE_WRITE_ATTRIBUTES, 0,
                             NULL, OPEN_EXISTING,
@@ -4440,14 +4426,11 @@ PyDoc_STRVAR(posix__isdir__doc__,
 static PyObject *
 posix__isdir(PyObject *self, PyObject *args)
 {
-    PyObject *opath;
     char *path;
-    PyUnicodeObject *po;
+    Py_UNICODE *wpath;
     DWORD attributes;
 
-    if (PyArg_ParseTuple(args, "U|:_isdir", &po)) {
-        Py_UNICODE *wpath = PyUnicode_AS_UNICODE(po);
-
+    if (PyArg_ParseTuple(args, "u|:_isdir", &wpath)) {
         attributes = GetFileAttributesW(wpath);
         if (attributes == INVALID_FILE_ATTRIBUTES)
             Py_RETURN_FALSE;
@@ -6326,7 +6309,7 @@ posix_lstat(PyObject *self, PyObject *args)
     return posix_do_stat(self, args, "et:lstat", lstat, NULL, NULL);
 #else /* !HAVE_LSTAT */
 #ifdef MS_WINDOWS
-    return posix_do_stat(self, args, "et:lstat", STAT, "U:lstat", win32_wstat);
+    return posix_do_stat(self, args, "et:lstat", STAT, "u:lstat", win32_wstat);
 #else
     return posix_do_stat(self, args, "et:lstat", STAT, NULL, NULL);
 #endif
@@ -6600,12 +6583,10 @@ posix_open(PyObject *self, PyObject *args)
     int fd;
 
 #ifdef MS_WINDOWS
-    PyUnicodeObject *po;
-    if (PyArg_ParseTuple(args, "Ui|i:mkdir", &po, &flag, &mode)) {
+    Py_UNICODE *wpath;
+    if (PyArg_ParseTuple(args, "ui|i:mkdir", &wpath, &flag, &mode)) {
         Py_BEGIN_ALLOW_THREADS
-        /* PyUnicode_AS_UNICODE OK without thread
-           lock as it is a simple dereference. */
-        fd = _wopen(PyUnicode_AS_UNICODE(po), flag, mode);
+        fd = _wopen(wpath, flag, mode);
         Py_END_ALLOW_THREADS
         if (fd < 0)
             return posix_error();
@@ -8662,12 +8643,13 @@ static PyObject *
 win32_startfile(PyObject *self, PyObject *args)
 {
     char *filepath;
+    Py_UNICODE *wpath;
     char *operation = NULL;
     HINSTANCE rc;
 
-    PyObject *unipath, *woperation = NULL;
-    if (!PyArg_ParseTuple(args, "U|s:startfile",
-                          &unipath, &operation)) {
+    PyObject *woperation = NULL;
+    if (!PyArg_ParseTuple(args, "u|s:startfile",
+                          &wpath, &operation)) {
         PyErr_Clear();
         goto normal;
     }
@@ -8684,14 +8666,13 @@ win32_startfile(PyObject *self, PyObject *args)
 
     Py_BEGIN_ALLOW_THREADS
     rc = ShellExecuteW((HWND)0, woperation ? PyUnicode_AS_UNICODE(woperation) : 0,
-        PyUnicode_AS_UNICODE(unipath),
+        wpath,
         NULL, NULL, SW_SHOWNORMAL);
     Py_END_ALLOW_THREADS
 
     Py_XDECREF(woperation);
     if (rc <= (HINSTANCE)32) {
-        PyObject *errval = win32_error_unicode("startfile",
-                                               PyUnicode_AS_UNICODE(unipath));
+        PyObject *errval = win32_error_unicode("startfile", wpath);
         return errval;
     }
     Py_INCREF(Py_None);

diff --git a/Objects/fileobject.c b/Objects/fileobject.c
index b29da8a5f58408b2f22c37d540b97364cc76ec03..a7d64ba16d440b878ce6a257e098c0f17c4ae4b8 100644 (file)
--- a/Objects/fileobject.c
+++ b/Objects/fileobject.c
@@ -2394,7 +2394,8 @@ file_init(PyObject *self, PyObject *args, PyObject *kwds)
 
 #ifdef MS_WINDOWS
     if (PyArg_ParseTupleAndKeywords(args, kwds, "U|si:file",
-                                    kwlist, &po, &mode, &bufsize)) {
+                                    kwlist, &po, &mode, &bufsize) &&
+        wcslen(PyUnicode_AS_UNICODE(po)) == (size_t)PyUnicode_GET_SIZE(po)) {
         wideargument = 1;
         if (fill_file_fields(foself, NULL, po, mode,
                              fclose) == NULL)

diff --git a/Python/getargs.c b/Python/getargs.c
index bd15c2e08991ed772e574771bd0a63b094ab59ca..32ff6fc069d3f3a1eb369cd0c895d4e891c3162b 100644 (file)
--- a/Python/getargs.c
+++ b/Python/getargs.c
@@ -576,6 +576,17 @@ float_argument_error(PyObject *arg)
         return 0;
 }
 
+#ifdef Py_USING_UNICODE
+static size_t
+_ustrlen(Py_UNICODE *u)
+{
+    size_t i = 0;
+    Py_UNICODE *v = u;
+    while (*v != 0) { i++; v++; }
+    return i;
+}
+#endif
+
 /* Convert a non-tuple argument.  Return NULL if conversion went OK,
    or a string with a message describing the failure.  The message is
    formatted as "must be <desired type>, not <actual type>".
@@ -1202,8 +1213,14 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
             format++;
         } else {
             Py_UNICODE **p = va_arg(*p_va, Py_UNICODE **);
-            if (PyUnicode_Check(arg))
+            if (PyUnicode_Check(arg)) {
                 *p = PyUnicode_AS_UNICODE(arg);
+                if (_ustrlen(*p) != (size_t)PyUnicode_GET_SIZE(arg)) {
+                    return converterr(
+                        "unicode without null characters",
+                        arg, msgbuf, bufsize);
+                }
+            }
             else
                 return converterr("unicode", arg, msgbuf, bufsize);
         }