Issue 18184: Add range check for %c in PyUnicode_FromFormat (original) (raw)

Created on 2013-06-10 21:00 by serhiy.storchaka, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
format_c.diff serhiy.storchaka,2013-06-10 21:00 Patch for 3.3+ review
format_c-2.7.diff serhiy.storchaka,2013-06-10 21:00 Patch for 2.7 review
Messages (5)
msg190935 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-06-10 21:00
Currently PyUnicode_FromFormat doesn't check an argument for %c and can raise SystemError only due maxchar check in PyUnicode_New. On 2.7 an error doesn't raised, but %c argument can be silently wrapped (on narrow build) or illegal Unicode string can be created (on wide build). The proposed patch adds explicit range check for %c argument.
msg191437 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-06-18 22:19
Both patches look good to me. Do you feel motivated to check if all formating methods have a test? Here is a list of format methods: PyUnicode_Format(): - 3.3, 3.4: formatchar() raises OverflowError if x > MAX_UNICODE - 2.7: formatchar() raises OverflowError if x > 0x10ffff (or x > 0xffff, in narrow mode) PyUnicode_FromFromatV(): - 2.7: no check, *s++ = va_arg(vargs, int); => BUG - 3.3: indirect check, maxchar = Py_MAX(maxchar, ordinal); and then PyUnicode_New(n, maxchar); should fail => (ok) - 3.4: raise ValueError if ordinal > MAX_UNICODE => OK int.__format__('c'): - 3.3, 3.4: format_long_internal() raises OverflowError if x > 0x10ffff => OK - 2.7: format_int_or_long_internal() raises OverflowError if x > 0x10ffff (or x > 0xffff in narrow mode) => OK IMO a ValueError would be better than OverflowError, it's not really an overflow (limitation of the C language, or a C type). It is maybe too late to change this.
msg191710 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-06-23 17:03
No, I don't feel motivated right now (and this is a case when even never is better than right now). I checked other format methods and found a bug only in PyUnicode_FromFormatV(). I had added test for 3.3+ because 3.3+ already have a test for PyUnicode_FromFormatV(). I hadn't add test for 2.7 because there is no test for PyUnicode_FromFormatV() on 2.7.
msg191713 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-06-23 17:55
New changeset f8ede55cf92b by Serhiy Storchaka in branch '3.3': Issue #18184: PyUnicode_FromFormat() and PyUnicode_FromFormatV() now raise http://hg.python.org/cpython/rev/f8ede55cf92b New changeset 42def600210e by Serhiy Storchaka in branch 'default': Issue #18184: PyUnicode_FromFormat() and PyUnicode_FromFormatV() now raise http://hg.python.org/cpython/rev/42def600210e New changeset 2f1e8b7fa534 by Serhiy Storchaka in branch '2.7': Issue #18184: PyUnicode_FromFormat() and PyUnicode_FromFormatV() now raise http://hg.python.org/cpython/rev/2f1e8b7fa534
msg191718 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-06-23 18:35
I agree, a ValueError may be better than OverflowError, but all other formattings raise OverflowError, and we should support them consistent.
History
Date User Action Args
2022-04-11 14:57:46 admin set github: 62384
2013-06-23 18:35:55 serhiy.storchaka set status: open -> closedresolution: fixedmessages: + stage: patch review -> resolved
2013-06-23 17:55:56 python-dev set nosy: + python-devmessages: +
2013-06-23 17:03:23 serhiy.storchaka set assignee: serhiy.storchakamessages: +
2013-06-18 22:19:57 vstinner set messages: +
2013-06-10 21:08:58 vstinner set nosy: + vstinner
2013-06-10 21:00:38 serhiy.storchaka set files: + format_c-2.7.diff
2013-06-10 21:00:13 serhiy.storchaka create