msg240435 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-04-10 16:43 |
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.
|
msg240594 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2015-04-13 07:29 |
> 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.
|
msg240849 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-04-14 06:27 |
I can't test the patch on UNIX, this branch of the code is executed only on Windows.
|
msg240857 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2015-04-14 08:06 |
> 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?
|
msg240872 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-04-14 11:39 |
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.
|
msg241620 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-04-20 07:14 |
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
|
msg241629 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-04-20 11:03 |
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).
|
msg268942 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-06-20 21:52 |
Proposed patch fixes this issue and issue13848 in 2.7.
|
msg268943 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-06-20 21:53 |
Since the large part of code is Windows specific (and even is not compiled on Linux), the patch needs testing on Windows.
|
msg269647 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-07-01 06:25 |
Ping. Can anybody please test the patch on Windows?
|
msg269677 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2016-07-01 16:17 |
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.
|
msg269694 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-07-01 20:35 |
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
|
msg269695 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-07-01 20:36 |
Thank you Zachary.
|
msg269752 - (view) |
Author: Kubilay Kocak (koobs) |
Date: 2016-07-03 05:09 |
koobs-freebsd{9,10,current) failing after 30099abdb3a46d0e306a4cf995b95fa8cfb8b78a merge to 2.7
|
msg269753 - (view) |
Author: Kubilay Kocak (koobs) |
Date: 2016-07-03 05:10 |
======================================================================
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
|
msg269754 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-07-03 07:55 |
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
|
msg269755 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-07-03 07:57 |
Thank you for noticing this.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:15 | admin | set | github: 68096 |
2016-07-17 10:03:23 | serhiy.storchaka | set | status: open -> closed resolution: fixed |
2016-07-03 07:57:14 | serhiy.storchaka | set | messages:
+ msg269755 |
2016-07-03 07:55:41 | python-dev | set | messages:
+ msg269754 |
2016-07-03 05:10:18 | koobs | set | messages:
+ msg269753 |
2016-07-03 05:09:26 | koobs | set | status: closed -> open
nosy:
+ koobs messages:
+ msg269752
resolution: fixed -> (no value) |
2016-07-01 20:36:25 | serhiy.storchaka | set | status: open -> closed resolution: fixed messages:
+ msg269695
stage: patch review -> resolved |
2016-07-01 20:35:17 | python-dev | set | messages:
+ msg269694 |
2016-07-01 16:17:45 | zach.ware | set | messages:
+ msg269677 |
2016-07-01 06:25:01 | serhiy.storchaka | set | messages:
+ msg269647 |
2016-06-20 21:53:16 | serhiy.storchaka | set | messages:
+ msg268943 |
2016-06-20 21:52:07 | serhiy.storchaka | set | files:
+ unicode_converter_null_char-2.7.patch
nosy:
+ benjamin.peterson messages:
+ msg268942
stage: needs patch -> patch review |
2015-05-12 11:06:09 | serhiy.storchaka | unlink | issue21859 dependencies |
2015-04-20 11:03:51 | serhiy.storchaka | set | stage: patch review -> needs patch messages:
+ msg241629 versions:
+ Python 2.7, - Python 3.4, Python 3.5 |
2015-04-20 07:14:33 | python-dev | set | nosy:
+ python-dev messages:
+ msg241620
|
2015-04-14 11:39:47 | serhiy.storchaka | set | files:
+ path_converter_null_char_2.patch
messages:
+ msg240872 |
2015-04-14 08:06:58 | vstinner | set | messages:
+ msg240857 |
2015-04-14 06:27:57 | serhiy.storchaka | set | messages:
+ msg240849 |
2015-04-13 07:29:38 | vstinner | set | messages:
+ msg240594 |
2015-04-12 05:20:44 | serhiy.storchaka | set | nosy:
+ tim.golden, zach.ware, steve.dower components:
+ Windows
|
2015-04-10 16:44:48 | serhiy.storchaka | link | issue21859 dependencies |
2015-04-10 16:43:52 | serhiy.storchaka | create | |