fix: preserve valid error descriptions

parser/src/lib.rs

@@ -10,7 +10,6 @@ use core::ffi::{c_char, c_uchar, c_void};
 use core::fmt::Debug;
 use core::str;
 use core::{mem, ptr};
-use core::{slice, slice::from_raw_parts};
 
 use milo_macros::generate;
 
@@ -309,7 +308,10 @@ impl Parser {
   #[inline(always)]
   pub(crate) fn set_error(&mut self, code: u8, description: &str) {
     let bytes = description.as_bytes();
-    let len = bytes.len().min(254);
+    let mut len = bytes.len().min(254);
+    while !description.is_char_boundary(len) {
+      len -= 1;
+    }
 
     self.state = STATE_ERROR;
     self.error_code = code;
@@ -326,16 +328,11 @@ impl Parser {
 
   /// Returns the current parser's error description as string.
   pub fn error_description_str(&self) -> &str {
-    unsafe {
-      if self.error_description_len > 0 {
-        str::from_utf8_unchecked(from_raw_parts(
-          self.error_description.as_ptr(),
-          self.error_description_len as usize,
-        ))
-      } else {
-        ""
-      }
+    if self.error_description_len == 0 {
+      return "";
     }
+
+    str::from_utf8(&self.error_description[..self.error_description_len as usize]).unwrap_or("")
   }
 }
 

parser/src/native.rs

@@ -1,8 +1,6 @@
 #![allow(clippy::not_unsafe_ptr_arg_deref)]
 
-use core::ptr;
-use core::str;
-use core::{slice, slice::from_raw_parts};
+use core::{slice, str};
 use std::ffi::{CString, c_char, c_uchar};
 
 use crate::parse;
@@ -16,11 +14,13 @@ pub struct CStringWithLength {
 
 impl CStringWithLength {
   fn new(value: &str) -> CStringWithLength {
-    let cstring = CString::new(value).unwrap();
+    let bytes = value.as_bytes();
+    let len = bytes.iter().position(|byte| *byte == 0).unwrap_or(bytes.len());
+    let cstring = CString::new(&bytes[..len]).expect("bytes before the first NUL cannot contain a NUL");
 
     CStringWithLength {
       ptr: cstring.into_raw() as *const c_uchar,
-      len: value.len(),
+      len,
     }
   }
 }
@@ -29,12 +29,6 @@ impl From<&str> for CStringWithLength {
   fn from(value: &str) -> Self { CStringWithLength::new(value) }
 }
 
-impl From<CStringWithLength> for &str {
-  fn from(value: CStringWithLength) -> Self {
-    unsafe { str::from_utf8_unchecked(slice::from_raw_parts(value.ptr, value.len)) }
-  }
-}
-
 /// Returns if debug informations are available in this build.
 #[unsafe(no_mangle)]
 pub extern "C" fn milo_has_debug() -> bool { cfg!(any(debug_assertions, feature = "debug")) }
@@ -148,7 +142,15 @@ pub extern "C" fn milo_finish(parser: *mut Parser) { unsafe { (*parser).finish()
 /// Marks the parsing a failed, setting a error code and and error message.
 #[unsafe(no_mangle)]
 pub extern "C" fn milo_fail(parser: *mut Parser, code: u8, description: CStringWithLength) {
-  unsafe { (*parser).fail(code, description.into()) };
+  let description = if description.len == 0 {
+    ""
+  } else if description.ptr.is_null() {
+    "Invalid error description pointer"
+  } else {
+    let bytes = unsafe { slice::from_raw_parts(description.ptr, description.len) };
+    str::from_utf8(bytes).unwrap_or("Invalid UTF-8 error description")
+  };
+  unsafe { (*parser).fail(code, description) };
 }
 
 /// Returns the current parser's state as string.

parser/src/wasm.rs

@@ -90,11 +90,16 @@ pub fn finish(parser: *mut c_void) { unsafe { (*(parser as *mut Parser)).finish(
 /// Marks the parser as failed.
 #[unsafe(no_mangle)]
 pub fn fail(parser: *mut c_void, code: u8, description_ptr: *const c_uchar, description_len: usize) {
+  let description = if description_len == 0 {
+    ""
+  } else if description_ptr.is_null() {
+    "Invalid error description pointer"
+  } else {
+    let bytes = unsafe { slice::from_raw_parts(description_ptr, description_len) };
+    std::str::from_utf8(bytes).unwrap_or("Invalid UTF-8 error description")
+  };
   unsafe {
-    (*(parser as *mut Parser)).fail(
-      code,
-      std::str::from_utf8_unchecked(slice::from_raw_parts(description_ptr, description_len)),
-    );
+    (*(parser as *mut Parser)).fail(code, description);
   }
 }
 

parser/tests/basic.rs

@@ -4,9 +4,9 @@ mod helpers;
 use std::ffi::c_uchar;
 
 use milo_parser::{
-  CALLBACK_ACTIVE_ON_HEADERS, ERROR_NONE, ERROR_UNEXPECTED_CHARACTER, ERROR_UNEXPECTED_STATE,
+  CALLBACK_ACTIVE_ON_HEADERS, CStringWithLength, ERROR_NONE, ERROR_UNEXPECTED_CHARACTER, ERROR_UNEXPECTED_STATE,
   EVENT_ACTIVE_ON_HEADER_NAME, EVENT_ACTIVE_ON_HEADER_VALUE, STATE_BODY_DECISION, STATE_ERROR, STATE_FINISH,
-  STATE_HEADER, STATE_START,
+  STATE_HEADER, STATE_START, milo_error_description_string, milo_fail, milo_free_string,
 };
 
 use crate::helpers::{context, create_parser, http, parse};
@@ -27,6 +27,44 @@ fn basic_error_description_is_clamped_and_terminated() {
 
   assert_eq!(parser.error_description_len, 0);
   assert_eq!(parser.error_description[0], 0);
+
+  let description = format!("{}é", "a".repeat(253));
+  parser.fail(ERROR_UNEXPECTED_CHARACTER, &description);
+  assert_eq!(parser.error_description_len, 253);
+  assert_eq!(parser.error_description_str(), "a".repeat(253));
+
+  parser.error_description[0] = 0xff;
+  parser.error_description_len = 1;
+  assert_eq!(parser.error_description_str(), "");
+}
+
+#[test]
+fn basic_ffi_error_description_rejects_invalid_utf8() {
+  let mut parser = create_parser();
+  let invalid = [0xff];
+  let description = CStringWithLength {
+    ptr: invalid.as_ptr(),
+    len: invalid.len(),
+  };
+
+  milo_fail(&mut parser, ERROR_UNEXPECTED_CHARACTER, description);
+  assert_eq!(parser.error_description_str(), "Invalid UTF-8 error description");
+
+  milo_fail(
+    &mut parser,
+    ERROR_UNEXPECTED_CHARACTER,
+    CStringWithLength {
+      ptr: std::ptr::null(),
+      len: 0,
+    },
+  );
+  assert_eq!(parser.error_description_str(), "");
+
+  parser.fail(ERROR_UNEXPECTED_CHARACTER, "before\0after");
+  let output = milo_error_description_string(&mut parser);
+  assert_eq!(output.len, 6);
+  assert_eq!(unsafe { std::slice::from_raw_parts(output.ptr, output.len) }, b"before");
+  milo_free_string(output);
 }
 
 #[test]

parser/wasm/src/template.js

@@ -31,13 +31,16 @@ function parse (parser, data, limit) {
 }
 
 function fail (parser, code, description) {
-  const len = description.length
+  const bytes = textEncoder.encode(description)
+  const len = bytes.byteLength
   const ptr = this.alloc(len)
-  const buffer = new Uint8Array(this.memory.buffer, ptr, len)
-  textEncoder.encodeInto(description, buffer)
 
-  this.fail(parser, code, ptr, len)
-  this.dealloc(ptr, len)
+  try {
+    new Uint8Array(this.memory.buffer, ptr, len).set(bytes)
+    this.fail(parser, code, ptr, len)
+  } finally {
+    this.dealloc(ptr, len)
+  }
 }
 
 function hasDebug () {

parser/wasm/test/basic.test.js

@@ -12,6 +12,15 @@ it('basic_error_description_is_clamped_and_terminated', t => {
   assert.equal(memory[parser + milo.PARSER_FIELD_ERROR_DESCRIPTION + 254], 0)
   assert.equal(milo.getErrorDescription(parser), 'a'.repeat(254))
 
+  milo.fail(parser, milo.ERROR_UNEXPECTED_CHARACTER, `${'a'.repeat(253)}é`)
+  assert.equal(milo.getErrorDescription(parser), 'a'.repeat(253))
+
+  milo.fail(parser, milo.ERROR_UNEXPECTED_CHARACTER, 'é😀')
+  assert.equal(milo.getErrorDescription(parser), 'é😀')
+
+  milo.fail(parser, milo.ERROR_UNEXPECTED_CHARACTER, '')
+  assert.equal(milo.getErrorDescription(parser), '')
+
   milo.reset(parser, false)
   memory = new Uint8Array(milo.memory.buffer)
   assert.equal(memory[parser + milo.PARSER_FIELD_ERROR_DESCRIPTION_LEN], 0)