From 79d72c115820efcff6432337a29e7688a9e810dc Mon Sep 17 00:00:00 2001 From: Rafael Caricio Date: Sun, 5 Jan 2020 02:11:55 +0100 Subject: [PATCH] Make use of drop, avoid clone, avoid null pointers --- Cargo.toml | 1 + README.md | 3 + src/lib.rs | 553 +++++++++++++++++++++++++++++++---------------------- 3 files changed, 328 insertions(+), 229 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index daa18cc..5581fb7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,3 +11,4 @@ description = "Safe(er) rust wrapper for dukbind." [dependencies] dukbind = { path = "../dukbind" } +anyhow = "1.0.26" diff --git a/README.md b/README.md index d21918f..64c2862 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,9 @@ ## Work In Progress This library is a work in progress and is currently limited in features. + - [ ] Remove `as_number`/`as_str`/`as_*` in general and use `From` trait instead. + - [ ] Revisit error handling and maybe change to use `anyhow`. + ## What can it do? At the moment, duktape-rs diff --git a/src/lib.rs b/src/lib.rs index 630b82f..df5f1c5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,10 +1,21 @@ -extern crate dukbind; - -use dukbind::*; +use anyhow; +use dukbind::{ + duk_bool_t, duk_context, duk_create_heap_default, duk_del_prop, duk_destroy_heap, duk_dup, + duk_eval_string, duk_get_boolean, duk_get_error_code, duk_get_heapptr, duk_get_number, + duk_get_prop_lstring, duk_get_string, duk_get_type, duk_int_t, duk_is_undefined, + duk_json_decode, duk_json_encode, duk_pop, duk_pop_2, duk_push_boolean, duk_push_heap_stash, + duk_push_heapptr, duk_push_lstring, duk_push_nan, duk_push_null, duk_push_number, + duk_push_pointer, duk_push_undefined, duk_put_prop, duk_put_prop_lstring, duk_size_t, + DUK_ERR_ERROR, DUK_ERR_EVAL_ERROR, DUK_ERR_NONE, DUK_ERR_RANGE_ERROR, DUK_ERR_SYNTAX_ERROR, + DUK_ERR_TYPE_ERROR, DUK_ERR_URI_ERROR, DUK_TYPE_BOOLEAN, DUK_TYPE_NONE, DUK_TYPE_NULL, + DUK_TYPE_NUMBER, DUK_TYPE_STRING, DUK_TYPE_UNDEFINED, +}; use std::error::Error; -use std::fmt; use std::f64; +use std::ffi::CStr; +use std::fmt; use std::os::raw::c_void; +use std::ptr::NonNull; /// An error code representing why an error occurred. #[allow(missing_docs)] @@ -18,220 +29,245 @@ pub enum DukErrorCode { Syntax = DUK_ERR_SYNTAX_ERROR, Type = DUK_ERR_TYPE_ERROR, URI = DUK_ERR_URI_ERROR, - NullPtr + NullPtr, } /// Represents a JavaScript number value. JavaScript numbers can be either floats or integers, as well as NaN and Infinity. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub enum DukNumber { NaN, Infinity, Float(f64), - Int(i64) + Int(i64), } impl DukNumber { pub fn as_str(&self) -> String { - match self.clone() { + match self { DukNumber::NaN => "NaN".to_string(), DukNumber::Infinity => "Infinity".to_string(), DukNumber::Float(v) => v.clone().to_string(), - DukNumber::Int(v) => v.clone().to_string() + DukNumber::Int(v) => v.clone().to_string(), } } + pub fn is_f64(&self) -> bool { match self { DukNumber::Int(_v) => false, - _ => true + _ => true, } } + pub fn is_i64(&self) -> bool { self.is_f64() == false } + pub fn is_nan(&self) -> bool { match self { DukNumber::NaN => true, - _ => false + _ => false, } } + pub fn is_infinity(&self) -> bool { match self { DukNumber::Infinity => true, - _ => false + _ => false, } } + pub fn as_f64(&self) -> f64 { match self { DukNumber::NaN => f64::NAN, DukNumber::Infinity => f64::INFINITY, DukNumber::Float(v) => *v, - DukNumber::Int(v) => *v as f64 + DukNumber::Int(v) => *v as f64, } } + pub fn as_i64(&self) -> i64 { match self { DukNumber::NaN => f64::NAN as i64, DukNumber::Infinity => f64::INFINITY as i64, DukNumber::Float(v) => *v as i64, - DukNumber::Int(v) => *v + DukNumber::Int(v) => *v, } } } /// A wrapper around duktape's heapptr. These represent JavaScript objects. -#[derive(Clone, Debug)] -pub struct DukObject { - context: DukContext, - heap: *mut c_void +#[derive(Debug)] +pub struct DukObject<'a> { + context: &'a DukContext, + heap: NonNull, } -impl DukObject { - /// Encode this object to a JSON string. - pub fn encode(&mut self) -> Option { - unsafe { - match self.context.ctx { - Some(ctx) => { - let idx = duk_push_heapptr(ctx, self.heap); - if duk_is_undefined(ctx, idx) == 0 { - duk_dup(ctx, idx); - let raw = duk_json_encode(ctx, -1); - use std::ffi::CStr; - let t = CStr::from_ptr(raw); - let cow = t.to_string_lossy(); - duk_pop_2(ctx); - Some(String::from(cow)) - } else { - duk_pop(ctx); - None - } - }, - None => None - } - } - } +impl<'a> Drop for DukObject<'a> { /// Deletes the object from the heap stash and nullifies the internal heap pointer value. /// The object value is useless after calling this and should no longer be used. - pub fn free(&mut self) { + fn drop(&mut self) { + let ctx = self.context.ctx.as_ptr(); unsafe { - use std::ptr::null_mut; - match self.context.ctx { - Some(ctx) => { - duk_push_heap_stash(ctx); - duk_push_pointer(ctx, self.heap); - duk_del_prop(ctx, -2); - duk_pop(ctx); - self.heap = null_mut(); - }, - None => () - } - } - } - /// Get a property on this object as a DukValue. - pub fn get_prop(&mut self, name: &str) -> DukResult { - unsafe { - let ctx = self.context.ctx.expect("Invalid context pointer."); - let idx = duk_push_heapptr(ctx, self.heap); - if duk_get_prop_lstring(ctx, idx, name.as_ptr() as *const i8, name.len() as duk_size_t) == 1 { - let result = self.context.get_value(); - duk_pop(ctx); - Ok(result) - } else { - Err(DukError{ code: DukErrorCode::Error, message: Some("Could not get property.".to_string())}) - } - } - } - /// Set a property on this object. - pub fn set_prop(&mut self, name: &str, value: DukValue) -> DukResult<()> { - match self.context.ctx { - Some(ctx) => { - unsafe { - duk_push_heapptr(ctx, self.heap); - if duk_is_undefined(ctx, -1) == 0 { - let mut ok: bool = true; - match value { - DukValue::Undefined => duk_push_undefined(ctx), - DukValue::Null => duk_push_null(ctx), - DukValue::Number(ref n) => { - if n.is_nan() { - duk_push_nan(ctx); - } else if n.is_infinity() { - duk_push_lstring(ctx, "Infinity".as_ptr() as *const i8, "Infinity".len() as duk_size_t); - } else { - duk_push_number(ctx, n.as_f64()); - } - }, - DukValue::Boolean(b) => duk_push_boolean(ctx, value.as_duk_bool().expect("Not a boolean!")), - DukValue::String(s) => { - let t = &s; - duk_push_lstring(ctx, t.as_ptr() as *const i8, t.len() as duk_size_t); - }, - DukValue::Object(ref o) => { - duk_push_heapptr(ctx, o.heap); - if duk_is_undefined(ctx, -1) == 1 { - duk_pop(ctx); - ok = false; - } - } - }; - if ok { - if duk_put_prop_lstring(ctx, -2, name.as_ptr() as *const i8, name.len() as duk_size_t) == 1 { - duk_pop(ctx); - Ok(()) - } else { - duk_pop(ctx); - Err(DukError::from(DukErrorCode::Error, "Failed to set prop.")) - } - } else { - duk_pop(ctx); - Err(DukError::from(DukErrorCode::Error, "Error setting prop.")) - } - } else { - duk_pop(ctx); - Err(DukError::from(DukErrorCode::NullPtr, "Invalid heap pointer.")) - } - } - }, - None => Err(DukError::from(DukErrorCode::NullPtr, "Invalid context pointer.")) + duk_push_heap_stash(ctx); + duk_push_pointer(ctx, self.heap.as_ptr()); + duk_del_prop(ctx, -2); + duk_pop(ctx); } } +} + +impl<'a> DukObject<'a> { /// Creates a new DukObject from the object at the top of the value stack. - pub fn new(context: DukContext) -> DukObject { - unsafe { - let ctx = context.ctx.expect("Invalid context pointer."); + pub fn new(context: &'a DukContext) -> Self { + let ctx = context.ctx.as_ptr(); + let heap = unsafe { let ptr = duk_get_heapptr(ctx, -1); duk_push_heap_stash(ctx); duk_push_pointer(ctx, ptr); duk_dup(ctx, -3); duk_put_prop(ctx, -3); duk_pop(ctx); - DukObject { heap: ptr, context: context } + NonNull::new_unchecked(ptr) + }; + + Self { heap, context } + } + + /// Encode this object to a JSON string. + pub fn encode(&self) -> Option { + let ctx = self.context.ctx.as_ptr(); + unsafe { + let idx = duk_push_heapptr(ctx, self.heap.as_ptr()); + if duk_is_undefined(ctx, idx) == 0 { + duk_dup(ctx, idx); + let raw = duk_json_encode(ctx, -1); + let t = CStr::from_ptr(raw); + let cow = t.to_string_lossy(); + duk_pop_2(ctx); + Some(String::from(cow)) + } else { + duk_pop(ctx); + None + } + } + } + + /// Get a property on this object as a DukValue. + pub fn get_prop(&self, name: &str) -> DukResult { + let ctx = self.context.ctx.as_ptr(); + unsafe { + let idx = duk_push_heapptr(ctx, self.heap.as_ptr()); + if duk_get_prop_lstring( + ctx, + idx, + name.as_ptr() as *const i8, + name.len() as duk_size_t, + ) == 1 + { + let result = self.context.get_value(); + duk_pop(ctx); + Ok(result) + } else { + Err(DukError { + code: DukErrorCode::Error, + message: Some(String::from("Could not get property.")), + }) + } + } + } + + /// Set a property on this object. + pub fn set_prop(&self, name: &str, value: DukValue) -> DukResult<()> { + let ctx = self.context.ctx.as_ptr(); + unsafe { + duk_push_heapptr(ctx, self.heap.as_ptr()); + if duk_is_undefined(ctx, -1) == 0 { + let mut ok = true; + match value { + DukValue::Undefined => duk_push_undefined(ctx), + DukValue::Null => duk_push_null(ctx), + DukValue::Number(ref n) => { + if n.is_nan() { + duk_push_nan(ctx); + } else if n.is_infinity() { + let inf = "Infinity"; + duk_push_lstring( + ctx, + inf.as_ptr() as *const i8, + inf.len() as duk_size_t, + ); + } else { + duk_push_number(ctx, n.as_f64()); + } + } + DukValue::Boolean(b) => { + duk_push_boolean(ctx, value.as_duk_bool().expect("Not a boolean!")) + } + DukValue::String(s) => { + let t = &s; + duk_push_lstring(ctx, t.as_ptr() as *const i8, t.len() as duk_size_t); + } + DukValue::Object(ref o) => { + duk_push_heapptr(ctx, o.heap.as_ptr()); + if duk_is_undefined(ctx, -1) == 1 { + duk_pop(ctx); + ok = false; + } + } + }; + if ok { + if duk_put_prop_lstring( + ctx, + -2, + name.as_ptr() as *const i8, + name.len() as duk_size_t, + ) == 1 + { + duk_pop(ctx); + Ok(()) + } else { + duk_pop(ctx); + Err(DukError::from(DukErrorCode::Error, "Failed to set prop.")) + } + } else { + duk_pop(ctx); + Err(DukError::from(DukErrorCode::Error, "Error setting prop.")) + } + } else { + duk_pop(ctx); + Err(DukError::from( + DukErrorCode::NullPtr, + "Invalid heap pointer.", + )) + } } } } /// Represents a JavaScript value type. -#[derive(Clone, Debug)] -pub enum DukValue { +#[derive(Debug)] +pub enum DukValue<'a> { Undefined, Null, Number(DukNumber), Boolean(bool), String(String), - Object(DukObject) + Object(DukObject<'a>), } -impl DukValue { +impl<'a> DukValue<'a> { /// Return the value as a string. pub fn as_str(&self) -> Option { match self { DukValue::Undefined => Some(String::from("undefined")), DukValue::Null => Some(String::from("null")), - DukValue::Number(ref n) => Some(String::from(n.as_str())), + DukValue::Number(n) => Some(String::from(n.as_str())), DukValue::Boolean(b) => Some(b.to_string()), DukValue::String(s) => Some(s.clone()), - DukValue::Object(ref _o) => _o.clone().encode() + DukValue::Object(o) => o.encode(), } } + /// Return the value as a duk_bool_t (u32). pub fn as_duk_bool(&self) -> Option { match self { @@ -241,66 +277,72 @@ impl DukValue { } else { Some(0) } - }, - _ => None + } + _ => None, } } + /// Return the value as a bool. pub fn as_bool(&self) -> Option { match self { DukValue::Boolean(b) => Some(*b), - _ => None + _ => None, } } + /// Return the value as a DukNumber. pub fn as_number(&self) -> Option { match self { DukValue::Number(ref n) => Some(n.clone()), - _ => None + _ => None, } } + /// Return the value as a DukObject. - pub fn as_object(&mut self) -> Option<&mut DukObject> { + pub fn as_object(&self) -> Option<&'a DukObject> { match self { - DukValue::Object(ref mut o) => { - Some(o) - }, - _ => None + DukValue::Object(o) => Some(o), + _ => None, } } + /// Return the value as an f64. pub fn as_f64(&self) -> Option { match self { DukValue::Number(ref n) => Some(n.as_f64()), - _ => None + _ => None, } } + /// Check if this value is an f64. pub fn is_f64(&self) -> bool { match self { DukValue::Number(ref n) => n.is_f64(), - _ => false + _ => false, } } + /// Check if this value is an i64. pub fn is_i64(&self) -> bool { match self { DukValue::Number(ref n) => n.is_i64(), - _ => false + _ => false, } } + /// Check if this value is a bool. pub fn is_bool(&self) -> bool { match self { DukValue::Boolean(_b) => true, - _ => false + _ => false, } } + /// Return the value as an i64. pub fn as_i64(&self) -> Option { match self { DukValue::Number(ref n) => Some(n.as_i64()), - _ => None + _ => None, } } } @@ -315,21 +357,30 @@ pub struct DukError { /// Errors have some sort of internal structure, but the duktape /// documentation always just converts them to strings. So that's all /// we'll store for now. - message: Option + message: Option, } impl DukError { /// Create a DukError from an error code (no message). pub fn from_code(code: DukErrorCode) -> DukError { - DukError{code: code, message: None} + DukError { + code: code, + message: None, + } } /// Create a DukError from an error message (no code). pub fn from_str(message: &str) -> DukError { - DukError{code: DukErrorCode::Error, message: Some(message.to_string())} + DukError { + code: DukErrorCode::Error, + message: Some(message.to_string()), + } } /// Create a DukError from a code and message. pub fn from(code: DukErrorCode, message: &str) -> DukError { - DukError{code: code, message: Some(message.to_string())} + DukError { + code: code, + message: Some(message.to_string()), + } } /// Return the message stored in the DukError (or None if there isn't one). pub fn to_string(&self) -> Option { @@ -337,19 +388,14 @@ impl DukError { } } -impl Error for DukError { - fn description(&self) -> &str { "script error:" } - - fn cause(&self) -> Option<&Error> { None } -} +impl Error for DukError {} impl fmt::Display for DukError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match (&self.message, self.code) { (&Some(ref msg), _) => write!(f, "{}", msg), (&None, DukErrorCode::Error) => write!(f, "an unknown error occurred"), - (&None, code) => - write!(f, "type: {:?} code: {:?}", code, code as duk_int_t) + (&None, code) => write!(f, "type: {:?} code: {:?}", code, code as duk_int_t), } } } @@ -359,95 +405,106 @@ pub type DukResult = std::result::Result; /// Wrapper around a duktape context. Usable for evaluating and returning values from the context that can be used in Rust. #[derive(Clone, Debug)] pub struct DukContext { - pub ctx: Option<*mut duk_context>, + ctx: NonNull, +} + +impl Drop for DukContext { + fn drop(&mut self) { + let raw_ctx = self.ctx.as_ptr(); + unsafe { + duk_destroy_heap(raw_ctx); + } + } } impl DukContext { /// Create a duktape context. - pub fn new() -> DukContext { - unsafe { - DukContext { ctx: Some(duk_create_heap_default()) } - } - } - /// Destroy the duktape context's heap. Should not be used after calling this. - pub fn destroy(self) { - unsafe { - duk_destroy_heap(self.ctx.expect("Invalid context pointer.")); + pub fn new() -> anyhow::Result { + let ctx = unsafe { NonNull::new(duk_create_heap_default()) }; + match ctx { + Some(ctx) => Ok(Self { ctx }), + None => Err(anyhow::anyhow!("Could not create context")), } } + /// Decode a JSON string into the context, returning a DukObject. pub fn decode_json(&mut self, json: &str) -> DukValue { - match self.ctx { - Some(ctx) => { - unsafe { - duk_push_lstring(ctx, json.as_ptr() as *const i8, json.len() as duk_size_t); - duk_json_decode(ctx, -1); - self.get_value() - } - }, - None => DukValue::Undefined - } - } - /// Get a DukValue from the value at the top of the value stack in the context. - pub fn get_value(&mut self) -> DukValue { unsafe { - let t = duk_get_type(self.ctx.expect("Invalid context pointer"), -1); - match t as u32 { - DUK_TYPE_NONE => DukValue::Null, - DUK_TYPE_UNDEFINED => DukValue::Undefined, - DUK_TYPE_NULL => DukValue::Null, - DUK_TYPE_BOOLEAN => DukValue::Boolean(duk_get_boolean(self.ctx.expect("Invalid context pointer"), -1) == 1), - DUK_TYPE_NUMBER => { - let v = duk_get_number(self.ctx.expect("Invalid context pointer"), -1); - if v.fract() > 0_f64 { - DukValue::Number(DukNumber::Float(v)) - } else { - if v.is_nan() { - DukValue::Number(DukNumber::NaN) - } else if v.is_infinite() { - DukValue::Number(DukNumber::Infinity) - } else { - DukValue::Number(DukNumber::Int(v as i64)) - } - } - }, - DUK_TYPE_STRING => { - use std::ffi::CStr; - let v = duk_get_string(self.ctx.expect("Invalid context pointer"), -1); - let t = CStr::from_ptr(v); - let cow = t.to_string_lossy(); - DukValue::String(String::from(cow)) - }, - DUK_TYPE_OBJECT => { - let obj = DukObject::new(self.clone()); - DukValue::Object(obj) - }, - _ => DukValue::Undefined + duk_push_lstring( + self.ctx.as_ptr(), + json.as_ptr() as *const i8, + json.len() as duk_size_t, + ); + duk_json_decode(self.ctx.as_ptr(), -1); + } + self.get_value() + } + + /// Get a DukValue from the value at the top of the value stack in the context. + pub fn get_value(&self) -> DukValue { + let r#type = unsafe { duk_get_type(self.ctx.as_ptr(), -1) as u32 }; + match r#type { + DUK_TYPE_NONE => DukValue::Null, + DUK_TYPE_UNDEFINED => DukValue::Undefined, + DUK_TYPE_NULL => DukValue::Null, + DUK_TYPE_BOOLEAN => { + let val = unsafe { duk_get_boolean(self.ctx.as_ptr(), -1) }; + DukValue::Boolean(val == 1) } + DUK_TYPE_NUMBER => { + let v = unsafe { duk_get_number(self.ctx.as_ptr(), -1) }; + if v.fract() > 0_f64 { + DukValue::Number(DukNumber::Float(v)) + } else { + if v.is_nan() { + DukValue::Number(DukNumber::NaN) + } else if v.is_infinite() { + DukValue::Number(DukNumber::Infinity) + } else { + DukValue::Number(DukNumber::Int(v as i64)) + } + } + } + DUK_TYPE_STRING => { + let v = unsafe { + let v = duk_get_string(self.ctx.as_ptr(), -1); + CStr::from_ptr(v) + }; + let cow = v.to_string_lossy(); + DukValue::String(String::from(cow)) + } + DUK_TYPE_OBJECT => { + let obj = DukObject::new(self); + DukValue::Object(obj) + } + _ => DukValue::Undefined, } } /// Evaluate a string, returning the resulting value. - pub fn eval_string(&mut self, code: &str) -> DukResult { + pub fn eval_string(&self, code: &str) -> DukResult { unsafe { - if duk_eval_string(self.ctx.expect("Invalid context pointer"), code) == 0 { + if duk_eval_string(self.ctx.as_ptr(), code) == 0 { let result = self.get_value(); - duk_pop_2(self.ctx.expect("Invalid context pointer")); + duk_pop_2(self.ctx.as_ptr()); Ok(result) } else { - let code = duk_get_error_code(self.ctx.expect("Invalid context pointer"), -1) as u32; + let code = duk_get_error_code(self.ctx.as_ptr(), -1) as u32; let name = "stack"; - duk_get_prop_lstring(self.ctx.expect("Invalid context pointer"), -1, name.as_ptr() as *const i8, name.len() as duk_size_t); + duk_get_prop_lstring( + self.ctx.as_ptr(), + -1, + name.as_ptr() as *const i8, + name.len() as duk_size_t, + ); let val = self.get_value(); - duk_pop(self.ctx.expect("Invalid context pointer")); + duk_pop(self.ctx.as_ptr()); match val.as_str() { Some(v) => { use std::mem; let c: DukErrorCode = mem::transmute(code); Err(DukError::from(c, v.as_ref())) - }, - None => { - Err(DukError::from_code(DukErrorCode::Error)) } + None => Err(DukError::from_code(DukErrorCode::Error)), } } } @@ -457,21 +514,59 @@ impl DukContext { #[cfg(test)] mod tests { use super::*; + + #[test] + fn test_create_context() { + let ctx = DukContext::new(); + assert!(ctx.is_ok()); + } + + #[test] + fn test_eval_to_number() { + let ctx = DukContext::new().unwrap(); + let val = ctx.eval_string("10+5").unwrap(); + let val = val.as_number().unwrap(); + assert_eq!(val, DukNumber::Int(15)); + } + + #[test] + fn test_eval_to_string() { + let ctx = DukContext::new().unwrap(); + let val = ctx.eval_string("'something'.toUpperCase()").unwrap(); + let val = val.as_str().unwrap(); + assert_eq!(val, String::from("SOMETHING")); + } + + #[test] + fn test_eval_to_object() { + let ctx = DukContext::new().unwrap(); + let val = ctx.eval_string("({\"some\":\"thing\"})").unwrap(); + let _ = val.as_object().unwrap(); + } + + #[test] + fn test_get_prop_from_object() { + let ctx = DukContext::new().unwrap(); + let val = ctx.eval_string("({\"some\":\"thing\"})").unwrap(); + let obj = val.as_object().unwrap(); + assert_eq!( + obj.get_prop("some").unwrap().as_str().unwrap(), + String::from("thing") + ); + } + #[test] fn test_eval_ret() { // Create a new context - let mut ctx = DukContext::new(); + let mut ctx = DukContext::new().unwrap(); // Obtain array value from eval - let mut val = ctx.eval_string("([1,2,3])").unwrap(); + let mut val = ctx.eval_string("([1,2,3])").unwrap(); // Get the array as an object - let obj = val.as_object().expect("WAS NOT AN OBJECT"); + let obj = val.as_object().expect("WAS NOT AN OBJECT"); // Set index 3 as 4 - obj.set_prop("3", DukValue::Number(DukNumber::Int(4))).unwrap(); + obj.set_prop("3", DukValue::Number(DukNumber::Int(4))) + .unwrap(); // Encode the object to json and validate it is correct assert_eq!("[1,2,3,4]", obj.encode().expect("Should be a string")); - // Free the object for garbage collection - obj.free(); - // Destroy the heap to free the memory - ctx.destroy(); } }