fix: comment incorporation

* use libc::free instead of custom deallocator
* impl TryFrom<Request/Response>

Author: cdmurph32

Cargo.lock

@@ -33,6 +33,7 @@ c2pa = { path = "../sdk", version = "0.78.4", default-features = false, features
     "file_io",
     "pdf",
 ] }
+libc = "0.2"
 scopeguard = "1.2.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"

c2pa_c_ffi/Cargo.toml

@@ -126,8 +126,6 @@ static std::vector<const char*> parse_headers(const std::string& raw) {
     return result;
 }
 
-static void free_body(uint8_t* ptr) { free(ptr); }
-
 static int emscripten_http_handler(
     void* /*ctx*/, const C2paHttpRequest* req, C2paHttpResponse* resp)
 {
@@ -150,7 +148,6 @@ static int emscripten_http_handler(
     resp->body_len = static_cast<size_t>(fetch->numBytes);
     resp->body     = static_cast<uint8_t*>(malloc(resp->body_len));
     memcpy(resp->body, fetch->data, resp->body_len);
-    resp->body_free = free_body;
     emscripten_fetch_close(fetch);
     return 0;
 }

c2pa_c_ffi/examples/emscripten_example.cpp

@@ -242,21 +242,102 @@ pub struct C2paHttpRequest {
 /// HTTP response filled in by the resolver callback.
 ///
 /// The callback must set `status`, `body`, and `body_len`.
-/// `body` must point to callback-allocated memory. Rust will call
-/// `body_free(body)` to release it. If `body_free` is NULL, Rust
-/// assumes the body is stack- or arena-owned and will not free it
-/// (the data is copied before the callback returns).
+/// `body` must be allocated with `malloc()`. Rust will call `free()` on it
+/// after copying the data.
 #[repr(C)]
 pub struct C2paHttpResponse {
     /// HTTP status code (e.g. 200, 404)
     pub status: i32,
-    /// Response body bytes, allocated by the callback.
+    /// Response body bytes, allocated by the callback with `malloc()`.
+    /// Rust takes ownership and will call `free()`.
     pub body: *mut c_uchar,
     /// Length of `body` in bytes
     pub body_len: usize,
-    /// Deallocator for `body`. Called by Rust after copying the data.
-    /// May be NULL if `body` does not need to be freed.
-    pub body_free: Option<unsafe extern "C" fn(*mut c_uchar)>,
+}
+
+/// Owns the backing storage for a [`C2paHttpRequest`].
+///
+/// Use [`as_ffi`](OwnedC2paHttpRequest::as_ffi) to obtain the `#[repr(C)]` view
+/// whose pointers borrow from this struct.
+struct OwnedC2paHttpRequest {
+    url: std::ffi::CString,
+    method: std::ffi::CString,
+    headers: std::ffi::CString,
+    body: Vec<u8>,
+}
+
+impl TryFrom<c2pa::http::http::Request<Vec<u8>>> for OwnedC2paHttpRequest {
+    type Error = c2pa::http::HttpResolverError;
+
+    fn try_from(request: c2pa::http::http::Request<Vec<u8>>) -> Result<Self, Self::Error> {
+        use std::ffi::CString;
+
+        use c2pa::http::HttpResolverError;
+
+        let url = CString::new(request.uri().to_string())
+            .map_err(|e| HttpResolverError::Other(Box::new(e)))?;
+        let method = CString::new(request.method().as_str())
+            .map_err(|e| HttpResolverError::Other(Box::new(e)))?;
+        let headers_str: String = request
+            .headers()
+            .iter()
+            .filter_map(|(k, v)| v.to_str().ok().map(|v| format!("{k}: {v}\n")))
+            .collect();
+        let headers =
+            CString::new(headers_str).map_err(|e| HttpResolverError::Other(Box::new(e)))?;
+        let body = request.into_body();
+
+        Ok(Self {
+            url,
+            method,
+            headers,
+            body,
+        })
+    }
+}
+
+impl OwnedC2paHttpRequest {
+    /// Returns the `#[repr(C)]` view. The returned struct borrows from `self`
+    /// and must not outlive it.
+    fn as_ffi(&self) -> C2paHttpRequest {
+        let (body, body_len) = if self.body.is_empty() {
+            (std::ptr::null(), 0)
+        } else {
+            (self.body.as_ptr(), self.body.len())
+        };
+        C2paHttpRequest {
+            url: self.url.as_ptr(),
+            method: self.method.as_ptr(),
+            headers: self.headers.as_ptr(),
+            body,
+            body_len,
+        }
+    }
+}
+
+impl C2paHttpResponse {
+    /// Consumes the C-allocated response body and converts it into an
+    /// `http::Response`. Calls `free()` on the body pointer.
+    ///
+    /// # Safety
+    /// `self.body` must have been allocated with `malloc()` (or be null).
+    unsafe fn into_http_response(
+        self,
+    ) -> Result<c2pa::http::http::Response<Box<dyn std::io::Read>>, c2pa::http::HttpResolverError>
+    {
+        let body_vec = if self.body.is_null() || self.body_len == 0 {
+            Vec::new()
+        } else {
+            let v = std::slice::from_raw_parts(self.body, self.body_len).to_vec();
+            libc::free(self.body as *mut c_void);
+            v
+        };
+
+        c2pa::http::http::Response::builder()
+            .status(self.status as u16)
+            .body(Box::new(std::io::Cursor::new(body_vec)) as Box<dyn std::io::Read>)
+            .map_err(c2pa::http::HttpResolverError::Http)
+    }
 }
 
 /// Callback type for custom HTTP request resolution.
@@ -292,73 +373,31 @@ impl c2pa::http::SyncHttpResolver for C2paHttpResolver {
         request: c2pa::http::http::Request<Vec<u8>>,
     ) -> Result<c2pa::http::http::Response<Box<dyn std::io::Read>>, c2pa::http::HttpResolverError>
     {
-        use std::ffi::CString;
-
         use c2pa::http::HttpResolverError;
 
-        let uri_cstr = CString::new(request.uri().to_string())
-            .map_err(|e| HttpResolverError::Other(Box::new(e)))?;
-        let method_cstr = CString::new(request.method().as_str())
-            .map_err(|e| HttpResolverError::Other(Box::new(e)))?;
-
-        let headers_str: String = request
-            .headers()
-            .iter()
-            .filter_map(|(k, v)| v.to_str().ok().map(|v| format!("{k}: {v}\n")))
-            .collect();
-        let headers_cstr =
-            CString::new(headers_str).map_err(|e| HttpResolverError::Other(Box::new(e)))?;
-
-        let body = request.into_body();
-        let (body_ptr, body_len) = if body.is_empty() {
-            (std::ptr::null(), 0usize)
-        } else {
-            (body.as_ptr(), body.len())
-        };
-
-        let c_request = C2paHttpRequest {
-            url: uri_cstr.as_ptr(),
-            method: method_cstr.as_ptr(),
-            headers: headers_cstr.as_ptr(),
-            body: body_ptr,
-            body_len,
-        };
+        let owned = OwnedC2paHttpRequest::try_from(request)?;
+        let c_request = owned.as_ffi();
 
         let mut c_response = C2paHttpResponse {
             status: 0,
             body: std::ptr::null_mut(),
             body_len: 0,
-            body_free: None,
         };
 
         let rc =
             unsafe { (self.callback)(self.context as *mut c_void, &c_request, &mut c_response) };
 
-        // Copy the body before we potentially free it.
-        let response_body = if c_response.body.is_null() || c_response.body_len == 0 {
-            Vec::new()
-        } else {
-            unsafe { std::slice::from_raw_parts(c_response.body, c_response.body_len).to_vec() }
-        };
-
-        // Release the callback-owned buffer.
-        if let Some(free_fn) = c_response.body_free {
+        if rc != 0 {
+            // Free any body the callback may have allocated before the error.
             if !c_response.body.is_null() {
-                unsafe { free_fn(c_response.body) };
+                unsafe { libc::free(c_response.body as *mut c_void) };
             }
-        }
-
-        if rc != 0 {
             let msg = CimplError::last_message()
                 .unwrap_or_else(|| "HTTP callback returned error".to_string());
             return Err(HttpResolverError::Other(msg.into()));
         }
 
-        let response = c2pa::http::http::Response::builder()
-            .status(c_response.status as u16)
-            .body(Box::new(std::io::Cursor::new(response_body)) as Box<dyn std::io::Read>)
-            .map_err(HttpResolverError::Http)?;
-        Ok(response)
+        unsafe { c_response.into_http_response() }
     }
 }