PyString::to_string panics on UnicodeDecodeError

[wkschwartz]

PyString::to_string returns a PyResult<Cow<str>>, so Python's raising an exception should cause PyString::to_string to return Err(PyErr) holding a UnicodeDecodeError. However, PyString::to_string panics if a Python str holds an unpaired surrogate, e.g.,

"foo \ud800 bar"

The problem arises because PyString::to_string's implementation relies on PyString::data, whose API is fundamentally incompatible: PyString::data does not return a PyResult. Thus, when the call to PyUnicode_AsUTF8AndSize in PyString::data raises an exception, the exception is not handled:

rust-cpython/src/objects/string.rs

Lines 291 to 295 in 633dafc

	let data = ffi::PyUnicode_AsUTF8AndSize(self.as_ptr(), &mut size) as *const u8;
	if data.is_null() {
	PyErr::fetch(py).print(py);
	panic!("PyUnicode_AsUTF8AndSize failed");
	}

The fix, I believe, is to

• call PyUnicode_AsUTF8AndSize directly in PyString::to_string's implementation and handle its exception rather than relying on PyString::data
  • Implementation note: Code can be shared between PyString::to_string and PyString::data, with the latter unwrap()ing and the former returning the PyResult
  • Alternative: Implement PyO3's PyString::to_str(&self, py: Python) -> PyResult<&str>, which does not have the panic bug
• Document that PyString::data may panic if the Python str is not representable as a Rust string
• Add a test case for this (e.g., PyO3's test case)

[dgrunwald]

The PyString::data API is fine, the issue is that it never got properly implemented for Python 3 (hence the TODO in the method body).
For your test string, I'd expect data() to return the PyStringData::Utf16 variant directly without attempting a conversion to UTF-8. Then PyStringData::to_string() will correctly handle the error instead of panicking.