RustyUsn icon indicating copy to clipboard operation
RustyUsn copied to clipboard

Bubble up errors from IterRecords

Open joshuawarner32 opened this issue 5 years ago • 3 comments

Hi! I'm building an app and trying to use this crate as a dependency. One thing I'm concerned about is that the record parsing in IterRecords will debug!-log errors and then attempt to keep parsing. For example: https://github.com/forensicmatt/RustyUsn/blob/2fae096d926a79521fefddcf088a8022295d54ea/src/usn.rs#L364

...with potentially disasterous results. Specifically, this might make corruption in the underlying records go unnoticed. In my app, I'd much rather receive an Err of some kind and be able to choose whether I want to continue parsing or bail and do something else.

Happy to open a pull request for this, if this is a change that seems like a positive direction to you.

joshuawarner32 avatar Aug 17 '20 22:08 joshuawarner32

I have the 8 byte aligned in there because it helped eliminate a lot of false positives when carving. The debug! statement was me validating that with testing. I can remove it. It's not really an error, was just a validation check which is why it was logging at the Debug level. Either way, you can always set the logger to something higher than Debug and never see the message.

forensicmatt avatar Aug 18 '20 14:08 forensicmatt

Oh interesting! Some of the other debug messages in there do look like legit errors that I do want to be notified about: https://github.com/forensicmatt/RustyUsn/blob/2fae096d926a79521fefddcf088a8022295d54ea/src/usn.rs#L402

For my use case, it's important that if the USN journal appears to be corrupted for any reason, I want to fall back to another code-path that's slower but preserves correctness.

joshuawarner32 avatar Aug 18 '20 15:08 joshuawarner32

I'd be okay with doing Option<Result<UsnEntry, UsnError >> as the iterator return.

I need to do some tlc with this repo and remove the listener as it has been moved to another repo. Will add this in too when I get a sec to touch it up.

Sorry for the delayed response.

forensicmatt avatar Aug 27 '20 16:08 forensicmatt