ZipArchive.TestArchive swallows important environmental exceptions

[yaakov-h]

Steps to reproduce

Call ZipFile.TestArchive when the application is under severe memory pressure.

Expected behavior

An OutOfMemoryException is thrown by the runtime and leaks out of SharpZipLib.

Actual behavior

SharpZipLib swallows the OutOfMemoryException and TestArchive returns false, leading people to believe that there is a problem with the ZIP file, rather than the runtime.

Version of SharpZipLib

v1.3.3.

Obtained from (only keep the relevant lines)

Package installed using NuGet

Repro case:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net48</TargetFramework>
    <RootNamespace>TestZipFile</RootNamespace>
    <Nullable>enable</Nullable>
    <LangVersion>10</LangVersion>
    <Platform>x86</Platform>
    <PlatformTarget>x86</PlatformTarget>
    <Prefer32Bit>true</Prefer32Bit>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="SharpZipLib" Version="1.3.3" />
  </ItemGroup>

</Project>

using ICSharpCode.SharpZipLib.Zip;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;

namespace TestZipFile
{
    public static class Program
    {
        public static int Main(string[] args)
        {
            if (args.Length != 1)
            {
                Console.WriteLine($"Usage: test-zip.exe <path to zip file>");
                return -1;
            }

            Console.WriteLine($"Process architecture: {RuntimeInformation.ProcessArchitecture}");

            using var fs = File.OpenRead(args[0]);
            using var zip = new ZipFile(fs);

            var list = new List<byte[]>(capacity: 20);
            var increment = 1_000_000_000;

            Console.WriteLine("Reserving memory...");

            while (true)
            {
                try
                {
                    var data = new byte[increment];
                    list.Add(data);
                }
                catch (OutOfMemoryException)
                {
                    if (increment > 10_000)
                    {
                        increment = increment / 10;
                    }
                    else
                    {
                        break;
                    }
                }
            }

            Console.WriteLine($"Reserved {list.Sum(static x => x.Length)} bytes.");

            AppDomain.CurrentDomain.FirstChanceException += (sender, e) =>
            {
                if (e.Exception is OutOfMemoryException)
                {
                    list.Clear();
                    GC.Collect();
                }
            };

            Console.WriteLine($"Verifying zip...");
            Console.WriteLine("---");
            Console.WriteLine();

            var result = zip.TestArchive(testData: true, TestStrategy.FindAllErrors, static (status, message) => { });

            GC.KeepAlive(list);

            Console.WriteLine("---");
            Console.WriteLine($"Result: {(result ? "PASS" : "FAIL")}");

            return result ? 0 : -2;
        }
    }
}

[piksel]

leading people to believe that there is a problem with the ZIP file, rather than the runtime.

Well, the error that's reported will still contain the cause: Exception during test: Out of memory.

Even though it might make sense to let environmental exceptions through, it might still be much more surprising for consumers that the TestArchive method would throw.
Saying that the archive could not be verified due to not having enough memory feels like the most generic way to handle it.
Perhaps it could be added to the test scenario as a flag?