search

neilharvey / FileSignatures

A small library for detecting the type of a file based on header signature (also known as magic number).
MIT License
250 stars 41 forks source link

Fixed issue with stream position changed when determining file format. #6

Closed cserkaran closed 6 years ago

cserkaran commented 6 years ago

@neilharvey please review this whenever you get time. This is fix for issue i created at https://github.com/neilharvey/FileSignatures/issues/5

neilharvey commented 6 years ago

Thanks for the PR. I agree with you in that the API should not leave the stream in an altered state, however the problem with setting stream.Position = 0 is that if the stream doesn't support seeking then this will throw a NotSupportedException.

We could reset the position only if the stream supports seeking, but then this leaves us with an API which can leave the original stream in an unreliable condition. Wrapping the original stream in a MemoryStream would guarantee seekable, but would consume the original stream in the process.

At this point, I'm tempted to throw a NotSupportedException if a non-seekable stream is provided, this would encourage the developer to copy their stream somewhere before passing it into FileFormatInspector if they hit this use case. Thoughts?

I also wasn't clear what the try-finally was protecting against, I think you can drop it unless there's a particular exception you're expecting.

cserkaran commented 6 years ago

@neilharvey Thanks a lot for replying and your feedback. I agree with your feedback and made the changes accordingly. Kindly review.

neilharvey commented 6 years ago

Thanks. I've merged in the PR and will push a new release shortly.