feat(zend): add helper for try catch and bailout in PHP (#275)

* 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

Author: joelwurtz

allowed_bindings.rs

@@ -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
 }

build.rs

@@ -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"),

docsrs_bindings.rs

@@ -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)>;

src/embed/embed.c

@@ -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)
 

src/embed/ffi.rs

@@ -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;
 }

src/embed/mod.rs

@@ -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;
 
@@ -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(());
@@ -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),
         }
     }
 
@@ -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
     ///
     /// ```
@@ -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);
+            }
         }
     }
 
@@ -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())),
         }
     }
 }
@@ -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());
+        });
+    }
 }

src/ffi.rs

@@ -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"));

src/wrapper.c

@@ -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();
+}

src/wrapper.h

@@ -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();

src/zend/mod.rs

@@ -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;
@@ -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";