search

amazon-ion / ion-python

A Python implementation of Amazon Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
261 stars 51 forks source link

Fix invalid memory access in ionc_write #376

Closed nirosys closed 1 month ago

nirosys commented 1 month ago

Issue #, if available: #375

Description of changes: The ionc extension had a few uninitialized pointers defined in ionc_write that would get Py_DECREF'd when the arguments provided where invalid. This was found with the feature added with #372, when an extra parameter was passed to track whether to print trialing commas or not.

This PR initializes the values to NULL and changes the Py_DECREF to Py_XDECREF because Py_DECREF doesn't handle NULL values. Unit tests were also added to ensure we're returning an exception and not blowing up in these cases.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

nirosys commented 1 month ago

Looks like PyPy 3.8 is failing due to ujson and rapidjson extension build issues.. 3.10 pypy on macos looks to be failing because of a linker option in rapidjson.

rmarrowstone commented 1 month ago

Looks like PyPy 3.8 is failing due to ujson and rapidjson extension build issues.. 3.10 pypy on macos looks to be failing because of a linker option in rapidjson.

We should defintely have checks that pass by default.

I'm not concerned about the benchmark for those formats not working on PyPy. I think we were just benchmarking against plain old json in fact, and IIRC we had disabled pypy for those? If not, or if it got lost somehow, we should.