Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 91 additions & 2 deletions crates/capi/src/object.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::PyObject;
use crate::pystate::with_vm;
use core::ffi::{c_int, c_uint, c_ulong};
use rustpython_vm::builtins::PyType;
use core::ffi::{CStr, c_char, c_int, c_uint, c_ulong};
use core::ptr::NonNull;
use rustpython_vm::builtins::{PyStr, PyType};
use rustpython_vm::{AsObject, Py};

pub type PyTypeObject = Py<PyType>;
Expand Down Expand Up @@ -46,3 +47,91 @@ pub extern "C" fn Py_GetConstantBorrowed(constant_id: c_uint) -> *mut PyObject {
Ok(constant)
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyObject_GetAttr(
obj: *mut PyObject,
name: *mut PyObject,
) -> *mut PyObject {
with_vm(|vm| {
let obj = unsafe { &*obj };
let name = unsafe { &*name }.try_downcast_ref::<PyStr>(vm)?;
obj.get_attr(name, vm)
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyObject_GetAttrString(
obj: *mut PyObject,
attr_name: *const c_char,
) -> *mut PyObject {
with_vm(|vm| {
let obj = unsafe { &*obj };
let name = unsafe {
CStr::from_ptr(attr_name)
.to_str()
.expect("attribute name must be valid UTF-8")
};
Comment on lines +70 to +74

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace .expect() with proper error handling to avoid panics.

The .expect() call will panic if attr_name contains invalid UTF-8, crashing the entire process. In C APIs, panics cannot be caught by C callers. Instead, convert UTF-8 errors to Python exceptions.

🛡️ Proposed fix to handle UTF-8 decode errors gracefully
         let obj = unsafe { &*obj };
-        let name = unsafe {
-            CStr::from_ptr(attr_name)
-                .to_str()
-                .expect("attribute name must be valid UTF-8")
-        };
+        let name = unsafe { CStr::from_ptr(attr_name) }
+            .to_str()
+            .map_err(|_| vm.new_unicode_decode_error("attribute name must be valid UTF-8".to_owned()))?;
         obj.get_attr(name, vm)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/capi/src/object.rs` around lines 70 - 74, The code currently calls
CStr::from_ptr(attr_name).to_str().expect(...), which will panic on invalid
UTF-8; replace the expect with explicit error handling by matching the Result
from to_str(): on Ok(s) assign s to name, on Err(e) set a Python exception (e.g.
PyErr_SetString(PyExc_ValueError, format!("invalid UTF-8 in attribute name: {}",
e).as_ptr()) or using pyo3::exceptions::PyValueError::new_err(...)) and return
the function's error sentinel (NULL for pointer returns or -1 for int returns).
Update the block around CStr::from_ptr(attr_name) / to_str() to use match or
map_err so attr_name decoding never panics and the function returns a
Python-level error instead; reference the variables/functions attr_name, name,
CStr::from_ptr and to_str when making the change.

obj.get_attr(name, vm)
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyObject_SetAttrString(
obj: *mut PyObject,
attr_name: *const c_char,
value: *mut PyObject,
) -> c_int {
with_vm(|vm| {
let obj = unsafe { &*obj };
let name = unsafe { CStr::from_ptr(attr_name) }
.to_str()
.expect("attribute name must be valid UTF-8");
Comment on lines +87 to +89

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace .expect() with proper error handling to avoid panics.

Same issue as in PyObject_GetAttrString: the .expect() call will panic on invalid UTF-8, crashing the process. Convert UTF-8 errors to Python exceptions instead.

🛡️ Proposed fix to handle UTF-8 decode errors gracefully
         let obj = unsafe { &*obj };
-        let name = unsafe { CStr::from_ptr(attr_name) }
-            .to_str()
-            .expect("attribute name must be valid UTF-8");
+        let name = unsafe { CStr::from_ptr(attr_name) }
+            .to_str()
+            .map_err(|_| vm.new_unicode_decode_error("attribute name must be valid UTF-8".to_owned()))?;
         let value = unsafe { &*value }.to_owned();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/capi/src/object.rs` around lines 87 - 89, The code currently uses
CStr::from_ptr(attr_name).to_str().expect(...) which will panic on invalid
UTF-8; instead decode the C string and handle errors by converting the UTF-8
error into a Python exception and returning the appropriate error indicator
(e.g., null pointer or error code) from the surrounding FFI function. Replace
the .expect() on to_str() with a match or map_err that calls the Python C-API to
set an exception (e.g., PyErr_SetString(PyExc_UnicodeDecodeError, ... ) or the
project's helper for raising Python errors), include the invalid data/error
message, and then return early from the function using the function's error
return convention; update uses of the local variable name accordingly after
successful decoding.

let value = unsafe { &*value }.to_owned();
obj.set_attr(name, value, vm)
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyObject_SetAttr(
obj: *mut PyObject,
name: *mut PyObject,
value: *mut PyObject,
) -> c_int {
with_vm(|vm| {
let obj = unsafe { &*obj };
let name = unsafe { &*name }.try_downcast_ref::<PyStr>(vm)?;
let value = unsafe { &*value }.to_owned();
obj.set_attr(name, value, vm)
})
}

#[unsafe(no_mangle)]
pub extern "C" fn PyObject_Repr(obj: *mut PyObject) -> *mut PyObject {
with_vm(|vm| {
let Some(obj) = NonNull::new(obj) else {
return Ok(vm.ctx.new_str("<NULL>"));
};

unsafe { obj.as_ref() }.repr(vm)
})
}

#[unsafe(no_mangle)]
pub extern "C" fn PyObject_Str(obj: *mut PyObject) -> *mut PyObject {
with_vm(|vm| {
let Some(obj) = NonNull::new(obj) else {
return Ok(vm.ctx.new_str("<NULL>"));
};

unsafe { obj.as_ref() }.str(vm)
})
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn PyObject_IsTrue(obj: *mut PyObject) -> c_int {
with_vm(|vm| {
let obj = unsafe { &*obj };
obj.to_owned().is_true(vm)
})
}
Loading