1fanwang opened a new pull request, #51129:
URL: https://github.com/apache/arrow/pull/51129
### Rationale for this change
Calling `pyarrow.compute.FunctionOptions.deserialize()` with anything that
is not a `Buffer` crashes the interpreter. `None`, an `int`, a `list` and a
`bytes` object all segfault, so a caller who passes the wrong object, or who
forwards an unvalidated value, loses the process rather than seeing an
exception.
`deserialize()` hands its argument to `pyarrow_unwrap_buffer()`, which
returns a null pointer for a non-`Buffer`. The `deref()` on the next line then
dereferences null.
Before this change the call terminates the process. After it, the call
raises `TypeError` naming the type it received.
### What changes are included in this PR?
The static method now declares its parameter as `Buffer buf not None`, which
is what its docstring already says it takes. Cython rejects a wrong type before
the unwrap runs, so the null pointer is never produced.
### Are these changes tested?
Yes, by a parametrized case over the four inputs above.
<details>
<summary>Red, with this commit's source file reverted to its parent</summary>
```console
$ git checkout HEAD~1 python/pyarrow/_compute.pyx
$ pip install -e python --no-build-isolation
$ python -X faulthandler -c "import pyarrow.compute as pc;
pc.FunctionOptions.deserialize(None)"
Fatal Python error: Segmentation fault
Current thread 0x00000001f6fbe180 (most recent call first):
File "<string>", line 1 in <module>
Extension modules: numpy._core._multiarray_umath,
numpy.linalg._umath_linalg, pyarrow.lib, pyarrow._compute (total: 4)
```
The same crash occurs for `1`, `[]` and `b''`.
</details>
<details>
<summary>Green, with the fix applied</summary>
```console
$ python -m pytest pyarrow/tests/test_compute.py -k
deserialize_rejects_non_buffers -v
collected 607 items / 603 deselected / 4 selected
pyarrow/tests/test_compute.py::test_function_options_deserialize_rejects_non_buffers[None]
PASSED [ 25%]
pyarrow/tests/test_compute.py::test_function_options_deserialize_rejects_non_buffers[1]
PASSED [ 50%]
pyarrow/tests/test_compute.py::test_function_options_deserialize_rejects_non_buffers[value2]
PASSED [ 75%]
pyarrow/tests/test_compute.py::test_function_options_deserialize_rejects_non_buffers[]
PASSED [100%]
====================== 4 passed, 603 deselected in 0.06s
=======================
```
```console
$ for v in None 1 "[]" "b''"; do python -c "
import pyarrow.compute as pc
try:
pc.FunctionOptions.deserialize($v)
except TypeError as e:
print('$v', '->', e)
"; done
None -> Argument 'buf' has incorrect type (expected pyarrow.lib.Buffer, got
NoneType)
1 -> Argument 'buf' has incorrect type (expected pyarrow.lib.Buffer, got int)
[] -> Argument 'buf' has incorrect type (expected pyarrow.lib.Buffer, got
list)
b -> Argument 'buf' has incorrect type (expected pyarrow.lib.Buffer, got
bytes)
```
</details>
A valid buffer still round-trips, and the rest of `test_compute.py` is
unaffected.
<details>
<summary>Round-trip and full suite</summary>
```console
$ python -c "import pyarrow.compute as pc; o =
pc.ArraySortOptions(order='descending');
print(pc.FunctionOptions.deserialize(o.serialize()) == o)"
True
$ python -m pytest pyarrow/tests/test_compute.py -q
584 passed, 23 skipped, 11 warnings in 3.09s
```
</details>
### Are there any user-facing changes?
Passing a non-`Buffer` to `FunctionOptions.deserialize()` now raises
`TypeError` instead of terminating the process. Callers already passing a
`Buffer`, which is what the docstring documents, are unaffected.
**This PR contains a fix for a segmentation fault reachable from Python.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
