From 14a403399edf821057b3e8cd3d975f581724c736 Mon Sep 17 00:00:00 2001 From: pablo Date: Fri, 27 Mar 2026 10:43:29 +0100 Subject: [PATCH] pr feedback and test fix --- core/conversations/src/api.rs | 19 +++++++++++++++---- .../src/conversation/privatev1.rs | 15 ++++++++++++--- core/conversations/src/inbox/handler.rs | 19 ++++++++++++++----- flake.nix | 10 ++++++++-- nim-bindings/src/bindings.nim | 4 ++-- nim-bindings/tests/test_all_endpoints.nim | 12 ++++++------ 6 files changed, 57 insertions(+), 22 deletions(-) diff --git a/core/conversations/src/api.rs b/core/conversations/src/api.rs index 3041b1f..a189627 100644 --- a/core/conversations/src/api.rs +++ b/core/conversations/src/api.rs @@ -47,11 +47,13 @@ pub struct ContextHandle(pub(crate) Context); /// Creates a new libchat Ctx /// /// # Returns -/// Opaque handle to the store. Must be freed with destroy_context() +/// Opaque handle to the store. Must be freed with destroy_context(). +/// Uses lossy UTF-8 conversion: invalid bytes are replaced with U+FFFD +/// so the caller always gets a deterministic name reflecting their input. #[ffi_export] pub fn create_context(name: c_slice::Ref<'_, u8>) -> repr_c::Box { - let name_str = std::str::from_utf8(name.as_slice()).unwrap_or("default"); - Box::new(ContextHandle(Context::new_with_name(name_str))).into() + let name_str = std::string::String::from_utf8_lossy(name.as_slice()); + Box::new(ContextHandle(Context::new_with_name(&name_str))).into() } /// Returns the friendly name of the contexts installation. @@ -193,7 +195,16 @@ pub fn send_content( content: c_slice::Ref<'_, u8>, out: &mut SendContentResult, ) { - let convo_id_str = std::str::from_utf8(convo_id.as_slice()).unwrap_or(""); + let convo_id_str = match std::str::from_utf8(convo_id.as_slice()) { + Ok(s) => s, + Err(_) => { + *out = SendContentResult { + error_code: ErrorCode::BadConvoId as i32, + payloads: safer_ffi::Vec::EMPTY, + }; + return; + } + }; let payloads = match ctx.0.send_content(convo_id_str, &content) { Ok(p) => p, Err(_) => { diff --git a/core/conversations/src/conversation/privatev1.rs b/core/conversations/src/conversation/privatev1.rs index 0289be5..080f315 100644 --- a/core/conversations/src/conversation/privatev1.rs +++ b/core/conversations/src/conversation/privatev1.rs @@ -60,7 +60,11 @@ pub struct PrivateV1Convo { } impl PrivateV1Convo { - pub fn new_initiator(seed_key: SymmetricKey32, remote: PublicKey, remote_delivery_address: String) -> Self { + pub fn new_initiator( + seed_key: SymmetricKey32, + remote: PublicKey, + remote_delivery_address: String, + ) -> Self { let base_convo_id = BaseConvoId::new(&seed_key); let local_convo_id = base_convo_id.id_for_participant(Role::Initiator); let remote_convo_id = base_convo_id.id_for_participant(Role::Responder); @@ -79,7 +83,11 @@ impl PrivateV1Convo { } } - pub fn new_responder(seed_key: SymmetricKey32, dh_self: &PrivateKey, remote_delivery_address: String) -> Self { + pub fn new_responder( + seed_key: SymmetricKey32, + dh_self: &PrivateKey, + remote_delivery_address: String, + ) -> Self { let base_convo_id = BaseConvoId::new(&seed_key); let local_convo_id = base_convo_id.id_for_participant(Role::Responder); let remote_convo_id = base_convo_id.id_for_participant(Role::Initiator); @@ -239,7 +247,8 @@ mod tests { let seed_key_saro = SymmetricKey32::from(seed_key); let seed_key_raya = SymmetricKey32::from(seed_key); let send_content_bytes = vec![0, 2, 4, 6, 8]; - let mut sr_convo = PrivateV1Convo::new_initiator(seed_key_saro, pub_raya, "test_addr".into()); + let mut sr_convo = + PrivateV1Convo::new_initiator(seed_key_saro, pub_raya, "test_addr".into()); let mut rs_convo = PrivateV1Convo::new_responder(seed_key_raya, &raya, "test_addr".into()); let send_frame = PrivateV1Frame { diff --git a/core/conversations/src/inbox/handler.rs b/core/conversations/src/inbox/handler.rs index 6752b3a..6b1862e 100644 --- a/core/conversations/src/inbox/handler.rs +++ b/core/conversations/src/inbox/handler.rs @@ -72,8 +72,13 @@ impl Inbox { let (seed_key, ephemeral_pub) = InboxHandshake::perform_as_initiator(self.ident.secret(), &pkb, &mut rng); - let remote_delivery_addr = Inbox::inbox_identifier_for_key(*remote_bundle.installation_key()); - let mut convo = PrivateV1Convo::new_initiator(seed_key, *remote_bundle.ephemeral_key(), remote_delivery_addr.clone()); + let remote_delivery_addr = + Inbox::inbox_identifier_for_key(*remote_bundle.installation_key()); + let mut convo = PrivateV1Convo::new_initiator( + seed_key, + *remote_bundle.ephemeral_key(), + remote_delivery_addr.clone(), + ); let mut payloads = convo.send_message(initial_message)?; @@ -119,17 +124,21 @@ impl Inbox { let ephemeral_key = self.lookup_ephemeral_key(&key_index)?; // Extract initiator's identity key for delivery address before header is consumed - let initiator_static_bytes: [u8; 32] = header.initiator_static.as_ref() + let initiator_static_bytes: [u8; 32] = header + .initiator_static + .as_ref() .try_into() .map_err(|_| ChatError::BadBundleValue("wrong size - initiator static".into()))?; - let remote_delivery_addr = Inbox::inbox_identifier_for_key(PublicKey::from(initiator_static_bytes)); + let remote_delivery_addr = + Inbox::inbox_identifier_for_key(PublicKey::from(initiator_static_bytes)); // Perform handshake and decrypt frame let (seed_key, frame) = self.perform_handshake(ephemeral_key, header, handshake.payload)?; match frame.frame_type.unwrap() { proto::inbox_v1_frame::FrameType::InvitePrivateV1(_invite_private_v1) => { - let mut convo = PrivateV1Convo::new_responder(seed_key, ephemeral_key, remote_delivery_addr); + let mut convo = + PrivateV1Convo::new_responder(seed_key, ephemeral_key, remote_delivery_addr); let Some(enc_payload) = _invite_private_v1.initial_message else { return Err(ChatError::Protocol("missing initial encpayload".into())); diff --git a/flake.nix b/flake.nix index 7653b8c..ea8d250 100644 --- a/flake.nix +++ b/flake.nix @@ -51,18 +51,24 @@ doCheck = false; # tests require network access unavailable in nix sandbox postBuild = '' - cargo run --release --bin generate-libchat-headers --features headers + cargo run --frozen --release --bin generate-libchat-headers --features headers ''; installPhase = '' runHook preInstall mkdir -p $out/lib $out/include - # Copy shared library + # Copy shared library (platform-dependent extension) cp target/release/liblibchat.so $out/lib/ 2>/dev/null || true cp target/release/liblibchat.dylib $out/lib/ 2>/dev/null || true cp target/release/liblibchat.a $out/lib/ 2>/dev/null || true + # Fail if no library was produced + if [ -z "$(ls $out/lib/liblibchat.* 2>/dev/null)" ]; then + echo "ERROR: No library artifact found in target/release/" + exit 1 + fi + # Copy generated header cp libchat.h $out/include/ diff --git a/nim-bindings/src/bindings.nim b/nim-bindings/src/bindings.nim index eb6053e..aa7310a 100644 --- a/nim-bindings/src/bindings.nim +++ b/nim-bindings/src/bindings.nim @@ -85,7 +85,7 @@ type ## Creates a new libchat Context ## Returns: Opaque handle to the context. Must be freed with destroy_context() -proc create_context*(name: ReprCString): ContextHandle {.importc.} +proc create_context*(name: SliceUint8): ContextHandle {.importc.} ## Returns the friendly name of the context's identity ## The result must be freed by the caller (repr_c::String ownership transfers) @@ -129,7 +129,7 @@ proc list_conversations*( ## The result must be freed with destroy_send_content_result() proc send_content*( ctx: ContextHandle, - convo_id: ReprCString, + convo_id: SliceUint8, content: SliceUint8, ): SendContentResult {.importc.} diff --git a/nim-bindings/tests/test_all_endpoints.nim b/nim-bindings/tests/test_all_endpoints.nim index e9e3f54..bc12f95 100644 --- a/nim-bindings/tests/test_all_endpoints.nim +++ b/nim-bindings/tests/test_all_endpoints.nim @@ -72,7 +72,7 @@ proc testHelperProcs() = proc testContextLifecycle() = echo "\n--- testContextLifecycle ---" - let ctx = create_context(toReprCString("lifecycle-test")) + let ctx = create_context(toSlice("lifecycle-test")) check(ctx != nil, "create_context: returns non-nil handle") let iname = installation_name(ctx) @@ -94,10 +94,10 @@ proc testContextLifecycle() = proc testFullConversationFlow() = echo "\n--- testFullConversationFlow ---" - let aliceCtx = create_context(toReprCString("alice")) + let aliceCtx = create_context(toSlice("alice")) check(aliceCtx != nil, "Alice: create_context non-nil") - let bobCtx = create_context(toReprCString("bob")) + let bobCtx = create_context(toSlice("bob")) check(bobCtx != nil, "Bob: create_context non-nil") # --- create_intro_bundle --- @@ -177,7 +177,7 @@ proc testFullConversationFlow() = # --- send_content --- var sendRes = send_content( aliceCtx, - toReprCString(aliceConvoId), + toSlice(aliceConvoId), toSlice("How are you, Bob?") ) check(sendRes.error_code == ErrNone, @@ -213,13 +213,13 @@ proc testFullConversationFlow() = proc testErrorCases() = echo "\n--- testErrorCases ---" - let ctx = create_context(toReprCString("error-tester")) + let ctx = create_context(toSlice("error-tester")) check(ctx != nil, "error-tester: create_context non-nil") # send_content with a nonexistent convo_id must fail var badSend = send_content( ctx, - toReprCString("00000000-0000-0000-0000-nonexistent"), + toSlice("00000000-0000-0000-0000-nonexistent"), toSlice("payload") ) check(badSend.error_code != ErrNone,