homepage

Proposed patch adds checks for null character in unicode path arguments of os functions on Windows. Null character is already tested on Unix, in bytes paths on Windows, and in unicode argument of _io.FileIO.
Removed private function _PyUnicode_HasNULChars(), because it is used only in two places and inlined code is simpler and more efficient.
The patch doesn't contain tests because I can't test them. But they should be simple, just pass a path with null character to os function.

> The patch doesn't contain tests because I can't test them. But they should be simple, just pass a path with null character to os function.
IMO this issue is a bug, so you must write an unit test to test for non regression. One test for FileIO, another for the os module should be enough, and you can test them on UNIX since the behaviour now must be the same.
Except of the test, path_converter_null_char.patch looks good to me.
Do you want to fix Python 2.7 and 3.4? A filename with an embedded null character can be a security vulnerability, so it's good to fix these versions too.

I can't test the patch on UNIX, this branch of the code is executed only on Windows.

> I can't test the patch on UNIX, this branch of the code is executed only on Windows.
If you write a test which ensures that a path with a null character raises a TypeError, it should pass on UNIX on your PC. Then builbots will ensure that it also pass on Windows, no?

I afraid not that builbots will fail, but that that test could pass without the fix. Tests should be tested without applying the fix. Writing right tests can require several iterations with running tests on patched and unpatched the os module.
Here is a patch with tests.

New changeset cdadde8396a4 by Serhiy Storchaka in branch '3.4':
Issue #23908: os functions now reject paths with embedded null character
https://hg.python.org/cpython/rev/cdadde8396a4
New changeset bdf13c7dcf7f by Serhiy Storchaka in branch 'default':
Issue #23908: os functions now reject paths with embedded null character
https://hg.python.org/cpython/rev/bdf13c7dcf7f 

The patch for 2.7 should be different and more complex. First at all, "s" and "u" format units don't check for null character (but "et" and "es" used in Unix implementations of os functions do).

Proposed patch fixes this issue and issue13848 in 2.7.

Since the large part of code is Windows specific (and even is not compiled on Linux), the patch needs testing on Windows.

Ping. Can anybody please test the patch on Windows?

I get one test failure:
ERROR: test_u (__main__.Unicode_TestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "P:\ath\to\cpython\lib\test\test_getargs2.py", line 782, in test_u
 self.assertEqual(getargs_u(u'nul:0円'), u'nul:')
TypeError: argument 1 must be unicode without null characters, not unicode
All other tests appear to pass.

New changeset 30099abdb3a4 by Serhiy Storchaka in branch '2.7':
Issue #23908: os functions, open() and the io.FileIO constructor now reject
https://hg.python.org/cpython/rev/30099abdb3a4 

Thank you Zachary.

koobs-freebsd{9,10,current) failing after 30099abdb3a46d0e306a4cf995b95fa8cfb8b78a merge to 2.7

======================================================================
ERROR: test_path_with_null_unicode (test.test_posix.PosixTester)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/usr/home/buildbot/python/2.7.koobs-freebsd9/build/Lib/test/test_posix.py", line 585, in test_path_with_null_unicode
 test_support.unlink(fn)
 File "/usr/home/buildbot/python/2.7.koobs-freebsd9/build/Lib/test/test_support.py", line 243, in unlink
 _unlink(filename)
UnicodeEncodeError: 'ascii' codec can't encode characters in position 6-7: ordinal not in range(128)
======================================================================
ERROR: test_path_with_null_unicode (test.test_posix.PosixTester)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/usr/home/buildbot/python/2.7.koobs-freebsd9/build/Lib/test/test_support.py", line 243, in unlink
 _unlink(filename)
UnicodeEncodeError: 'ascii' codec can't encode characters in position 6-7: ordinal not in range(128)
----------------------------------------------------------------------
Ran 43 tests in 0.105s

New changeset b0e8a2e7c361 by Serhiy Storchaka in branch '2.7':
Fixed a test for issue23908 with C locale.
https://hg.python.org/cpython/rev/b0e8a2e7c361 

Thank you for noticing this.