Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make reader / writer exceptions more distinct #235

Open
Washi1337 opened this issue Dec 22, 2021 · 0 comments
Open

Make reader / writer exceptions more distinct #235

Washi1337 opened this issue Dec 22, 2021 · 0 comments

Comments

@Washi1337
Copy link
Owner

Washi1337 commented Dec 22, 2021

Problem Description

I get sent many questions with regards to ignoring both reader and writer errors. Usually the question involves a process where people write .NET images manually using ManagedPEImageBuilder. Often they are confused and/or surprised about the fact that AsmResolver can throw errors upon writing the file as a result of invalid metadata, even if the DiagnosticBag from the writer is ignored as the docs state you should do in this situation. This is a direct result of the lazy loading nature of AsmResolver. The reader might delay parsing certain sections of the PE until the very end when an image is written to the disk again. If it encounters invalid data at that point, the error listener for the reader is triggered, and not the one for the writer.

It seems it is sometimes unclear where errors are coming from, which causes confusion when one class of errors is ignored but the other class isn't (Questions are often like "Didn't I ignore errors before?" or "Why do I need to ignore more errors?"). We should help users identifying the type of error that they are dealing with, so that they know what to look for.

Proposal

Categorize reader and writer errors using dedicated Exception class derivatives. This helps identifying whether something is a reader or a writer error, or an actual bug.

Example names could be:

  • ReaderException
  • WriterException

They can probably be put into the base AsmResolver package.

Alternatives

We could reuse BadImageFormatException more for all reader errors.

We could also clarify this phenomenon better in the documentation.

Additional context

Splitting reader and writer exceptions seems to be different behavior than other libraries adopt (e.g. Cecil or dnlib), which is probably the reason why people are confused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant