Skip to content

bpo-25457: Allow json.encode() to handle mixed keys when sort_keys=True #8011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

bpo-25457: Allow json.encode() to handle mixed keys when sort_keys=True #8011

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8d3612f
Allow json(..., sort_keys=True) to handle mixed keys. (https://bugs.p…
jheiv Jun 29, 2018
9ffe28b
Tabs -> Spaces (woops)
jheiv Jun 29, 2018
07fb4d1
Better mirror what _json.c now does, but use a yielding generator ins…
jheiv Jun 29, 2018
d94498f
Better mirror what _json.c now does, but use a yielding generator ins…
jheiv Jun 29, 2018
23939cc
Merge branch 'master' of https://github.com/jheiv/cpython
jheiv Jun 29, 2018
e659cad
Fix (?) negative refcount
jheiv Jun 29, 2018
0cb9191
resolve merge conflicts from merge upstream/master
jheiv Mar 24, 2020
9e52b24
update failing tests to handle mixed-key-type sorting, rm now obsolet…
jheiv Mar 24, 2020
fe71ad6
rm trailing whitespace
jheiv Mar 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Tabs -> Spaces (woops)
  • Loading branch information
@jheiv
jheiv committed Jun 29, 2018
commit 9ffe28bf519213efce4209740a8d5ba3b94482c6
118 changes: 59 additions & 59 deletions Modules/_json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1562,7 +1562,7 @@ encoder_listencode_dict(PyEncoderObject *s, _PyAccu *acc,
PyObject *ident = NULL;
PyObject *it = NULL;
PyObject *items;
PyObject *coerced_items;
PyObject *coerced_items;
PyObject *item = NULL;
Py_ssize_t idx;

Expand Down Expand Up @@ -1609,63 +1609,63 @@ encoder_listencode_dict(PyEncoderObject *s, _PyAccu *acc,
if (items == NULL)
goto bail;

coerced_items = PyList_New(0);
it = PyObject_GetIter(items);
Py_DECREF(items);
if (it == NULL)
goto bail;

while ((item = PyIter_Next(it)) != NULL) {
PyObject *key, *value, *coerced_item;
if (!PyTuple_Check(item) || PyTuple_GET_SIZE(item) != 2) {
PyErr_SetString(PyExc_ValueError, "items must return 2-tuples");
goto bail;
}
key = PyTuple_GET_ITEM(item, 0);
if (PyUnicode_Check(key)) {
Py_INCREF(key);
kstr = key;
}
else if (PyFloat_Check(key)) {
kstr = encoder_encode_float(s, key);
if (kstr == NULL)
goto bail;
}
else if (key == Py_True || key == Py_False || key == Py_None) {
/* This must come before the PyLong_Check because
True and False are also 1 and 0.*/
kstr = _encoded_const(key);
if (kstr == NULL)
goto bail;
}
else if (PyLong_Check(key)) {
kstr = PyLong_Type.tp_str(key);
if (kstr == NULL) {
goto bail;
}
}
else if (s->skipkeys) {
Py_DECREF(item);
continue;
}
else {
PyErr_Format(PyExc_TypeError,
"keys must be str, int, float, bool or None, "
"not %.100s", key->ob_type->tp_name);
goto bail;
}

value = PyTuple_GET_ITEM(item, 1);
coerced_item = PyTuple_Pack(2, kstr, value);
if (coerced_item == NULL) {
goto bail;
}
/* Append instead of set because skipkeys=True may
"shrink" the number of items */
if (-1 == PyList_Append(coerced_items, coerced_item))
goto bail;
}
coerced_items = PyList_New(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to introduce a few possible refleaks for coerced_items, effectively in each goto bail location where coerced_items isn't NULL, prior to the Py_DECREF(coerced_items). IMO, the easiest solution would be adding a Py_XDECREF(coerced_items) to bail since there are multiple paths where this could potentially occur.

There should probably also be the following error condition, in case the list can't be instantiated:

Suggested change
coerced_items = PyList_New(0);
coerced_items = PyList_New(0);
if (coerced_items == NULL) {
goto bail;
}

What do you think @serhiy-storchaka? My experience with the C-API is fairly limited, so I'm not entirely certain about the above.

it = PyObject_GetIter(items);
Py_DECREF(items);
if (it == NULL)
goto bail;

while ((item = PyIter_Next(it)) != NULL) {
PyObject *key, *value, *coerced_item;
if (!PyTuple_Check(item) || PyTuple_GET_SIZE(item) != 2) {
PyErr_SetString(PyExc_ValueError, "items must return 2-tuples");
goto bail;
}
key = PyTuple_GET_ITEM(item, 0);
if (PyUnicode_Check(key)) {
Py_INCREF(key);
kstr = key;
}
else if (PyFloat_Check(key)) {
kstr = encoder_encode_float(s, key);
if (kstr == NULL)
goto bail;
}
else if (key == Py_True || key == Py_False || key == Py_None) {
/* This must come before the PyLong_Check because
True and False are also 1 and 0.*/
kstr = _encoded_const(key);
if (kstr == NULL)
goto bail;
}
else if (PyLong_Check(key)) {
kstr = PyLong_Type.tp_str(key);
if (kstr == NULL) {
goto bail;
}
}
else if (s->skipkeys) {
Py_DECREF(item);
continue;
}
else {
PyErr_Format(PyExc_TypeError,
"keys must be str, int, float, bool or None, "
"not %.100s", key->ob_type->tp_name);
goto bail;
}

value = PyTuple_GET_ITEM(item, 1);
coerced_item = PyTuple_Pack(2, kstr, value);
if (coerced_item == NULL) {
goto bail;
}
/* Append instead of set because skipkeys=True may
"shrink" the number of items */
if (-1 == PyList_Append(coerced_items, coerced_item))
goto bail;
}
if (s->sort_keys && PyList_Sort(coerced_items) < 0) {
Py_DECREF(coerced_items);
goto bail;
Expand Down Expand Up @@ -1730,7 +1730,7 @@ encoder_listencode_dict(PyEncoderObject *s, _PyAccu *acc,
Py_XDECREF(item);
Py_XDECREF(kstr);
Py_XDECREF(ident);
Py_XDECREF(coerced_items);
Py_XDECREF(coerced_items);
return -1;
}

Expand Down