ultimate-research / ssbh_lib

Reading and writing SSBH file formats in Rust
MIT License
8 stars 3 forks source link

fuzz testing fails for ssbh_lib formats due to versioning #116

Closed ScanMountGoat closed 2 years ago

ScanMountGoat commented 2 years ago

The current implementation doesn't require the major and minor version to be in sync with the data. The following matl fails to produce the same output after a write + read + write. This is caused by a failed read after the write since the specified version is not recognized.

 Matl {
    major_version: 1028,
    minor_version: 1028,
    entries: EntriesV15(
        SsbhArray {
            elements: [],
        },
    ),
}

This affects all SSBH formats that have more than one supported versions. Adj and MeshEx aren't effected since they don't specify a version.

ScanMountGoat commented 2 years ago

Using an enum for the version doesn't entirely fix the issue. For example, setting the version to 1.6 but initializing the entries as 1.5 will also return an error when attempting to read after the write. The entire matl struct needs to be versioned to guarantee at compile time that the data is able to be read later. This may require some modifications of SsbhWrite to support hardcoded values for the major and minor version.

ScanMountGoat commented 2 years ago

A potential design to share the reader and writer logic. An example type would be Versioned<Matl> where Matl is an enum with variants Matl::V15(MatlV15) and Matl::V16(MatlV16).

#[derive_binread]
struct Versioned<T: BinRead<Args = (u16, u16)>> {
    #[br(temp)]
    major_version: u16,

    #[br(temp)]
    minor_version: u16,

    #[br(args(major_version, minor_version))]
    pub data: T,
}

impl<T> SsbhWrite for Versioned<T>
where
    T: BinRead<Args = (u16, u16)> + SsbhWrite + Version,
{
    fn ssbh_write<W: std::io::Write + std::io::Seek>(
        &self,
        writer: &mut W,
        data_ptr: &mut u64,
    ) -> std::io::Result<()> {
        // TODO: Get the version from the trait?
        todo!()
    }

    fn size_in_bytes(&self) -> u64 {
        2 + 2 + self.size_in_bytes()
    }
}

trait Version {
    fn major_version(&self) -> u16;
    fn minor_version(&self) -> u16;
}
ScanMountGoat commented 2 years ago

Matl and nufx still fail due to invalid data types. This requires #110 to be finished.