Read strings until first null byte

The MAST subrecords and description part of the HEDR subrecord have
their values treated as null-terminated strings, so anything after
the first null byte is ignored. This has been seen in Skyrim SE's
plugin loading behaviour and in the descriptions displayed for plugins
in Starfield's Creation Kit: the behaviour for other games is untested
but assumed to be the same.

There is also at least one plugin seen "in the wild" that has a MAST
subrecord with extra trailing null bytes:
cheery_AetherArtifactsCraftingAddon_NoDuplicateCrowns.esp from
<https://www.nexusmods.com/skyrimspecialedition/mods/10643>.

As such, ignore the first null byte and everything after it when getting
the description string and vector of master strings for a plugin.

Cargo.toml

@@ -15,6 +15,7 @@ coveralls = { repository = "Ortham/esplugin" }
 
 [dependencies]
 encoding_rs = "0.8.17"
+memchr = "2.7.4"
 nom = "7.0.0"
 flate2 = { version = "1.0.1", optional = true }
 unicase = "2.7.0"

ffi/src/helpers.rs

@@ -27,18 +27,6 @@ pub fn to_c_string(string: &str) -> Result<*mut c_char, u32> {
     })
 }
 
-pub fn to_truncated_c_string(string: &str) -> *mut c_char {
-    CString::new(string)
-        .unwrap_or_else(|error| {
-            let nul_position = error.nul_position();
-            let mut bytes = error.into_vec();
-            bytes.truncate(nul_position + 1);
-
-            CString::from_vec_with_nul(bytes).expect("vec should end in a nul byte")
-        })
-        .into_raw()
-}
-
 pub fn to_c_string_array(strings: &[String]) -> Result<(*mut *mut c_char, size_t), u32> {
     let mut c_strings = strings
         .iter()
@@ -100,26 +88,4 @@ mod tests {
 
         assert_eq!(ESP_ERROR_TEXT_ENCODE_ERROR, err);
     }
-
-    #[test]
-    fn to_truncated_c_string_should_return_full_string_if_it_does_not_contain_nul() {
-        let c_string = to_truncated_c_string("test without null");
-        let rust_c_string = unsafe { CStr::from_ptr(c_string) };
-
-        assert_eq!(
-            CString::new("test without null").unwrap().to_bytes(),
-            rust_c_string.to_bytes()
-        );
-    }
-
-    #[test]
-    fn to_truncated_c_string_should_truncate_string_to_first_nul() {
-        let c_string = to_truncated_c_string("test\0with null");
-        let rust_c_string = unsafe { CStr::from_ptr(c_string) };
-
-        assert_eq!(
-            CString::new("test").unwrap().to_bytes(),
-            rust_c_string.to_bytes()
-        );
-    }
 }

ffi/src/plugin.rs

@@ -3,7 +3,7 @@ use std::{f32, panic, ptr};
 
 use libc::{c_char, c_float, size_t};
 
-use esplugin::{GameId, ParseOptions, Plugin, PluginMetadata};
+use esplugin::{ParseOptions, Plugin, PluginMetadata};
 
 use crate::constants::*;
 use crate::error::{error, handle_error};
@@ -349,16 +349,10 @@ pub unsafe extern "C" fn esp_plugin_description(
 
             let c_string = match description_option {
                 None => ptr::null_mut(),
-                Some(d) => {
-                    if plugin.game_id() == GameId::Morrowind {
-                        to_truncated_c_string(&d)
-                    } else {
-                        match to_c_string(&d) {
-                            Ok(s) => s,
-                            Err(e) => return error(e, "The description contained a null byte"),
-                        }
-                    }
-                }
+                Some(d) => match to_c_string(&d) {
+                    Ok(s) => s,
+                    Err(e) => return error(e, "The description contained a null byte"),
+                },
             };
 
             *description = c_string;

src/plugin.rs

@@ -225,9 +225,9 @@ impl Plugin {
         match self.game_id {
             GameId::Fallout4 | GameId::SkyrimSE | GameId::Starfield => {
                 self.is_master_flag_set()
-                    || self.has_extension(FileExtension::Esm)
-                    // The .esl extension implies the master flag, but the light and medium flag do not.
-                    || self.has_extension(FileExtension::Esl)
+                     || self.has_extension(FileExtension::Esm)
+                     // The .esl extension implies the master flag, but the light and medium flag do not.
+                     || self.has_extension(FileExtension::Esl)
             }
             _ => self.is_master_flag_set(),
         }
@@ -293,7 +293,7 @@ impl Plugin {
                     ));
                 }
 
-                let data = &subrecord.data()[description_offset..(subrecord.data().len() - 1)];
+                let data = until_first_null(&subrecord.data()[description_offset..]);
 
                 return WINDOWS_1252
                     .decode_without_bom_handling_and_without_replacement(data)
