Skip to content

Commit

Permalink
multiboot2: Follow C behavior for string parsing
Browse files Browse the repository at this point in the history
The various strings in the Multiboot2 spec are specified as C-style
zero-terminated strings.

Previously, if the multiboot2 crate is handed a string with non-NUL characters
present between the terminating NUL (within the tag), they were returned as
well.

This revision instead terminates the strings on the first NUL character,
as would be typical in C handling. Although unterminated strings in Multiboot2
are technically an error, current unterminated string behavior is maintained.
  • Loading branch information
A0lson committed Jul 18, 2023
1 parent 4078c90 commit 007c169
Showing 1 changed file with 13 additions and 18 deletions.
31 changes: 13 additions & 18 deletions multiboot2/src/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,17 @@ impl Tag {
/// UTF-8 Rust string slice without a terminating null byte or an error is
/// returned.
pub fn get_dst_str_slice(bytes: &[u8]) -> Result<&str, Utf8Error> {
if bytes.is_empty() {
// Very unlikely. A sane bootloader would omit the tag in this case.
// But better be safe.
return Ok("");
}
// Determine length of string before first NUL character

let length = match bytes.iter().position(|ch| (*ch) == 0x00) {
// Unterminated string case:
None => bytes.len(),

// Return without a trailing null byte. By spec, the null byte should
// always terminate the string. However, for safety, we do make an extra
// check.
let str_slice = if bytes.ends_with(&[b'\0']) {
let str_len = bytes.len() - 1;
&bytes[0..str_len]
} else {
// Unlikely that a bootloader doesn't follow the spec and does not
// add a terminating null byte.
bytes
// Terminated case:
Some(idx) => idx,
};
core::str::from_utf8(str_slice)
// Convert to Rust string:
core::str::from_utf8(&bytes[..length])
}
}

Expand Down Expand Up @@ -141,9 +134,11 @@ mod tests {
// also unlikely case
assert_eq!(Ok(""), Tag::get_dst_str_slice(&[b'\0']));
// unlikely case: missing null byte. but the lib can cope with that
assert_eq!(Ok("foobar"), Tag::get_dst_str_slice("foobar".as_bytes()));
assert_eq!(Ok("foobar"), Tag::get_dst_str_slice(b"foobar"));
// test that the null bytes is not included in the string slice
assert_eq!(Ok("foobar"), Tag::get_dst_str_slice("foobar\0".as_bytes()));
assert_eq!(Ok("foobar"), Tag::get_dst_str_slice(b"foobar\0"));
// test that C-style null string termination works as expected
assert_eq!(Ok("foo"), Tag::get_dst_str_slice(b"foo\0bar"));
// test invalid utf8
assert!(matches!(
Tag::get_dst_str_slice(&[0xff, 0xff]),
Expand Down

0 comments on commit 007c169

Please sign in to comment.