[issue6986] _json crash on scanner/encoder initialization error

Serhiy Storchaka added the comment: New changeset 7c2d97827fd2ccd72ab7a98427f87fcfe3891644 by Serhiy Storchaka (Oren Milman) in branch 'master': bpo-6986: Add a comment to clarify a test of _json.make_encoder(). (GH-3789) https://github.com/python/cpython/commit/7c2d97827fd2ccd72ab7a98427f87fc

[issue6986] _json crash on scanner/encoder initialization error

Change by Oren Milman : -- pull_requests: +3775 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.py

[issue6986] _json crash on scanner/encoder initialization error

Serhiy Storchaka added the comment: It would be nice. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https

[issue6986] _json crash on scanner/encoder initialization error

Oren Milman added the comment: I would be happy to open such a PR, if you don't mind. -- nosy: +Oren Milman ___ Python tracker ___ __

[issue6986] _json crash on scanner/encoder initialization error

STINNER Victor added the comment: Oh wow, this issue was fixed 8 years ago and it's now closed! Please open a write issue, or maybe even propose directly a PR? (If you do, I will review it.) -- ___ Python tracker

[issue6986] _json crash on scanner/encoder initialization error

Serhiy Storchaka added the comment: Could you please add a comment for test_make_encoder Victor? Invalid arguments are passed to c_make_encoder, but it raises a TypeError not because wrong types of argument, but because the number of arguments is insufficient. At first glance it looks as an e

[issue6986] _json crash on scanner/encoder initialization error

Antoine Pitrou added the comment: This has been committed in r76708, r76710, r76711. Thank you! -- resolution: accepted -> fixed stage: commit review -> committed/rejected status: open -> closed ___ Python tracker

[issue6986] _json crash on scanner/encoder initialization error

Antoine Pitrou added the comment: The patch looks good to me. -- priority: -> high resolution: -> accepted stage: -> commit review type: -> crash versions: +Python 2.7, Python 3.1, Python 3.2 ___ Python tracker

[issue6986] _json crash on scanner/encoder initialization error

STINNER Victor added the comment: I wrote new patches to: - implement the "alternate method" - add unit tests - and finally to move the tests in the right file Is the patch now correct? Can you apply it? It doesn't still crash Python trunk and it generates a lot of noise in my fuzzer :-) --

[issue6986] _json crash on scanner/encoder initialization error

STINNER Victor added the comment: Oops, you forget json-crash-2.patch, the patch was completly wrong. Check the json-crash-3.patch. -- Added file: http://bugs.python.org/file15292/json-crash-3.patch ___ Python tracker

[issue6986] _json crash on scanner/encoder initialization error

Changes by STINNER Victor : Removed file: http://bugs.python.org/file15291/json-crash-2.patch ___ Python tracker ___ ___ Python-bugs-list maili

[issue6986] _json crash on scanner/encoder initialization error

STINNER Victor added the comment: New version of my patch: * don't create file in Lib/json/tests/: add new tests in Lib/json/tests/test_speedups.py as asked by pitrou * use json.scanner.c_make_scanner and json.encoder.c_make_encoder in the test, instead of using directly the _json module

[issue6986] _json crash on scanner/encoder initialization error

Antoine Pitrou added the comment: The tests could go in Lib/json/tests/test_speedups.py. -- ___ Python tracker ___ ___ Python-bugs-lis

[issue6986] _json crash on scanner/encoder initialization error

Antoine Pitrou added the comment: Ah, my bad, I thought it created the file in Lib/test. The tests probably need to be skipped if _json can't be imported, though. (the _json accelerators are just an implementation detail) -- ___ Python tracker

[issue6986] _json crash on scanner/encoder initialization error

STINNER Victor added the comment: pitrou> There is actually a Lib/json/tests/ directory contain all pitrou> JSON tests, you should add yours there. My patch creates the file Lib/json/tests/test__json.py. Do you mean that I should add the new tests to an existing file? Which one? -- _

[issue6986] _json crash on scanner/encoder initialization error

Antoine Pitrou added the comment: There is actually a Lib/json/tests/ directory contain all JSON tests, you should add yours there. -- ___ Python tracker ___

[issue6986] _json crash on scanner/encoder initialization error

STINNER Victor added the comment: pitrou> Unit tests are definitely desireable! done pitou> I would also like the "alternate approach" fix, pitou> it would probably be cleaner. done. See the new patch. -- Added file: http://bugs.python.org/file15190/json-crash.patch ___

[issue6986] _json crash on scanner/encoder initialization error

Antoine Pitrou added the comment: Unit tests are definitely desireable! I would also like the "alternate approach" fix, it would probably be cleaner. -- nosy: +pitrou ___ Python tracker

[issue6986] _json crash on scanner/encoder initialization error

Changes by Benjamin Peterson : -- assignee: -> bob.ippolito nosy: +bob.ippolito ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue6986] _json crash on scanner/encoder initialization error

STINNER Victor added the comment: About _json_encoder_init.patch, an alternative patch is to write arguments references (addresses) in local variables, and only copy them on success. Something like: PyObject *arg; if (!PyTuple_ParseArgs(..., &arg)) return NULL; Py_INCREF(arg); self->arg

[issue6986] _json crash on scanner/encoder initialization error

Changes by STINNER Victor : Added file: http://bugs.python.org/file14965/_json_scanner_init.patch ___ Python tracker ___ ___ Python-bugs-list m

[issue6986] _json crash on scanner/encoder initialization error

New submission from STINNER Victor : scanner_init() and encoder_init() don't manage errors correctly. scanner_init() gets context.encoding argument without checking context type, nor GetAttrString() error. It should check for NULL result... which is done in the same function for other attributes