@@ -724,7 +724,7 @@ fn masters(header_record: &Record) -> Result<Vec<String>, Error> {
         .subrecords()
         .iter()
         .filter(|s| s.subrecord_type() == b"MAST")
-        .map(|s| &s.data()[0..(s.data().len() - 1)])
+        .map(|s| until_first_null(&s.data()[0..]))
         .map(|d| {
             WINDOWS_1252
                 .decode_without_bom_handling_and_without_replacement(d)
@@ -794,9 +794,19 @@ fn read_plugin<R: BufRead + Seek>(
     })
 }
 
+/// Return the slice up to and not including the first null byte. If there is no
+/// null byte, return the whole string.
+fn until_first_null(bytes: &[u8]) -> &[u8] {
+    if let Some(i) = memchr::memchr(0, bytes) {
+        bytes.split_at(i).0
+    } else {
+        bytes
+    }
+}
+
 #[cfg(test)]
 mod tests {
-    use std::fs::copy;
+    use std::fs::{copy, read};
     use std::io::Cursor;
     use tempfile::tempdir;
 
@@ -922,16 +932,15 @@ mod tests {
         }
 
         #[test]
-        fn description_should_return_plugin_header_hedr_subrecord_content() {
+        fn description_should_trim_nulls_in_plugin_header_hedr_subrecord_content() {
             let mut plugin = Plugin::new(
                 GameId::Morrowind,
                 Path::new("testing-plugins/Morrowind/Data Files/Blank.esm"),
             );
 
             assert!(plugin.parse_file(ParseOptions::header_only()).is_ok());
 
-            let expected_description = format!("{:\0<218}{:\0<38}\n\0\0", "v5.0", "\r");
-            assert_eq!(expected_description, plugin.description().unwrap().unwrap());
+            assert_eq!("v5.0", plugin.description().unwrap().unwrap());
         }
 
         #[test]
@@ -1191,7 +1200,7 @@ mod tests {
     }
 
     mod skyrim {
-        use super::super::*;
+        use super::*;
 
         #[test]
         fn parse_file_should_succeed() {
@@ -1291,14 +1300,33 @@ mod tests {
                 GameId::Skyrim,
                 Path::new(
                     "testing-plugins/Skyrim/Data/Blank - \
-                     Master Dependent.esm",
+                      Master Dependent.esm",
                 ),
             );
 
             assert!(plugin.parse_file(ParseOptions::header_only()).is_ok());
             assert_eq!("", plugin.description().unwrap().unwrap());
         }
 
+        #[test]
+        fn description_should_trim_nulls_in_plugin_description_field_content() {
+            let mut plugin = Plugin::new(
+                GameId::Skyrim,
+                Path::new("testing-plugins/Skyrim/Data/Blank.esm"),
+            );
+
+            let mut bytes = read(plugin.path()).unwrap();
+
+            assert_eq!(0x2E, bytes[0x39]);
+            bytes[0x39] = 0;
+
+            assert!(plugin
+                .parse_reader(Cursor::new(bytes), ParseOptions::whole_plugin())
+                .is_ok());
+
+            assert_eq!("v5", plugin.description().unwrap().unwrap());
+        }
+
         #[test]
         fn header_version_should_return_plugin_header_hedr_subrecord_field() {
             let mut plugin = Plugin::new(
@@ -1458,8 +1486,6 @@ mod tests {
     mod skyrimse {
         use super::*;
 
-        use std::fs::read;
-
         #[test]
         fn is_master_file_should_use_file_extension_and_flag() {
             let tmp_dir = tempdir().unwrap();
@@ -2677,9 +2703,9 @@ mod tests {
         let result = plugin.parse_file(ParseOptions::whole_plugin());
         assert!(result.is_err());
         assert_eq!(
-            "An error was encountered while parsing the plugin content [42, 53, 41, 00, 67, 00, 00, 00, 24, 00, 00, 00, 07, 07, 00, 00]: Expected record type [54, 45, 53, 34]",
-            result.unwrap_err().to_string()
-        );
+             "An error was encountered while parsing the plugin content [42, 53, 41, 00, 67, 00, 00, 00, 24, 00, 00, 00, 07, 07, 00, 00]: Expected record type [54, 45, 53, 34]",
+             result.unwrap_err().to_string()
+         );
     }
 
     #[test]
@@ -2775,7 +2801,7 @@ mod tests {
             GameId::Skyrim,
             Path::new(
                 "testing-plugins/Skyrim/Data/Blank - \
-                 Master Dependent.esm",
+                  Master Dependent.esm",
             ),
         );
 
@@ -2786,6 +2812,25 @@ mod tests {
         assert_eq!("Blank.esm", masters[0]);
     }
 
+    #[test]
+    fn masters_should_only_read_up_to_the_first_null_for_each_master() {
+        let mut plugin = Plugin::new(
+            GameId::Skyrim,
+            Path::new("testing-plugins/Skyrim/Data/Blank - Master Dependent.esm"),
+        );
+
+        let mut bytes = read(plugin.path()).unwrap();
+
+        assert_eq!(0x2E, bytes[0x43]);
+        bytes[0x43] = 0;
+
+        assert!(plugin
+            .parse_reader(Cursor::new(bytes), ParseOptions::whole_plugin())
+            .is_ok());
+
+        assert_eq!("Blank", plugin.masters().unwrap()[0]);
+    }
+
     #[test]
     fn description_should_error_for_a_plugin_header_subrecord_that_is_too_small() {
         let mut plugin = Plugin::new(