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

Unexpected crashes in SetData method of ExtendedUnixData #887

Open
reuterma24 opened this issue Jan 2, 2025 · 0 comments
Open

Unexpected crashes in SetData method of ExtendedUnixData #887

reuterma24 opened this issue Jan 2, 2025 · 0 comments
Labels
bug *no response* zip Related to ZIP file format

Comments

@reuterma24
Copy link

reuterma24 commented Jan 2, 2025

Describe the bug

Hi :)

The SetData method in Zip/ZipExtraData.cs crashes with a System.IO.EndOfStreamException in two specific scenarios:

  • when count is set to 0
  • when index is equal to count

In both cases, the documentation does not indicate that these inputs are invalid.
I think, based on the documentation and signature, one would expect no operation to occur for these cases, as there is no data to process.

Steps to reproduce

The following two test cases can be used to demonstrate the behavior.
As of right now, they would both fail.

[Test]
public void SetDataCountZero()
{
    var extendedUnixData = new ExtendedUnixData();
    byte[] data = new byte[] { 1, 2, 3, 4 };
    int index = 0;
    int count = 0; // Nothing available to be read

    Assert.DoesNotThrow(() => extendedUnixData.SetData(data, index, count));
}

[Test]
public void SetDataOffsetAndCountEqual()
{
    var extendedUnixData = new ExtendedUnixData();
    byte[] data = new byte[] { 1, 2, 3, 4 };
    int index = 4;
    int count = 4;

    Assert.DoesNotThrow(() => extendedUnixData.SetData(data, index, count));
}

Expected behavior

I would expect the method to handle this gracefully and perform no operation.

I recommend that either,
1. the implementation is updated to handle these edge cases (e.g. return without reading from stream)

or

2. the documentation is adjusted to explicitly describe these constraints.

For 1., a simple check paired with an early return would do the trick:

/// <summary>
/// Set the data from the raw values provided.
/// </summary>
/// <param name = "data">The raw data to extract values from.</param>
/// <param name = "index">The index to start extracting values from.</param>
/// <param name = "count">The number of bytes available.</param>
public void SetData(byte[] data, int index, int count)
{
     if (count == 0 || index == count)
         return;

     using (MemoryStream ms = new MemoryStream(data, index, count, false))
     { ... }
}

Operating System

macOS

Framework Version

.NET 6

Tags

ZIP

Additional context

No response

@reuterma24 reuterma24 added the bug label Jan 2, 2025
@github-actions github-actions bot added *no response* zip Related to ZIP file format labels Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug *no response* zip Related to ZIP file format
Projects
None yet
Development

No branches or pull requests

1 participant