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

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).

Adding tests helps the other VMs, which generally are trailing behind the CPython releases.

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.

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.

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.)

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.

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.

This is a bit related to #13848 files 2 seconds earlier.

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).

See also issue #13617.

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.

> 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 :-)

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 :)

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.

> 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.

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.

Attached patch adds tests for common members of Lib/genericpath.py.

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.

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

Thanks for your review, Victor. I don't care about 2.7 at this point so I went with 3.5+ :)

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

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

Buildbots seem happier now