Feat/sclone#11
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds structured clone functionality and a comprehensive crypto module to the project. The changes include modifications to the core value serialization methods and introduction of WebCrypto API support with subtle crypto operations.
Key Changes
- Refactored
serializeanddeserializemethods inValueto work withTypedArrayinstead of raw byte vectors - Added
structured_clonemethod for JavaScript's structured clone algorithm - Introduced complete crypto module with WebCrypto API support (SubtleCrypto, CryptoKey, hash functions, encryption/decryption, key generation, signing/verification)
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rsquickjs/core/src/value.rs | Modified serialization methods and added structured clone support |
| modules/src/serdeserclone.rs | Global functions for serialize, deserialize, and structured clone |
| modules/src/crypto/ | Complete crypto module implementation with subtle crypto operations |
| modules/src/text/ | Text encoder/decoder implementations |
| repl/src/lib.rs | Added test for serialization (with naming issue) |
| modules/Cargo.toml | Added crypto dependencies (using RC versions) |
| modules/src/lib.rs | Integrated crypto and text modules |
| TODO.md | Updated completion status for crypto features |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let buf = unsafe { | ||
| qjs::JS_WriteObject(ctx.as_ptr(), &mut len as *mut _, self.value, flags as i32) | ||
| }; | ||
| if buf.is_null() { | ||
| return Err(Error::new_from_js( | ||
| "SerializationError", | ||
| "Failed to serialize value", | ||
| )); | ||
| }; | ||
| let slice = unsafe { std::slice::from_raw_parts(buf as *const u8, len as usize) }; | ||
| let typedarray = TypedArray::new_copy(ctx, slice)?; | ||
| Ok(typedarray.into_value()) |
There was a problem hiding this comment.
Potential memory leak: The buffer returned by JS_WriteObject is not freed after creating the slice. The buffer should be freed using JS_FreeCString or similar after copying the data.
| pub fn structured_clone(&self, ctx: Ctx<'js>) -> Result<Self> { | ||
| let mut len: u64 = 0; | ||
| let flags = qjs::JS_WRITE_OBJ_BYTECODE | ||
| | qjs::JS_WRITE_OBJ_REFERENCE | ||
| | qjs::JS_WRITE_OBJ_STRIP_SOURCE; | ||
| let buf = unsafe { | ||
| qjs::JS_WriteObject(ctx.as_ptr(), &mut len as *mut _, self.value, flags as i32) | ||
| }; | ||
| if buf.is_null() { | ||
| return Err(Error::new_from_js( | ||
| "SerializationError", | ||
| "Failed to serialize value", | ||
| )); | ||
| }; | ||
| let flags = qjs::JS_READ_OBJ_BYTECODE | qjs::JS_READ_OBJ_REFERENCE; | ||
| let value = unsafe { qjs::JS_ReadObject(ctx.as_ptr(), buf, len as u64, flags as i32) }; | ||
| if unsafe { qjs::JS_IsException(value) } { | ||
| return Err(Error::new_from_js( | ||
| "DeserializationError", | ||
| "Failed to deserialize value", | ||
| )); | ||
| } | ||
| Ok(unsafe { Value::from_js_value_const(ctx, value) }) | ||
| } |
There was a problem hiding this comment.
Potential memory leak: The buffer returned by JS_WriteObject in structured_clone is not freed. Unlike serialize() which copies to TypedArray, structured_clone passes the raw buffer directly to JS_ReadObject. The buffer should be freed after deserialization.
| use rustyline::validate::MatchingBracketValidator; | ||
| use rustyline::{Completer, Helper, Hinter, Validator}; | ||
| use rustyline::{CompletionType, Config, EditMode, Editor}; | ||
| use std::any; |
There was a problem hiding this comment.
Unused import: The any module is imported but never used in this file.
Description of changes
added structured clone and crypto