classification
Title: _json: fix raise_errmsg(), py_encode_basestring_ascii() and linecol()
Type: crash Stage:
Components: Library (Lib) Versions: Python 3.0
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: bob.ippolito Nosy List: ajaksu2, alexandre.vassalotti, amaury.forgeotdarc, barry, benjamin.peterson, bob.ippolito, christian.heimes, haypo, orsenthil
Priority: release blocker Keywords: needs review, patch

Created on 2008-08-20 22:58 by haypo, last changed 2008-10-16 21:28 by benjamin.peterson.

Files
File name Uploaded Description Edit Remove
_json.patch haypo, 2008-08-20 22:58 Fix descibed bugs
_json26.patch haypo, 2008-09-26 17:16 raise_errmsg(): fix when errmsg() fails
json_test.patch haypo, 2008-10-06 22:26 Write an unit test for the two crashs
Messages
msg71588 (view) Author: STINNER Victor (haypo) Date: 2008-08-20 22:58
_json module of python 3.0 has some bugs.

(a) [_json.c] raise_errmsg() calls json.decoder.errmsg() from Python 
module, but this function may fails. If it fails, error should be be 
set to NULL. Example:

>>> import _json
>>> _json.scanstring(b"xxx", 1, "xxx")
Erreur de segmentation (core dumped)

=> json.decoder.errmsg() calls linecol() which raise an error on 
b"xxx".index("\n").

(b) [_json.c] py_encode_basestring_ascii() calls PyBytes_Check(rval) 
but rval can be NULL if ascii_escape_str() or ascii_escape_unicode() 
fails. Example:

>>> import _json
>>> _json.encode_basestring_ascii(b"xx\xff")
Erreur de segmentation (core dumped)

(c) [Lib/json/decoder.py] linecol() doesn't support bytes, but it's 
called with bytes argument: see point (a). For an example, see point 
(a).
msg71598 (view) Author: Bob Ippolito (bob.ippolito) Date: 2008-08-21 01:20
That patch looks ok to me, but I'm not at all familiar with the changes in 
python 3.0 yet. Is this something that needs to be backported to 2.6?
msg71599 (view) Author: Benjamin Peterson (benjamin.peterson) Date: 2008-08-21 01:22
Christian, I believe, did the original 3.0 porting, so he can probably
shed some light on this.
msg72460 (view) Author: Barry A. Warsaw (barry) Date: 2008-09-04 02:15
Deferring until rc2
msg72614 (view) Author: Senthil (orsenthil) Date: 2008-09-05 18:11
issue3763 mentions about the similar problem with json library.
The traceback posted illustrates the issue (c) mentioned here:

  File "C:\Python30\lib\json\decoder.py", line 30, in errmsg
    lineno, colno = linecol(doc, pos)
  File "C:\Python30\lib\json\decoder.py", line 21, in linecol
    lineno = doc.count('\n', 0, pos) + 1
TypeError: expected an object with the buffer interface
msg73866 (view) Author: STINNER Victor (haypo) Date: 2008-09-26 17:16
> Is this something that needs to be backported to 2.6?

Hum, here is a part of my patch which can be applied to python 2.6. I 
don't know if it fixes real bugs, but the code looks better with the 
patch: PyErr_SetObject() and Py_DECREF() should not be called with 
NULL argument.
msg73869 (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) Date: 2008-09-26 17:40
_json26.patch looks good, and is necessary, because the called python
function has more than one way to raise an exception (KeyboardInterrupt
at least...)

I would just add another minor change: this raise_errmsg() function
should be made static.
msg74100 (view) Author: Bob Ippolito (bob.ippolito) Date: 2008-09-30 21:18
I applied the changes proposed in _json26.patch to simplejson and they 
worked perfectly fine. Again I'm not the best judge of python 3.0 code 
because I have not made myself familiar with the API changes so someone 
else should review it (but it's probably fine).
msg74422 (view) Author: Alexandre Vassalotti (alexandre.vassalotti) Date: 2008-10-07 03:46
The patch looks good to me. 

You may want to fix the refleak in the PyList_Append() calls (I counted
4) too:

            if (PyList_Append(chunks, chunk)) {
                goto bail;
            }

should be:

            if (PyList_Append(chunks, chunk)) {
                Py_DECREF(chunk);
                goto bail;
            }

Also, line 384 and 548:

    Py_DECREF(chunks);
    chunks = NULL;

should changed to

    Py_CLEAR(chunks);
msg74861 (view) Author: Benjamin Peterson (benjamin.peterson) Date: 2008-10-16 21:18
Fixed in r66932 and r66934.
msg74864 (view) Author: Benjamin Peterson (benjamin.peterson) Date: 2008-10-16 21:28
Refleak was fixed in r66934.
msg74865 (view) Author: Benjamin Peterson (benjamin.peterson) Date: 2008-10-16 21:28
I mean r66938. Sorry!
History
Date User Action Args
2008-10-16 21:28:27benjamin.petersonsetmessages: + msg74865
2008-10-16 21:28:10benjamin.petersonsetmessages: + msg74864
2008-10-16 21:18:02benjamin.petersonsetstatus: open -> closed
resolution: fixed
messages: + msg74861
2008-10-07 03:46:47alexandre.vassalottisetnosy: + alexandre.vassalotti
messages: + msg74422
2008-10-06 22:26:37hayposetfiles: + json_test.patch
2008-10-02 12:56:21barrysetpriority: deferred blocker -> release blocker
2008-09-30 21:18:15bob.ippolitosetmessages: + msg74100
2008-09-30 20:29:33amaury.forgeotdarcsetkeywords: + needs review
2008-09-26 22:20:51barrysetpriority: release blocker -> deferred blocker
2008-09-26 17:40:22amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg73869
2008-09-26 17:16:05hayposetfiles: + _json26.patch
messages: + msg73866
2008-09-18 05:42:51barrysetpriority: deferred blocker -> release blocker
2008-09-05 18:11:09orsenthilsetnosy: + orsenthil
messages: + msg72614
2008-09-04 02:15:16barrysetpriority: release blocker -> deferred blocker
nosy: + barry
messages: + msg72460
2008-08-27 21:41:53ajaksu2setnosy: + ajaksu2
2008-08-23 16:03:38benjamin.petersonsetpriority: release blocker
2008-08-21 01:22:36benjamin.petersonsetnosy: + christian.heimes, benjamin.peterson
messages: + msg71599
2008-08-21 01:20:44bob.ippolitosetmessages: + msg71598
2008-08-20 23:01:30benjamin.petersonsetassignee: bob.ippolito
nosy: + bob.ippolito
2008-08-20 22:58:44haypocreate