Skip to content

Commit

Permalink
feat(zend): add helper for try catch and bailout in PHP (#275)
Browse files Browse the repository at this point in the history
* feat(zend): add helper for try catch and bailout in PHP

* feat(try): add bindings for bailout

* fix(try): add missing feature flag for test

* feat(try): add a test that expose memory leak problem

* feat(try): make bailout unsafe and explain why

* feat(bailout): flag bailout as a panic function

* feat(embed): add try catch on script / eval
  • Loading branch information
joelwurtz authored Oct 25, 2023
1 parent dddc07f commit 5fdd8fa
Show file tree
Hide file tree
Showing 11 changed files with 259 additions and 37 deletions.
3 changes: 2 additions & 1 deletion allowed_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,5 +261,6 @@ bind! {
zend_file_handle,
zend_stream_init_filename,
php_execute_script,
zend_register_module_ex
zend_register_module_ex,
_zend_bailout
}
2 changes: 2 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ fn main() -> Result<()> {
for path in [
manifest.join("src").join("wrapper.h"),
manifest.join("src").join("wrapper.c"),
manifest.join("src").join("embed").join("embed.h"),
manifest.join("src").join("embed").join("embed.c"),
manifest.join("allowed_bindings.rs"),
manifest.join("windows_build.rs"),
manifest.join("unix_build.rs"),
Expand Down
3 changes: 3 additions & 0 deletions docsrs_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,9 @@ pub struct _zend_class_entry__bindgen_ty_4__bindgen_ty_2 {
pub builtin_functions: *const _zend_function_entry,
pub module: *mut _zend_module_entry,
}
extern "C" {
pub fn _zend_bailout(filename: *const ::std::os::raw::c_char, lineno: u32) -> !;
}
extern "C" {
pub static mut zend_interrupt_function:
::std::option::Option<unsafe extern "C" fn(execute_data: *mut zend_execute_data)>;
Expand Down
2 changes: 1 addition & 1 deletion src/embed/embed.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// We actually use the PHP embed API to run PHP code in test
// At some point we might want to use our own SAPI to do that
void* ext_php_rs_embed_callback(int argc, char** argv, void* (*callback)(void *), void *ctx) {
void *result;
void *result = NULL;

PHP_EMBED_START_BLOCK(argc, argv)

Expand Down
2 changes: 1 addition & 1 deletion src/embed/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ extern "C" {
pub fn ext_php_rs_embed_callback(
argc: c_int,
argv: *mut *mut c_char,
func: unsafe extern "C" fn(*const c_void) -> *mut c_void,
func: unsafe extern "C" fn(*const c_void) -> *const c_void,
ctx: *const c_void,
) -> *mut c_void;
}
92 changes: 59 additions & 33 deletions src/embed/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ use crate::ffi::{
zend_stream_init_filename, ZEND_RESULT_CODE_SUCCESS,
};
use crate::types::{ZendObject, Zval};
use crate::zend::ExecutorGlobals;
use crate::zend::{panic_wrapper, try_catch, ExecutorGlobals};
use parking_lot::{const_rwlock, RwLock};
use std::ffi::{c_char, c_void, CString, NulError};
use std::panic::{catch_unwind, resume_unwind, RefUnwindSafe};
use std::panic::{resume_unwind, RefUnwindSafe};
use std::path::Path;
use std::ptr::null_mut;

Expand All @@ -29,6 +29,13 @@ pub enum EmbedError {
ExecuteScriptError,
InvalidEvalString(NulError),
InvalidPath,
CatchError,
}

impl EmbedError {
pub fn is_bailout(&self) -> bool {
matches!(self, EmbedError::CatchError)
}
}

static RUN_FN_LOCK: RwLock<()> = const_rwlock(());
Expand Down Expand Up @@ -79,10 +86,12 @@ impl Embed {
zend_stream_init_filename(&mut file_handle, path.as_ptr());
}

if unsafe { php_execute_script(&mut file_handle) } {
Ok(())
} else {
Err(EmbedError::ExecuteScriptError)
let exec_result = try_catch(|| unsafe { php_execute_script(&mut file_handle) });

match exec_result {
Err(_) => Err(EmbedError::CatchError),
Ok(true) => Ok(()),
Ok(false) => Err(EmbedError::ExecuteScriptError),
}
}

Expand All @@ -93,6 +102,12 @@ impl Embed {
/// Which means subsequent calls to `Embed::eval` or `Embed::run_script` will be able to access
/// variables defined in previous calls
///
/// # Returns
///
/// * R - The result of the function passed to this method
///
/// R must implement [`Default`] so it can be returned in case of a bailout
///
/// # Example
///
/// ```
Expand All @@ -105,41 +120,36 @@ impl Embed {
/// assert_eq!(foo.unwrap().string().unwrap(), "foo");
/// });
/// ```
pub fn run<F: Fn() + RefUnwindSafe>(func: F) {
pub fn run<R, F: FnMut() -> R + RefUnwindSafe>(func: F) -> R
where
R: Default,
{
// @TODO handle php thread safe
//
// This is to prevent multiple threads from running php at the same time
// At some point we should detect if php is compiled with thread safety and avoid doing that in this case
let _guard = RUN_FN_LOCK.write();

unsafe extern "C" fn wrapper<F: Fn() + RefUnwindSafe>(ctx: *const c_void) -> *mut c_void {
// we try to catch panic here so we correctly shutdown php if it happens
// mandatory when we do assert on test as other test would not run correctly
let panic = catch_unwind(|| {
(*(ctx as *const F))();
});

let panic_ptr = Box::into_raw(Box::new(panic));

panic_ptr as *mut c_void
}

let panic = unsafe {
ext_php_rs_embed_callback(
0,
null_mut(),
wrapper::<F>,
panic_wrapper::<R, F>,
&func as *const F as *const c_void,
)
};

// This can happen if there is a bailout
if panic.is_null() {
return;
return R::default();
}

if let Err(err) = unsafe { *Box::from_raw(panic as *mut std::thread::Result<()>) } {
// we resume the panic here so it can be catched correctly by the test framework
resume_unwind(err);
match unsafe { *Box::from_raw(panic as *mut std::thread::Result<R>) } {
Ok(r) => r,
Err(err) => {
// we resume the panic here so it can be catched correctly by the test framework
resume_unwind(err);
}
}
}

Expand Down Expand Up @@ -170,21 +180,18 @@ impl Embed {

let mut result = Zval::new();

// this eval is very limited as it only allow simple code, it's the same eval used by php -r
let exec_result = unsafe {
let exec_result = try_catch(|| unsafe {
zend_eval_string(
cstr.as_ptr() as *const c_char,
&mut result,
b"run\0".as_ptr() as *const _,
)
};

let exception = ExecutorGlobals::take_exception();
});

if exec_result != ZEND_RESULT_CODE_SUCCESS {
Err(EmbedError::ExecuteError(exception))
} else {
Ok(result)
match exec_result {
Err(_) => Err(EmbedError::CatchError),
Ok(ZEND_RESULT_CODE_SUCCESS) => Ok(result),
Ok(_) => Err(EmbedError::ExecuteError(ExecutorGlobals::take_exception())),
}
}
}
Expand Down Expand Up @@ -244,4 +251,23 @@ mod tests {
panic!("test panic");
});
}

#[test]
fn test_return() {
let foo = Embed::run(|| {
return "foo";
});

assert_eq!(foo, "foo");
}

#[test]
fn test_eval_bailout() {
Embed::run(|| {
let result = Embed::eval("str_repeat('a', 100_000_000_000_000);");

assert!(result.is_err());
assert!(result.unwrap_err().is_bailout());
});
}
}
6 changes: 6 additions & 0 deletions src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ extern "C" {
pub fn ext_php_rs_zend_object_alloc(obj_size: usize, ce: *mut zend_class_entry) -> *mut c_void;
pub fn ext_php_rs_zend_object_release(obj: *mut zend_object);
pub fn ext_php_rs_executor_globals() -> *mut zend_executor_globals;
pub fn ext_php_rs_zend_try_catch(
func: unsafe extern "C" fn(*const c_void) -> *const c_void,
ctx: *const c_void,
result: *mut *mut c_void,
) -> bool;
pub fn ext_php_rs_zend_bailout() -> !;
}

include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
14 changes: 14 additions & 0 deletions src/wrapper.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,17 @@ zend_executor_globals *ext_php_rs_executor_globals() {
return &executor_globals;
#endif
}

bool ext_php_rs_zend_try_catch(void* (*callback)(void *), void *ctx, void **result) {
zend_try {
*result = callback(ctx);
} zend_catch {
return true;
} zend_end_try();

return false;
}

void ext_php_rs_zend_bailout() {
zend_bailout();
}
4 changes: 3 additions & 1 deletion src/wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,6 @@ void ext_php_rs_set_known_valid_utf8(zend_string *zs);
const char *ext_php_rs_php_build_id();
void *ext_php_rs_zend_object_alloc(size_t obj_size, zend_class_entry *ce);
void ext_php_rs_zend_object_release(zend_object *obj);
zend_executor_globals *ext_php_rs_executor_globals();
zend_executor_globals *ext_php_rs_executor_globals();
bool ext_php_rs_zend_try_catch(void* (*callback)(void *), void *ctx, void **result);
void ext_php_rs_zend_bailout();
4 changes: 4 additions & 0 deletions src/zend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mod globals;
mod handlers;
mod ini_entry_def;
mod module;
mod try_catch;

use crate::{error::Result, ffi::php_printf};
use std::ffi::CString;
Expand All @@ -22,6 +23,9 @@ pub use globals::ExecutorGlobals;
pub use handlers::ZendObjectHandlers;
pub use ini_entry_def::IniEntryDef;
pub use module::ModuleEntry;
#[cfg(feature = "embed")]
pub(crate) use try_catch::panic_wrapper;
pub use try_catch::{bailout, try_catch};

// Used as the format string for `php_printf`.
const FORMAT_STR: &[u8] = b"%s\0";
Expand Down
Loading

0 comments on commit 5fdd8fa

Please sign in to comment.