Position into the stream not reset

[LeGonidec]

When using GetFileType, the position into the stream is modified.
I had an issue trying to find out why my stream was suddenly considered as being 0 byte long, as after using FetFileType, I immediatly tried to upload it somewhere.
I suggest we could save the current position into the stream and update the position of the stream afterwords, when finishing copying the data from the stream :
in MimeType.cs

• line 214 : long currentPosition = stream.Position;
• line 224 : stream.Position = currentPosition;

[sandrock]

I think this behaviour is perfectly standard. Tools that consume a stream will move the stream position forward without moving it back. This tool should not restore the stream position for two reasons. 1. The standard behaviour is already implemented. 2. Some streams do not support seeking. This case will lead to an exception being thrown inside the tools with no control from the developer. It is your job as a user of any library to reposition your stream after you make a call that moves the stream cursor. Note: This is my opinion as an experienced developer.

…

________________________________ De : Aurélien Le Gonidec<mailto:notifications@github.com> Envoyé : ‎19/‎05/‎2017 22:51 À : Muraad/Mime-Detective<mailto:Mime-Detective@noreply.github.com> Cc : Subscribed<mailto:subscribed@noreply.github.com> Objet : [Muraad/Mime-Detective] Position into the stream not reset (#19) When using GetFileType, the position into the stream is modified. I had an issue trying to find out why my stream was suddenly considered as being 0 byte long, as after using FetFileType, I immediatly tried to upload it somewhere. I suggest we could save the current position into the stream and update the position of the stream afterwords, when finishing copying the data from the stream : in MimeType.cs * line 214 : long currentPosition = stream.Position; * line 224 : stream.Position = currentPosition; — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#19>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AATpTJyTh-Md64aZEA8YrienzyVhd42mks5r7gCigaJpZM4NhAFp>.

[LeGonidec]

1. Using this method changes the state of the stream. If this were a local copy, this wouldn't be the case. At best, I think this is unpractical.
2. If the stream you use does not support seeking, the actual implementation would fail (as you are seeking at position 0 before determining the type).

It seems logical to consider using a read or write function to change the position inside the stream. Using a method called GetType does not imply, in many people's mind, changing the state of the stream. I think it should be considered at least adding a comment.
As a note, three of our developers would not have imagined getfiletype would modify the state of the stream.

[sandrock]

I would like to come back to my previous statement.

1. "The standard behaviour is already implemented."

• It is not quite standard nor as it is as @LeGonidec desires.

2. "Some streams do not support seeking." - "the actual implementation would fail"

• I agree. But...

The actual implementation has 3 major flaws,

1. We are making filesystem access for no acceptable reason. Why create a FileStream instead of a MemoryStream?
2. Why are we copying all the stream when we can copy only the first 560 bytes?
3. The stream position is set to zero beforehand. It is the developers responsibility to do that, not the lib's. You may disagree here but all known stream operations operate like that.

• The StreamReader class will read from the current position and advance into the stream using a buffer. Reading to the end of the file does not reset the stream position.
• The Read methods on the Stream class itself advance the stream position without resetting it.
• There are many other stream oriented methods in the framework. There are none I know that will alter the stream position (seek to zero or reset)

Now I think I will submit a PR to fix the flaws.

I will be happy to continue the debate. I agree that a comment will be welcomed!

[clarkis117]

@sandrock One of the first things I did in my fork was eliminate the unneeded temp file creation and access. It uses memory streams when needed for the same effect. Then I built a story around extension methods for handling inputs like streams, byte arrays, and File Info objects.