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, orsenthil, vstinner
Priority: release blocker Keywords: needs review, patch

Created on 2008-08-20 22:58 by vstinner, last changed 2008-10-16 21:28 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
_json.patch vstinner, 2008-08-20 22:58 Fix descibed bugs
_json26.patch vstinner, 2008-09-26 17:16 raise_errmsg(): fix when errmsg() fails
json_test.patch vstinner, 2008-10-06 22:26 Write an unit test for the two crashs
Messages (12)
msg71588 - (view) Author: STINNER Victor (vstinner) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2008-09-04 02:15
Deferring until rc2
msg72614 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) 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 (vstinner) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2008-10-16 21:18
Fixed in r66932 and r66934.
msg74864 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-10-16 21:28
Refleak was fixed in r66934.
msg74865 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) 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:37vstinnersetfiles: + 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:05vstinnersetfiles: + _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:44vstinnercreate