PE: Add resource parser

[kkent030315]

• Addresses issue add ability to parse OriginalFilename #350.

Added resource parser implementation as proposed in the issue but with also manifests and fixed version info.

• Added parser for resource entries
• Added parser for RT_VERSION
• Added parser for RT_MANIFEST
• Added integration tests for the parsers

[m4b]

this is a huge PR! Thank you for adding this though.

Some general feedback:

1. There is a lot of new pub fields, pub structs and pub functions
   a. I believe i've mentioned in past, but to reiterate: goblin is very conservative with pub, can consider it in your mind as basically 1.0, but occasionally letting through breaking changes; with that in mind, it's important to ask yourself/ourselves whether:

   i. will the new pub structs/enums, etc., have new fields, either by addition by the binary stakeholders (microsoft, linux (i guess?), apple), or by some current oversight; if yes, or likely to have new fields added, either needs to be marked `#[non_exhaustive]`, or a field hidden so that it isn't constructible, and so adding a new field is legal. the owners of the format adding new fields is unlikely, but it has happened during the existence of this crate (e.g., apple adding new binary variants, new `LC_COMMANDS`, etc.)
   ii. do you _really_ need a field `pub`? similarly with a function; if yes, that's fine, but if any doubt, start with private/`pub(crate)` and we can make it pub later if needed; once it's pub it generally can't go back.
   

2. I haven't investigate in detail, but I assume a lot of the offset math is checked/etc., before doing any kind of unprotected/panicable &[offset..foo]; from what I've seen it all looks good, just want to double check, and I know you've fixed that in the past, so i suspect you already know this one.

3. be aware of allocations, etc., especially temporary ones, and as i mentioned, probably need to make a decision for the utf16 strings, as I mentioned in some comments.

Anyway, this is great work, other than some fairly minor nits, this looks pretty good to go, but just wanted to get some of the pub fields that don't need to be pub fixed, and etc.

Thank you for your patience and this amazing work!

[m4b]

is this in a reviewable state ? i can't remember where we left off exactly, a lot of PRs lately :)

[m4b]

friendly ping on state of this :)

[kkent030315]

@m4b Since it's almost half a year ago, it would be nice to get re-review. Thank you!

[m4b]

looks like CI is failing; rebase?

[m4b]

i will make it priority to review this and get this in once CI passing; i appreciate your patience! please feel free to aggressively ping me if i do not respond :)

[m4b]

once the utf16 string stuff is fixed, i think it will be ready to merge, after CI passes, etc. thank you for your patience and great work!

[kkent030315]

@m4b Thank you so much for your review.. it's being >2,000 lines of code. Everything you've concerned should be addressed now. Did double check on my end as well.

[m4b]

Sorry I'd really like to keep Utf16String non-pub for now if we can help it, as this is already adding a ton of new public datatypes. After this, I think we are safe to merge this; thank you for your patience!

[m4b]

perf: i'm not sure you have to allocate here if you just use it to generate fixed_info?

[kkent030315]

Unfortunately yes, we need it to find a VS_VERSION_INFO entry.

[kkent030315]

@m4b Sorry on a bit delay, I addresses the rest and would like to get the feedback :)

[m4b]

Thanks for your patience; the Utf16String is still pub, I think you forgot to add that after marking key pub, but otherwise this is ready to go, thanks for your patience! If you're too busy I can merge and fix it myself since it's very minor, but I'd prefer if possible to have it all in one patch. thank you again for your work on this!

[m4b]

Huzzah! Thank you for all your patience ! Great stuff !

[m4b]

NB: backwards compatible