Skip to content

Comments

Fix some IStreamStack and SharpCompressStream functions#1017

Merged
adamhathcock merged 4 commits intoadamhathcock:masterfrom
Morilli:fix-streamstack
Nov 14, 2025
Merged

Fix some IStreamStack and SharpCompressStream functions#1017
adamhathcock merged 4 commits intoadamhathcock:masterfrom
Morilli:fix-streamstack

Conversation

@Morilli
Copy link
Contributor

@Morilli Morilli commented Nov 14, 2025

Buffer handling seems to have been completely broken.
I've fixed the handling in IStreamStack.StackSeek that basically duplicated the logic from SharpCompressStream.Seek (while not actually working correctly), fixed incorrect behavior of the IStreamStack.BufferPosition setter and fixed and off-by-one in SharpCompressStream.Seek.

I suspect there are more issues in StackSeek that may or may not even be relevant for how buffered IStreamStacks are even used. It's hard to tell because the method documentation does not seem to fully match the implementation.

This allows reverting #964 now that the underlying issue has been fixed.

TODO:

  • make some tests for this to validate behavior

the BufferSize setter was completely broken and trashed the `_internalPosition` value on set, for `Seek` this is just an off-by-one fix allowing seeking to immediately past the last byte in the buffer
Copy link
Owner

@adamhathcock adamhathcock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll take a look at this. Seek and setting BufferPosition should be the same thing?

@adamhathcock adamhathcock merged commit bcf9a6b into adamhathcock:master Nov 14, 2025
2 checks passed
@Morilli
Copy link
Contributor Author

Morilli commented Nov 14, 2025

Seek and setting BufferPosition should be the same thing?

For a buffered stream, it is the same if the seek would only seek to somewhere inside of the currently buffered region. Unfortunately some buffered streams aren't seekable so we can't unconditionally call seek without trying to set the buffer position beforehand. This seems to be mainly required for archive detection logic as that uses a buffered, non-seekable stream for rewind logic where we need to be able to rewind the buffer position while not having access to the Seek method.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ExtractAllEntries returns no entries when reading a ZIP from a Stream

2 participants