locked
Inherited FileStream arbitrarily sets position to end of buffer rather than actual position RRS feed

  • Question

  • I inherited the FileStream class in my solution that is meant to copy the contents to a new file but give the user the ability to read, decompress, and analyze the stream before copying it in order to make changes along the way.

    public class MixedCompressionStreamRewriter : FileStream

    My first test of the code seemed to work just fine so I tried it on a few more files.  With one file, the buffer has been cleared at position 203.  It reads the start of the next record and determines what kind it is, advancing the the position to 207.  At position 207 it then reads (another) Uint32 (using a BinaryReader).  After reading the byte the position has advanced by 4092 bytes rather than 4 bytes.  I don't think it is a coincidence that the buffer size is 4096 and the buffer is now full. 

    I can build a workaround into my Read method but this is a problem as it is NOT happening in my code.  Why does it do this and how can I/we fix it?  Below are the two methods (the one that reads the uint32 and the overridden code):

    public static RecordHeaderBase FromFile(BinaryReader reader, uint internalIDMask = 0xFFFFFFFF)
    {
    	RecordConsolidator consolidator = reader.BaseStream as RecordConsolidator;
    	if (consolidator != null)
    		consolidator.EnableParseMode = true;
    	RecordTypeInfo typeInfo = (RecordType)reader.ReadUInt32(); // THIS IS THE LINE THAT CALLS MY OVERRIDE
    	if (consolidator != null)
    		consolidator.EnableParseMode = false;
    	if (typeInfo == RecordType.TES4)
    		return new FileHeader(reader);
    	else if (typeInfo.isContainer)
    		return new GroupHeader(reader, typeInfo, internalIDMask);
    	else
    		return new RecordHeader(reader, typeInfo, internalIDMask);
    }

    Here is the overridden read method:

    public override int Read(byte[] array, int offset, int count)
    {
    	int result;
    	if (compressionHeaderStream == null) // TRUE IN THIS CASE
    	{
    		result = base.Read(array, offset, count); // HERE IS WHERE IT JUMPS TO 4299 rather than 207
    		if (!parseMode) // NOT TRUE IN THIS CASE
    		{
    			long equivalentBufferLocation = MemoryBufferStart + MemoryBuffer.Length;
    			if (Position > equivalentBufferLocation)
    			{
    				long bufferCopyBytes = Position - equivalentBufferLocation;
    				if (bufferCopyBytes > int.MaxValue)
    					throw new OverflowException();
    				offset += (count - (int)bufferCopyBytes);
    				MemoryBuffer.Write(array, offset, (int)bufferCopyBytes);
    			}
    			if (MaxBufferSize != 0)
    				if (MemoryBuffer.Length >= MaxBufferSize)
    					FlushToOutput();
    		}
    	}
    	else if (MemoryBuffer == null)
    				result = base.Read(array, offset, count);
    	else
    		result = MemoryBuffer.Read(array, offset, count);
    	return result;
    }

    • Edited by primem0ver Sunday, November 8, 2020 4:58 PM Change title to be more accurate
    Sunday, November 8, 2020 4:53 PM

Answers

  • I believe you are misusing the FileStream. FileStream is an implementation of Stream for files. You should make no assumptions about its behavior. It sounds like you are trying to create a stream wrapper and therefore you shouldn't be inheriting from FileStream at all. This, to me, indicates a fundamental error. FileStream isn't really designed to be overridden. 

    It sounds like you want to create a stream wrapper. For that you should derive directly from Stream. Accept, as a constructor parameter, the stream you want to wrap. Inside your implementation you can internally read/write the underlying stream and track where you are at in it. This is independent of where the stream may actually be. This is how all the existing stream wrapping classes work and they work against arbitrary streams correctly.

    You may use a reader/writer if you want but bear in mind that reader/writers are using the shared underlying stream and the stream tracks the position. Therefore attempting to use multiple readers/writers on the same string is going to mangle things. If you are using a reader internally on the stream and the caller also is reading that stream then it will not work.

    In most stream wrappers that I've created it is easier to have the custom stream wrapped in the reader/writer by the client. The wrapper then internally is moving the stream position around as needed to implement the read/write functionality of the stream. To do header validation, for example, the wrapper would read the header from the underlying stream on the first read (or perhaps at construction) and fail if it didn't read properly. After that it would simply be reading the raw data. Since the underlying stream is not used directly by the client code the wrapper has complete control over the positioning and can read/write how it seems necessary.


    Michael Taylor http://www.michaeltaylorp3.net

    • Marked as answer by primem0ver Friday, November 13, 2020 8:05 PM
    Monday, November 9, 2020 3:44 PM

All replies

  • Are you mixing BinaryReader with other readers?  It would not surprise me to learn that BinaryReader assumes it owns the stream, so it reads ahead in the buffer.  I don't see any debug prints here.  In your overridden Read method, do you see the "count" value is 4096 when this happens?  FileStream.Read is certainly not going to advance more than you asked it to.

    It may be that you need to use BinaryReader for everything.


    Tim Roberts | Driver MVP Emeritus | Providenza & Boekelheide, Inc.

    Sunday, November 8, 2020 7:17 PM
  • I believe you are misusing the FileStream. FileStream is an implementation of Stream for files. You should make no assumptions about its behavior. It sounds like you are trying to create a stream wrapper and therefore you shouldn't be inheriting from FileStream at all. This, to me, indicates a fundamental error. FileStream isn't really designed to be overridden. 

    It sounds like you want to create a stream wrapper. For that you should derive directly from Stream. Accept, as a constructor parameter, the stream you want to wrap. Inside your implementation you can internally read/write the underlying stream and track where you are at in it. This is independent of where the stream may actually be. This is how all the existing stream wrapping classes work and they work against arbitrary streams correctly.

    You may use a reader/writer if you want but bear in mind that reader/writers are using the shared underlying stream and the stream tracks the position. Therefore attempting to use multiple readers/writers on the same string is going to mangle things. If you are using a reader internally on the stream and the caller also is reading that stream then it will not work.

    In most stream wrappers that I've created it is easier to have the custom stream wrapped in the reader/writer by the client. The wrapper then internally is moving the stream position around as needed to implement the read/write functionality of the stream. To do header validation, for example, the wrapper would read the header from the underlying stream on the first read (or perhaps at construction) and fail if it didn't read properly. After that it would simply be reading the raw data. Since the underlying stream is not used directly by the client code the wrapper has complete control over the positioning and can read/write how it seems necessary.


    Michael Taylor http://www.michaeltaylorp3.net

    • Marked as answer by primem0ver Friday, November 13, 2020 8:05 PM
    Monday, November 9, 2020 3:44 PM
  • Thank you Michael; this suggestion should suffice.  Other sites kept saying to use a non-stream wrapper which wont work for my situation because the writer must use my stream as a base in order for other functionality to work properly.   I will accept your submission as the answer for now (as it will improve efficiency on my workaround drastically) and test it out when I get the opportunity.  Meanwhile the workaround is doing the job (am anxious to get this project working since I have been working on it for almost a year).
    • Edited by primem0ver Friday, November 13, 2020 8:09 PM
    Friday, November 13, 2020 8:05 PM