Skip to content

bpo-43916: Apply Py_TPFLAGS_DISALLOW_INSTANTIATION to selected types #25748

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 30, 2021
Merged

bpo-43916: Apply Py_TPFLAGS_DISALLOW_INSTANTIATION to selected types #25748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71439d0
bpo-43916: Apply Py_TPFLAGS_DISALLOW_INSTANTIATION to selected types
Apr 26, 2021
009ce60
Address review: improve naming/comments
Apr 30, 2021
13010fc
Address review: rename curses test
Apr 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Lib/test/test_array.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ def test_bad_constructor(self):
self.assertRaises(TypeError, array.array, 'xx')
self.assertRaises(ValueError, array.array, 'x')

@support.cpython_only
def test_disallow_instantiation(self):
# Ensure that the type disallows instantiation (bpo-43916)
tp = type(iter(array.array('I')))
self.assertRaises(TypeError, tp)
Comment on lines +46 to +47
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it would expect, the constructor would expect an argument:

Suggested change
tp = type(iter(array.array('I')))
self.assertRaises(TypeError, tp)
my_array = array.array('I')
arrayiterator = type(iter(my_array))
self.assertRaises(TypeError, arrayiterator, my_array)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I don't think so.

$ python3.10                  
>>> import array
>>> tp = type(iter(array.array('I')))
>>> tp()
<array.arrayiterator object at 0x10f934140>
>>> tp([])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: array.arrayiterator() takes no arguments
>>>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto for the similar suggestions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On what do you want to iterate if there is no argument?

Obviously, today, it takes no argument... because the type doesn't implement tp_new nor tp_init ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really have anything to say? We hit the right exception anyway, AFAICS.

