Skip to content

fix(python): check each parameter in native callback validation loop - #1335

Open
jdymitarai wants to merge 1 commit into
google:masterfrom
jdymitarai:fix-python-param-validation
Open

fix(python): check each parameter in native callback validation loop#1335
jdymitarai wants to merge 1 commit into
google:masterfrom
jdymitarai:fix-python-param-validation

Conversation

@jdymitarai

Copy link
Copy Markdown

Summary

In python/_jsonnet.c, handle_native_callbacks validates that each parameter specified in a native callback's parameter tuple is a string:

        /* Check the params are all strings */
        num_params = PyTuple_Size(params);
        for (i = 0; i < num_params ; ++i) {
            PyObject *param = PyTuple_GetItem(params, 0);
            if (!PyUnicode_Check(param)) {
                PyErr_SetString(PyExc_TypeError, "native callback param must be string");
                goto bad;
            }
        }

The loop was indexing params at index 0 instead of loop counter i. As a consequence:

  • If a callback had multiple parameters where parameter 0 was a string but any subsequent parameter was not (e.g. (('name', 123), cb)), the loop repeatedly validated parameter 0 and completely skipped subsequent parameters.
  • In the subsequent setup loop, get_py_utf8_string called PyUnicode_AsUTF8String on the non-string object, which set an uncaught exception on the CPython thread state and produced a NULL string pointer.
  • evaluate_snippet then returned a result with an active exception, causing CPython to raise:
    SystemError: <built-in function evaluate_snippet> returned a result with an exception set

Changes

  • Fix parameter indexing from PyTuple_GetItem(params, 0) to PyTuple_GetItem(params, i).
  • Add unit test test_native_callback_param_type_validation in python/_jsonnet_test.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant