C module doesn't handle exceptions properly

[markopy]

The C module implementing ZstdDecompressionWriter does not handle exceptions raised by the write function passed to stream_writer(). Consider the following code:

import zstandard

class Writer:
    def write(self, data):
        raise Exception("breaks things")

decompressor = zstandard.ZstdDecompressor().stream_writer(Writer())
decompressor.write(open('test.zst', 'rb').read())

which results in the error:

Traceback (most recent call last):
  File "zstandard_fail.py", line 5, in write
    raise Exception("break things")
Exception: break things

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "zstandard_fail.py", line 8, in <module>
    decompressor.write(open('test.zst', 'rb').read())
SystemError: <method 'write' of 'zstd.ZstdDecompressionWriter' objects> returned a result with an error set

The reason for the SystemError is explained in [1]. The Writer.write() function raised an exception which was ignore by ZstdDecompressionWriter.write() and the later eventually returned with a value indicating success while the earlier exception was not cleared (which it shouldn't). The inconsistency between the success return value and the pending exception causes python to raise the SystemError.

I believe the broken code in this case is in https://github.com/indygreg/python-zstandard/blob/master/c-ext/decompressionwriter.c

#if PY_MAJOR_VERSION >= 3
			res = PyObject_CallMethod(self->writer, "write", "y#",
#else
			res = PyObject_CallMethod(self->writer, "write", "s#",
#endif
				output.dst, output.pos);
			Py_XDECREF(res);

There is no error checking after calling PyObject_CallMethod. The python documentation [2] indicates that the proper way to handle this is with something like:

if(res == NULL || PyErr_Occurred() != NULL) {
    // cleanup resources
    return NULL;
}

This example is for ZstdDecompressionWriter but ZstdDecompressionReader and ZstdCompressionWriter/Reader likely have the same issue.

[1] https://stackoverflow.com/questions/53796264/systemerror-class-int-returned-a-result-with-an-error-set-in-python
[2] https://docs.python.org/3/c-api/exceptions.html

[indygreg]

This is a legit bug and I should have a fix in the next release. You are correct that the bug exists in multiple code paths.

Thank you for reporting it.

[markopy]

Thank you, great to have this fixed!