$ ./python.exe 
Python 3.10.0a7+ (heads/add-disallow-instantiation-flag:71439d0de8, Apr 30 2021, 15:22:20) [Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import array
>>> tp = type(iter(array.array('I')))
>>> tp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'array.arrayiterator' instances
>>> tp([])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'array.arrayiterator' instances

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern can be addressed later by adding an helper function which checks the error message.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I insist on this issue because test_sys had a test which still passed when I modified sys.flags type to allow instantiation. I had to fix the test: 3bb0994

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I got it now. I misunderstood you first. Yes, that makes sense. We'll fix it when we've got a helper in place.


@support.cpython_only
def test_immutable(self):
# bpo-43908: check that array.array is immutable
Expand Down
6 changes: 4 additions & 2 deletions Lib/test/test_curses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import tempfile
import unittest

from test.support import requires, verbose, SaveSignals
from test.support import requires, verbose, SaveSignals, cpython_only
from test.support.import_helper import import_module

# Optionally test curses module. This currently requires that the
Expand Down Expand Up @@ -1046,8 +1046,10 @@ def __del__(self):
panel.set_userptr(A())
panel.set_userptr(None)

@cpython_only
@requires_curses_func('panel')
def test_new_curses_panel(self):
def test_disallow_instantiation(self):
# Ensure that the type disallows instantiation (bpo-43916)
w = curses.newwin(10, 10)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also guess that the constructor would expect an argument:

self.assertRaises(TypeError, type(panel), w)

panel = curses.panel.new_panel(w)
self.assertRaises(TypeError, type(panel))
Expand Down
9 changes: 8 additions & 1 deletion Lib/test/test_dbm_gnu.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from test import support
from test.support import import_helper
from test.support import import_helper, cpython_only
gdbm = import_helper.import_module("dbm.gnu") #skip if not supported
import unittest
import os
Expand Down Expand Up @@ -27,6 +27,13 @@ def tearDown(self):
self.g.close()
unlink(filename)

@cpython_only
def test_disallow_instantiation(self):
# Ensure that the type disallows instantiation (bpo-43916)
self.g = gdbm.open(filename, 'c')
tp = type(self.g)
self.assertRaises(TypeError, tp)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.assertRaises(TypeError, tp)
self.assertRaises(TypeError, tp, filename, 'c')


def test_key_methods(self):
self.g = gdbm.open(filename, 'c')
self.assertEqual(self.g.keys(), [])
Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_functools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,12 @@ class TestCmpToKeyC(TestCmpToKey, unittest.TestCase):
if c_functools:
cmp_to_key = c_functools.cmp_to_key

@support.cpython_only
def test_disallow_instantiation(self):
# Ensure that the type disallows instantiation (bpo-43916)
tp = type(c_functools.cmp_to_key(None))
self.assertRaises(TypeError, tp)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.assertRaises(TypeError, tp)
self.assertRaises(TypeError, tp, None)



class TestCmpToKeyPy(TestCmpToKey, unittest.TestCase):
cmp_to_key = staticmethod(py_functools.cmp_to_key)
Expand Down
9 changes: 9 additions & 0 deletions Lib/test/test_re.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2215,6 +2215,15 @@ def test_signedness(self):
self.assertGreaterEqual(sre_compile.MAXREPEAT, 0)
self.assertGreaterEqual(sre_compile.MAXGROUPS, 0)

@cpython_only
def test_disallow_instantiation(self):
# Ensure that the type disallows instantiation (bpo-43916)
self.assertRaises(TypeError, re.Match)
self.assertRaises(TypeError, re.Pattern)
pat = re.compile("")
tp = type(pat.scanner(""))
self.assertRaises(TypeError, tp)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.assertRaises(TypeError, tp)
self.assertRaises(TypeError, tp, "")



class ExternalTests(unittest.TestCase):

Expand Down
3 changes: 3 additions & 0 deletions Lib/test/test_tcl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,8 +736,11 @@ def check(value):
check('{\n')
check('}\n')

@support.cpython_only
def test_new_tcl_obj(self):
self.assertRaises(TypeError, _tkinter.Tcl_Obj)
self.assertRaises(TypeError, _tkinter.TkttType)
self.assertRaises(TypeError, _tkinter.TkappType)

class BigmemTclTest(unittest.TestCase):

Expand Down
7 changes: 7 additions & 0 deletions Lib/test/test_threading.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ def func(): pass
thread = threading.Thread(target=func)
self.assertEqual(thread.name, "Thread-5 (func)")

@cpython_only
def test_disallow_instantiation(self):
# Ensure that the type disallows instantiation (bpo-43916)
lock = threading.Lock()
tp = type(lock)
self.assertRaises(TypeError, tp)

# Create a bunch of threads, let each do some work, wait until all are
# done.
def test_various_ops(self):
Expand Down
8 changes: 7 additions & 1 deletion Lib/test/test_unicodedata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
import sys
import unicodedata
import unittest
from test.support import open_urlresource, requires_resource, script_helper
from test.support import (open_urlresource, requires_resource, script_helper,
cpython_only)


class UnicodeMethodsTest(unittest.TestCase):
Expand Down Expand Up @@ -225,6 +226,11 @@ def test_east_asian_width_9_0_changes(self):

class UnicodeMiscTest(UnicodeDatabaseTest):

@cpython_only
def test_disallow_instantiation(self):
# Ensure that the type disallows instantiation (bpo-43916)
self.assertRaises(TypeError, unicodedata.UCD)

def test_failed_import_during_compiling(self):
# Issue 4367
# Decoding \N escapes requires the unicodedata module. If it can't be
Expand Down
8 changes: 8 additions & 0 deletions Lib/test/test_zlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,14 @@ def test_overflow(self):
with self.assertRaisesRegex(OverflowError, 'int too large'):
zlib.decompressobj().flush(sys.maxsize + 1)

@support.cpython_only
def test_disallow_instantiation(self):
# Ensure that the type disallows instantiation (bpo-43916)
comp_type = type(zlib.compressobj())
decomp_type = type(zlib.decompressobj())
self.assertRaises(TypeError, comp_type)
self.assertRaises(TypeError, decomp_type)


class BaseCompressTestCase(object):
def check_big_compress_buffer(self, size, compress_func):
Expand Down
2 changes: 1 addition & 1 deletion Modules/_dbmmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ static PyType_Spec dbmtype_spec = {
// dbmtype_spec does not have Py_TPFLAGS_BASETYPE flag
// which prevents to create a subclass.
// So calling PyType_GetModuleState() in this file is always safe.
.flags = Py_TPFLAGS_DEFAULT,
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots = dbmtype_spec_slots,
};

Expand Down
4 changes: 2 additions & 2 deletions Modules/_functoolsmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ static PyType_Slot keyobject_type_slots[] = {
static PyType_Spec keyobject_type_spec = {
.name = "functools.KeyWrapper",
.basicsize = sizeof(keyobject),
.flags = Py_TPFLAGS_DEFAULT,
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots = keyobject_type_slots
};

Expand Down Expand Up @@ -766,7 +766,7 @@ static PyType_Slot lru_list_elem_type_slots[] = {
static PyType_Spec lru_list_elem_type_spec = {
.name = "functools._lru_list_elem",
.basicsize = sizeof(lru_list_elem),
.flags = Py_TPFLAGS_DEFAULT,
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots = lru_list_elem_type_slots
};

Expand Down
2 changes: 1 addition & 1 deletion Modules/_gdbmmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ static PyType_Spec gdbmtype_spec = {
// dbmtype_spec does not have Py_TPFLAGS_BASETYPE flag
// which prevents to create a subclass.
// So calling PyType_GetModuleState() in this file is always safe.
.flags = Py_TPFLAGS_DEFAULT,
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots = gdbmtype_spec_slots,
};

Expand Down
9 changes: 6 additions & 3 deletions Modules/_sre.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2690,7 +2690,8 @@ static PyType_Spec pattern_spec = {
.name = "re.Pattern",
.basicsize = sizeof(PatternObject),
.itemsize = sizeof(SRE_CODE),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE,
.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE |
Py_TPFLAGS_DISALLOW_INSTANTIATION),
.slots = pattern_slots,
};

Expand Down Expand Up @@ -2755,7 +2756,8 @@ static PyType_Spec match_spec = {
.name = "re.Match",
.basicsize = sizeof(MatchObject),
.itemsize = sizeof(Py_ssize_t),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE,
.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE |
Py_TPFLAGS_DISALLOW_INSTANTIATION),
.slots = match_slots,
};

Expand All @@ -2781,7 +2783,8 @@ static PyType_Slot scanner_slots[] = {
static PyType_Spec scanner_spec = {
.name = "_" SRE_MODULE ".SRE_Scanner",
.basicsize = sizeof(ScannerObject),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE,
.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE |
Py_TPFLAGS_DISALLOW_INSTANTIATION),
.slots = scanner_slots,
};

Expand Down
5 changes: 3 additions & 2 deletions Modules/_threadmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,8 @@ static PyType_Slot lock_type_slots[] = {
static PyType_Spec lock_type_spec = {
.name = "_thread.lock",
.basicsize = sizeof(lockobject),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
Py_TPFLAGS_DISALLOW_INSTANTIATION),
.slots = lock_type_slots,
};

Expand Down Expand Up @@ -683,7 +684,7 @@ static PyType_Slot local_dummy_type_slots[] = {
static PyType_Spec local_dummy_type_spec = {
.name = "_thread._localdummy",
.basicsize = sizeof(localdummyobject),
.flags = Py_TPFLAGS_DEFAULT,
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots = local_dummy_type_slots,
};

Expand Down
2 changes: 1 addition & 1 deletion Modules/_winapi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ static PyType_Slot winapi_overlapped_type_slots[] = {
static PyType_Spec winapi_overlapped_type_spec = {
.name = "_winapi.Overlapped",
.basicsize = sizeof(OverlappedObject),
.flags = Py_TPFLAGS_DEFAULT,
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots = winapi_overlapped_type_slots,
};

Expand Down
3 changes: 2 additions & 1 deletion Modules/arraymodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2987,7 +2987,8 @@ static PyType_Slot arrayiter_slots[] = {
static PyType_Spec arrayiter_spec = {
.name = "array.arrayiterator",
.basicsize = sizeof(arrayiterobject),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
Py_TPFLAGS_DISALLOW_INSTANTIATION),
.slots = arrayiter_slots,
};

Expand Down
3 changes: 2 additions & 1 deletion Modules/cjkcodecs/multibytecodec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,8 @@ static PyType_Slot multibytecodec_slots[] = {
static PyType_Spec multibytecodec_spec = {
.name = MODULE_NAME ".MultibyteCodec",
.basicsize = sizeof(MultibyteCodecObject),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
Py_TPFLAGS_DISALLOW_INSTANTIATION),
.slots = multibytecodec_slots,
};

Expand Down
3 changes: 2 additions & 1 deletion Modules/pyexpat.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1506,7 +1506,8 @@ static PyType_Slot _xml_parse_type_spec_slots[] = {
static PyType_Spec _xml_parse_type_spec = {
.name = "pyexpat.xmlparser",
.basicsize = sizeof(xmlparseobject),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
Py_TPFLAGS_DISALLOW_INSTANTIATION),
.slots = _xml_parse_type_spec_slots,
};

Expand Down
2 changes: 1 addition & 1 deletion Modules/unicodedata.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1454,7 +1454,7 @@ static PyType_Slot ucd_type_slots[] = {
static PyType_Spec ucd_type_spec = {
.name = "unicodedata.UCD",
.basicsize = sizeof(PreviousDBVersion),
.flags = Py_TPFLAGS_DEFAULT,
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots = ucd_type_slots
};

Expand Down
18 changes: 8 additions & 10 deletions Modules/zlibmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1386,11 +1386,10 @@ static PyType_Slot Comptype_slots[] = {
};

static PyType_Spec Comptype_spec = {
"zlib.Compress",
sizeof(compobject),
0,
Py_TPFLAGS_DEFAULT,
Comptype_slots
.name = "zlib.Compress",
.basicsize = sizeof(compobject),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots= Comptype_slots,
};

static PyType_Slot Decomptype_slots[] = {
Expand All @@ -1401,11 +1400,10 @@ static PyType_Slot Decomptype_slots[] = {
};

static PyType_Spec Decomptype_spec = {
"zlib.Decompress",
sizeof(compobject),
0,
Py_TPFLAGS_DEFAULT,
Decomptype_slots
.name = "zlib.Decompress",
.basicsize = sizeof(compobject),
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION,
.slots = Decomptype_slots,
};

PyDoc_STRVAR(zlib_module_documentation,
Expand Down