Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AbstractReader.Skip() does not fully read bytes from non-local streams #275

Closed
heebaek opened this issue Jul 14, 2017 · 2 comments
Closed

AbstractReader.Skip() does not fully read bytes from non-local streams #275

heebaek opened this issue Jul 14, 2017 · 2 comments
Labels
bug
Milestone

Comments

@heebaek
Copy link

@heebaek heebaek commented Jul 14, 2017

for (var i = 0; i < bytesToAdvance / skipBuffer.Length; i++)
{
rawStream.Read(skipBuffer, 0, skipBuffer.Length);
}
rawStream.Read(skipBuffer, 0, (int)(bytesToAdvance % skipBuffer.Length));

this code assume rawStream.Read always read until requested count.

but if you test with remote stream (from network like ftp), MoveToNextEntry() throw Unknown header exception. because skip function do not actually skip.

i fixed code like this, and it works fine for me.

                var bytesToAdvance = Entry.CompressedSize;
                int readBytes = 0;
                while (bytesToAdvance > 0)
                {
                    if (bytesToAdvance >= skipBuffer.Length)
                        readBytes = rawStream.Read(skipBuffer, 0, skipBuffer.Length);
                    else
                        readBytes = rawStream.Read(skipBuffer, 0, (int)bytesToAdvance);

                    bytesToAdvance -= readBytes;
                    if (readBytes == 0 && bytesToAdvance > 0)
                        throw new Exception("skip failed");
                }
                return;
            }

please let me know if it is not a bug.

@adamhathcock
Copy link
Owner

@adamhathcock adamhathcock commented Jul 14, 2017

You're probably correct in that I have some bad skip code. I've noticed recently there are several different implementations of this kind of thing in the codebase. I need to consolidate this at the very least.

I have Skip/Transfer code in Utility.cs that probably should be used instead and does what you're referring to.

I likely won't get time for a while to fix this but pull requests are welcome :)

@adamhathcock adamhathcock changed the title I think AbstractReader.Skip() has a bug. AbstractReader.Skip() does not fully read bytes from non-local streams Jul 14, 2017
@adamhathcock adamhathcock added the bug label Jul 14, 2017
@adamhathcock
Copy link
Owner

@adamhathcock adamhathcock commented Jul 15, 2017

Made a PR to fix this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.