classification
Title: Add range check for %c in PyUnicode_FromFormat
Type: behavior Stage: resolved
Components: Interpreter Core, Unicode Versions: Python 3.4, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: ezio.melotti, haypo, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-06-10 21:00 by serhiy.storchaka, last changed 2013-06-23 18:35 by serhiy.storchaka. 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 (haypo) * (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) 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
2013-06-23 18:35:55serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg191718

stage: patch review -> resolved
2013-06-23 17:55:56python-devsetnosy: + python-dev
messages: + msg191713
2013-06-23 17:03:23serhiy.storchakasetassignee: serhiy.storchaka
messages: + msg191710
2013-06-18 22:19:57hayposetmessages: + msg191437
2013-06-10 21:08:58hayposetnosy: + haypo
2013-06-10 21:00:38serhiy.storchakasetfiles: + format_c-2.7.diff
2013-06-10 21:00:13serhiy.storchakacreate