Can Stream.CopyTo(Stream) Corrupt Data?

Question

The Background:

I have the following WriteFileToStream function which is intended to complete a simple job: take the data from a file and copy it to a Stream.

I originally was using the Stream.CopyTo(Stream) method. However, after a long debugging process, I found that this was the cause of a 'corrupt data' error further in my processing pipeline.

Synopsis:

Using the Stream.CopyTo(Stream) method yields 65536 bytes of data and the stream does not process correctly.

Using the Stream.Write(...) method yields 45450 bytes of data and the stream processes correctly.

The Question:

Can anyone see why the following usage of CopyTo may have resulted in extraneous data being written to the stream?

PLEASE NOTE: The final code in WriteFileToStream was taken from answer to this question: Save and load MemoryStream to/from a file

public static void WriteFileToStream(string fileName, Stream outputStream)
{
    FileStream file = new FileStream(fileName, FileMode.Open, FileAccess.Read);
    long fileLength = file.Length;
    byte[] bytes = new byte[fileLength];
    file.Read(bytes, 0, (int)fileLength);
    outputStream.Write(bytes, 0, (int)fileLength);
    file.Close();
    outputStream.Close();

    // This was corrupting the data - adding superflous bytes to the result...somehow.
    //using (FileStream file = File.OpenRead(fileName))
    //{
    //    // 
    //    file.CopyTo(outputStream);
    //}
}

From looking at the code you're mixing it up - *your* code is buggy and may write superfluous data to the destination while `CopyTo()` actually works fine. — Lucero, Aug 03 '12 at 16:55
Word to the wise: as developers, it's always our fault. It's (almost) never a bug in the OS or framework. — John Saunders, Aug 03 '12 at 16:58
What does your console output look like when you use the MSDN example: http://msdn.microsoft.com/en-us/library/dd782932.aspx — CrashCodes, Aug 03 '12 at 18:23

Jon Skeet · Answer 1 · 2012-08-03T16:57:46.867

Look at this code:

byte[] bytes = new byte[fileLength];
file.Read(bytes, 0, (int)fileLength);

That's broken to start with. You're ignoring the result of Stream.Read. Never do that. Suppose the file is truncated between taking the length and reading from it - you'll write a bunch of zeroes. Suppose for whatever reason, the Read call doesn't read the whole of the data even though it's there (unlikely for a local file, but I wouldn't be surprised if files accessed over the network could exhibit that behaviour) - again, you'd be writing a bunch of zeroes incorrectly.

Having said that, this is certainly an odd situation. Personally I always try to treat stream as streams - I don't like taking the size and preallocating based on that value. For example, your code could very well demonstrate that problem if the file grew while you were reading it. Without knowing more details, I don't know whether that's possible.

But no, Stream.CopyTo is fine as far as I'm aware. I think the problem is far more likely to lie elsewhere.

Note that in your commented out version, you don't close the output stream - whereas in the version which explicitly reads the file (without using a using statement, btw...) you do.

Are you able to reproduce the problem reliably? A short but complete program demonstrating the problem would be far more likely to persuade me of a bug in the framework :)

Jon: why would i need to Close the stream in the presence of a `using` statement? — JTech, Aug 03 '12 at 17:22
@JTech: It's not clear which bit of my answer you're referring to. But your current code will leave an open stream if the `Read` call throws an exception. — Jon Skeet, Aug 03 '12 at 17:34

score 0 · Answer 2 · answered Dec 30 '13 at 20:39

I've commented where I think your bug is.

public static void WriteFileToStream(string fileName, Stream outputStream)
{
    FileStream file = new FileStream(fileName, FileMode.Open, FileAccess.Read);
    long fileLength = file.Length; //bug
    byte[] bytes = new byte[fileLength];
    file.Read(bytes, 0, (int)fileLength);
    outputStream.Write(bytes, 0, (int)fileLength); //bug
    file.Close();
    outputStream.Close();

    //your code here should work when you fix the bug
}

this is what you want:

long fileLength = outputStream.Length;

and

outputStream.Write(bytes, 0, bytes.Length);

long fileLength = outputStream.Length; // this results in 'fileLength' being 0 - which is in itself a bug. — JTech, Jan 13 '14 at 20:33

Can Stream.CopyTo(Stream) Corrupt Data?

2 Answers2