diff --git a/parser/src/lib.rs b/parser/src/lib.rs index f5f0b50..3a0d3cd 100644 --- a/parser/src/lib.rs +++ b/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("") } } diff --git a/parser/src/native.rs b/parser/src/native.rs index 071802e..8b66ccb 100644 --- a/parser/src/native.rs +++ b/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 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. diff --git a/parser/src/wasm.rs b/parser/src/wasm.rs index 73d380c..c8cde93 100644 --- a/parser/src/wasm.rs +++ b/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); } } diff --git a/parser/tests/basic.rs b/parser/tests/basic.rs index 278d194..02a980e 100644 --- a/parser/tests/basic.rs +++ b/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] diff --git a/parser/wasm/src/template.js b/parser/wasm/src/template.js index 8c3d066..d47f030 100644 --- a/parser/wasm/src/template.js +++ b/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 () { diff --git a/parser/wasm/test/basic.test.js b/parser/wasm/test/basic.test.js index 716ccd2..990f644 100644 --- a/parser/wasm/test/basic.test.js +++ b/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)