Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
12 changes: 12 additions & 0 deletions Lib/test/test_interpreters/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ def test_in_main(self):
# GH-126221: Passing an invalid Unicode character used to cause a SystemError
self.assertRaises(UnicodeEncodeError, _interpreters.create, '\udc80')

# A config object with a surrogate in a string field must raise, not crash.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Make it a separate test.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in 02053a9.

class BadConfig:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a need to set all other fields?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Switched to _interpreters.new_config() in 02053a9 so only .gil is set. Also fixes the CI failure on main where BadConfig()'s instance dict was empty.

use_main_obmalloc = False
allow_fork = False
allow_exec = False
allow_threads = False
allow_daemon_threads = False
check_multi_interp_extensions = False
own_gil = True
gil = 'own\udc80'
self.assertRaises(UnicodeEncodeError, _interpreters.create, BadConfig())

def test_in_thread(self):
lock = threading.Lock()
interp = None
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix a crash in :func:`!_interpreters.create` when a config object passes a
string with an unpaired surrogate as a value (for example ``gil``). The
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be confused with gil being the "bad" word. Instead, just mention that the crash was fixed if the value had unpaires surrogates (but do not mention which keys they could be associated with)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in 02053a9.

internal helper ``_config_dict_copy_str`` now checks the return of
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do not think it is necessary to mention the internals. Just mentioning interpreter's possible failures should be enough.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in 02053a9.

:c:func:`PyUnicode_AsUTF8` before copying, turning the segfault into a
:exc:`UnicodeEncodeError`.
7 changes: 6 additions & 1 deletion Python/interpconfig.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,12 @@ _config_dict_copy_str(PyObject *dict, const char *name,
config_dict_invalid_type(name);
return -1;
}
strncpy(buf, PyUnicode_AsUTF8(item), bufsize-1);
const char *utf8 = PyUnicode_AsUTF8(item);
if (utf8 == NULL) {
Py_DECREF(item);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should maybe add a note to the just-raised exception to indicate for which key it failed. But this can be done as a follow-up.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will send as a follow-up PR.

return -1;
}
strncpy(buf, utf8, bufsize-1);
buf[bufsize-1] = '\0';
Py_DECREF(item);
return 0;
Expand Down
Loading