msg151877 - (view) |
Author: Alex Gaynor (alex) *  |
Date: 2012-01-24 02:05 |
ATM there's no tests (at least in 2.x, I haven't checked 3.x yet) for this behavior:
>>> os.path.exists("/tmp\x00abcds")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python3.2/genericpath.py", line 18, in exists
os.stat(path)
TypeError: embedded NUL character
|
msg151878 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-01-24 02:23 |
Why is that a bug? There is no feature in Python saying that the test suite covers the code fully (by some kind of measurement).
New tests should only be added to 3.3, unless they test for a newly-fixed bug (and even then the test may not be backported to the maintenance release).
|
msg151879 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2012-01-24 02:42 |
Adding tests helps the other VMs, which generally are trailing behind the CPython releases.
|
msg151883 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-01-24 08:12 |
If other VMs need this test for some reason, they can easily add a test case themselves. I'm -1 on adding test cases to bug fix releases just for completeness. A lacking test is not a bug, and hence must not be added to a bug fix release.
|
msg151903 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2012-01-24 13:21 |
My understanding (and recollection, but I don't have notes I can point at to hand) is that one goal that arose from recent VM and language summits was for the CPython test suite to be used as the validating test suite, with CPython-specific tests marked as such so that the other VMs can easily "not conform" to those tests. Thus my comment that it is helpful to the other VMs to add tests.
|
msg151904 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2012-01-24 13:25 |
Oh, and in case it isn't clear, this request is *coming* from one of the other VMs (pypy), so if my summit recollection is correct, they are in fact "adding a test that they need" by submitting this issue :)
(Or at least they will have once Alex posts the patch.)
|
msg151907 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-01-24 13:46 |
If so, I think this change should not checked into the 2.7 branch. Instead, a separate branch should be made for changes not intended for CPython, but for Python implementations in general. Making the Python test suite usable for other implementations is certainly not a feature of Python 2.7.
|
msg151908 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-01-24 13:49 |
Reconsidering: I think it shouldn't be checked into the cpython *repository*. Instead, if PyPy developers want to contribute changes to the test suite and standard library to improve the standard library, there should be a separate Mercurial repository for that, and PyPy developers might get a blanket write permission to add changes unless they explicitly feel a need for review.
|
msg152140 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2012-01-27 23:26 |
This is a bit related to #13848 files 2 seconds earlier.
|
msg152141 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-01-27 23:35 |
Well, adding tests is certainly useful to avoid regressions in CPython (not only PyPy or Jython). It seems to me that the only reasonable discussion is whether they should be committed to 2.7 and 3.2, or only the default branch.
(I personally like adding tests to stable branches as well, since that avoids potential regressions in further bugfix releases)
If we had had such tests from the beginning, 3.x wouldn't have regressed in that matter (see #13848).
|
msg153436 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-02-15 21:13 |
See also issue #13617.
|
msg160847 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-05-16 12:28 |
Why is this one still open? I'm afraid the questions raised aren't fit to be discussed in a ticket (weren't the outsourcing of of stdlib an item at the language summit?).
I tend to close it as rejected (although I rather disagree) unless someone has compelling reasons to keep it lingering.
|
msg160863 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-05-16 13:44 |
> Why is this one still open? I'm afraid the questions raised aren't
> fit to be discussed in a ticket
The original request (add tests for NUL character checking) is still relevant AFAIK. All it needs is a patch - by Alex or anybody else :-)
|
msg160891 - (view) |
Author: Alex Gaynor (alex) *  |
Date: 2012-05-16 17:20 |
I'm perfectly happy to write a patch for this, the only reason I didn't was Martin and others expressed opposition to committing it. If there's a philosophicaly opposition to the patch I won't write it :)
|
msg160925 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2012-05-16 19:53 |
There are two issues. 1) what is the requested changed and is it technically correct; 2) if correct, should it be made from a policy viewpoint.
1) Alex, do I understand correctly that you see the behavior of disallowing nulls in paths as correct, and that you want an AssertRaises(..., TypeError) test added?
This rule seems like an OS rule, not a Python language rule. Is it uniform across all OSes?
The following is near the top of the 3.3 os module doc:
"Note: All functions in this module raise OSError in the case of invalid or inaccessible file names and paths, or other arguments that have the correct type, but are not accepted by the operating system."
On that basis, I would expect OSError. But perhaps the doc needs revision to accord with the reworking of OSErrors. On 3.3, Win7, I actually get
>>> os.stat("/tmp\x00abcds")
Traceback (most recent call last):
File "<pyshell#3>", line 1, in <module>
os.stat("/tmp\x00abcds")
FileNotFoundError: [Error 2] The system cannot find the file specified: '/tmp\x00abcds'
It appears that os.stat did not check for embedded nulls but just passed the string to Windows.
2. I am not sure if I understand the opposition to adding tests. As I remember, the 2011 PyCon sprint partly focused on adding tests. Were they not added to 3.2?
If the test is correct, it cannot break anything, and can catch future reversions (which do happen occasionally), besides helping other implementations. 3.2 will soon be a moot point, but I hope other implementations seriously work on 3.3 versions, and that we are willing to improve its test suite after the initial release.
If we are split here, discussion could move to pydev. It is not unusual that a specific issue raises a generic issue that needs broader discussion, but with the specific issue as a focal point.
Anyway, until there is a patch that we agree is technically correct, no decision need be made about commitment and the underlying generic policy.
|
msg160996 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-05-17 18:28 |
> It appears that os.stat did not check for embedded nulls but just passed the string to Windows.
JFTR, we have fixed it for open() because it's a potential security problem. I have implemented a helper function for checking for NUL chars: _PyUnicode_HasNULChars
See the already mentioned #13848.
|
msg221845 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-06-29 14:09 |
The type and versions fields have been set to what I think they ought to be, as I couldn't find any reference to this issue on python-dev. Note that #13848 is closed as fixed and I've asked Victor if he can pick up #13617.
|
msg270954 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2016-07-21 19:20 |
Attached patch adds tests for common members of Lib/genericpath.py.
|
msg270965 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2016-07-21 22:44 |
issue13849.diff: LGTM.
If the tests pass on Python 2.7 and 3.5, I think that it's worth to add the tests to these versions as well.
|
msg271055 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-07-23 04:32 |
New changeset 899f06eb390c by Berker Peksag in branch '3.5':
Issue #13849: Add tests for null byte checking in test_genericpath
https://hg.python.org/cpython/rev/899f06eb390c
New changeset 9498736fbd8f by Berker Peksag in branch 'default':
Issue #13849: Merge from 3.5
https://hg.python.org/cpython/rev/9498736fbd8f
|
msg271056 - (view) |
Author: Berker Peksag (berker.peksag) *  |
Date: 2016-07-23 04:35 |
Thanks for your review, Victor. I don't care about 2.7 at this point so I went with 3.5+ :)
|
msg271059 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-07-23 05:21 |
http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.5/builds/1072/steps/test/logs/stdio
======================================================================
FAIL: test_null_bytes (test.test_genericpath.TestGenericTest) (attr='getsize')
----------------------------------------------------------------------
Traceback (most recent call last):
File "C:\buildbot.python.org\3.5.kloth-win64\build\lib\test\test_genericpath.py", line 285, in test_null_bytes
self.assertEqual(str(cm.exception), 'embedded null byte')
AssertionError: 'stat: embedded null character' != 'embedded null byte'
- stat: embedded null character
+ embedded null byte
|
msg271061 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-07-23 05:43 |
New changeset 9e997c8f4876 by Berker Peksag in branch '3.5':
Issue #13849: Fix test_null_bytes under Windows
https://hg.python.org/cpython/rev/9e997c8f4876
New changeset a28abe83cf5c by Berker Peksag in branch 'default':
Issue #13849: Merge from 3.5
https://hg.python.org/cpython/rev/a28abe83cf5c
|
msg271075 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-07-23 13:20 |
Buildbots seem happier now
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:26 | admin | set | github: 58057 |
2016-07-23 13:20:44 | martin.panter | set | status: open -> closed
messages:
+ msg271075 |
2016-07-23 05:43:09 | python-dev | set | messages:
+ msg271061 |
2016-07-23 05:21:05 | martin.panter | set | status: closed -> open nosy:
+ martin.panter messages:
+ msg271059
|
2016-07-23 04:35:17 | berker.peksag | set | status: open -> closed resolution: fixed messages:
+ msg271056
stage: patch review -> resolved |
2016-07-23 04:32:17 | python-dev | set | nosy:
+ python-dev messages:
+ msg271055
|
2016-07-21 22:51:27 | BreamoreBoy | set | nosy:
- BreamoreBoy
|
2016-07-21 22:44:45 | vstinner | set | messages:
+ msg270965 |
2016-07-21 19:20:27 | berker.peksag | set | files:
+ issue13849.diff
versions:
+ Python 3.6, - Python 3.4 keywords:
+ patch nosy:
+ berker.peksag
messages:
+ msg270954 stage: needs patch -> patch review |
2014-06-29 14:09:52 | BreamoreBoy | set | versions:
+ Python 3.4, Python 3.5, - Python 2.7, Python 3.2, Python 3.3 nosy:
+ BreamoreBoy
messages:
+ msg221845
type: enhancement |
2012-05-17 18:28:36 | hynek | set | messages:
+ msg160996 |
2012-05-17 16:45:35 | Arfrever | set | nosy:
+ Arfrever
|
2012-05-16 19:53:24 | terry.reedy | set | messages:
+ msg160925 |
2012-05-16 17:20:15 | alex | set | messages:
+ msg160891 |
2012-05-16 13:45:23 | r.david.murray | set | stage: needs patch |
2012-05-16 13:44:06 | pitrou | set | messages:
+ msg160863 |
2012-05-16 12:28:59 | hynek | set | messages:
+ msg160847 |
2012-02-15 21:13:13 | vstinner | set | nosy:
+ vstinner messages:
+ msg153436
|
2012-01-27 23:35:43 | pitrou | set | nosy:
+ pitrou
messages:
+ msg152141 versions:
+ Python 3.2, Python 3.3 |
2012-01-27 23:26:35 | terry.reedy | set | nosy:
+ terry.reedy messages:
+ msg152140
|
2012-01-24 13:49:57 | loewis | set | messages:
+ msg151908 |
2012-01-24 13:46:32 | loewis | set | messages:
+ msg151907 |
2012-01-24 13:31:45 | hynek | set | nosy:
+ hynek
|
2012-01-24 13:25:16 | r.david.murray | set | messages:
+ msg151904 |
2012-01-24 13:21:50 | r.david.murray | set | messages:
+ msg151903 |
2012-01-24 08:12:10 | loewis | set | messages:
+ msg151883 |
2012-01-24 02:42:06 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg151879
|
2012-01-24 02:23:36 | loewis | set | nosy:
+ loewis messages:
+ msg151878
|
2012-01-24 02:05:53 | alex | create | |