Parsing into collection until encountering specfic byte

[romatthe]

Hi, I'm trying to parse the binary format Valve uses for Steam's appinfo.vdf file as documented here (thanks to the folks over at SteamDB).

The format is as follows:

uint32   - MAGIC: 29 44 56 07
uint32   - UNIVERSE: 1
int64    - Offset to string table from start of the file
---- repeated app sections ----
uint32   - AppID
uint32   - size // until end of binary_vdf
uint32   - infoState // mostly 2, sometimes 1 (may indicate prerelease or no info)
uint32   - lastUpdated
uint64   - picsToken
20bytes  - SHA1 // of text appinfo vdf, as seen in CMsgClientPICSProductInfoResponse.AppInfo.sha
uint32   - changeNumber
20bytes  - SHA1 // of binary_vdf
variable - binary_vdf
---- end of section ----
uint32   - EOF: 0

The primary challenge I faced was that the format provides no count on how many "app sections" there are in a given appinfo.vdf file. Rather, it seems you're just expected to continue parsing until at one point you start parsing a new "app section" and encounter a u32 with value 0.

I had some trouble expressing this in binrw. I have a solution, but it feels clunky and wrong. Currently, I do the following:

#[binread]
#[br(magic = b"\x29\x44\x56\x07", little)]
struct AppInfo {
    universe: u32,
    
    #[br(temp)]
    string_table_count_ptr: FilePtr<i64, u32>,
    
    #[br(parse_with = until_exclusive(|section: &AppSection| section.appid == 0))]
    apps: Vec<AppSection>,

    string_table_count: u32,

    #[br(count = string_table_count)]
    string_table: Vec<NullString>,
}

#[binread]
#[br(little)]
struct AppSection {
    appid: u32,

    #[br(if(appid != 0, 36))]
    size: u32, // -36?

    #[br(if(appid != 0))]
    info_state: u32,

    #[br(if(appid != 0))]
    last_updated: u32,

    #[br(if(appid != 0))]
    pics_token: u64,

    #[br(if(appid != 0))]
    sha1: [u8; 20],

    #[br(if(appid != 0))]
    #[br(count = size - 36)]
    blob: Vec<u8>,
}

As you can see, I've simply used #[br(parse_with = until_exclusive(|section: &AppSection| section.appid == 0))] to stop parsing when a parsed AppSection has an appid of 0. On top of that, I ignore parsing all other fields with #[br(if(appid != 0))] if the appid is 0. This feels a bit ugly, as I actually don't even want to attempt parsing an AppSection when I encounter a u32 with value 0 at the start.

To make matters worse, it seems the count directive takes precedence over the if directive. Before I fixed this, the following code caused a panics:

    #[br(if(appid != 0))]
    #[br(count = size - 36)]
    blob: Vec<u8>,

Why? I need to subtract 36 here because of some format quirk. But whenever appid is 0, the size field is not properly read (obviously), but the count directive is still executed. This results in 0 - 36 being performed for a u32, which obviously panics. The ugly fix I had to make for this was to add a default value of 36 to the size field.

    #[br(if(appid != 0, 36))]
    size: u32, // -36?

All of this totally works, but it feels very wrong and hacky to me. My two questions are as follows:

1. Is there a better strategy for the "app sections" than using the until_exclusive directive here? If so, what is the proper way of dealing with "parsing into you encounter byte X"?
2. Is it normal that the count directive seems to take precedence over the if directive?

Thank you!

[csnover]

There are a few ways to do this. The cleanest I can think of is to replace AppSection with an enum that looks like this:

enum AppSection {
  #[br(magic = 0u32)] End,
  Section { /* existing fields */ }
}

Then you just check if it is the End variant.

There are some other options I can think of but I don’t think any of them would be much less hacky than what you are doing already.

The count thing certainly seems like a bug with the output ordering of the code generator. I have fixed this in a personal branch, but I am waiting to push things to the main repository because:

1. I want to sleep on (and get feedback if anyone feels like it) about whether or not other directives like assert should still run or not in the alternate-branch of the condition;
2. The same change for BinWrite is a little more complicated and I would like to fix both sides together.