extraction of password protected archive without encrypted filenames about sevenzipsharp

[DavidRohlik]

@squid-box well. It more complicated than I thought. You can find OperationResult codes in 7Z source code, file IArchive.h:

    namespace NOperationResult
    {
      enum
      {
        kOK = 0,
        kUnsupportedMethod,
        kDataError,
        kCRCError,
        kUnavailable,
        kUnexpectedEnd,
        kDataAfterEnd,
        kIsNotArc,
        kHeadersError,
        kWrongPassword
      };
    }

Unfortunately, my solution throws correct exception only when you use wrong password and encrypted archive (ZIP for example) without encrypted file names. In that case, extractor.ArchiveFileData is not null and extractor.ExtractFiles is able to continue until SetOperationResult from ArchiveExtractCallback is reached.

When full encrypted archive is used, exception is thrown before the extraction itself. Thats why my exception is not reached. Problematic part is line 413 int res = _archive.Open(archiveStream, ref checkPos, openCallback); in SevenZipExtractor.cs. It tries to open the archive with incorrect password, but it fails with res = 1 (it means DataError). Unfortunately, I'm not sure how to fix that yet.

My current solution is far from perfection, but at least it does not extract password protected archive without encrypted filenames with OperationResult.Ok result, so I can live with that :-)

Btw. you can check this Cube.FileSystem.SevenZip. They got different problems (more serious) in their wrapper, but password handling works ok. Unfortunately a lot of the comments are in Japan :(

[DavidRohlik]

I'm sorry. I'm totally not familiar with this web interface, so I created wrong pull request. Please delete it.

If possible, please commit my solution to this issue attached in the archive. I added all possible OperationResults and new SevenZipEncryptionException. Now it's possible to distinguish between common exceptions and wrong or no passwords.
commit.zip

[squid-box]

I was super confused at first, but no worries :)

I'll take a look soon-ish (pending real life commitments)

[squid-box]

@droh99: I've taken a quick look now. While it does look OK, I'm a bit curious about where you got the additional OperationResult from.

Writing a simple unit test which tries to extract an encrypted archive without a password (or the wrong one) supplied will give me a OperationResult.DataError instead.

How would I get to where your SevenZipEncryptionException is thrown?

Trivial test code:

[Test]
public void ExtractEncryptedArchiveTest()
{
    using (var extractor = new SevenZipExtractor(@"TestData\encrypted.7z", "test"))
    {
        extractor.ExtractArchive(OutputDirectory);
        Assert.AreEqual("encrypted", File.ReadAllText(Path.Combine(OutputDirectory, "encrypted.txt")));
    }

    using (var extractor = new SevenZipExtractor(@"TestData\encrypted.7z", "wrong"))
    {
        Assert.Throws<SevenZipEncryptionException>(() => extractor.ExtractArchive(OutputDirectory));
    }
}

(Currently fails on Assert.Throws...)

[Moralis]

Huy guys! This issue is easily reproducible by just extraction of encrypted with password ZIP file and last 7z.dll. ArchiveExtractCallback.SetOperationResult receives code 3(kDataError) with 9.2 7z and 9(kWrongPassword) with 19.0 7z, so they specified more codes as @droh99 mentioned. Result of not catching new error codes is just success extraction with zero byte files.
I'd recommend to extend OperationResult with new values like:

public enum OperationResult
{
    /// <summary>
    /// Success
    /// </summary>
    Ok = 0,
    /// <summary>
    /// Method is unsupported
    /// </summary>
    UnsupportedMethod,
    /// <summary>
    /// Data error has occured
    /// </summary>
    DataError,
    /// <summary>
    /// CrcError has occured
    /// </summary>
    CrcError,
    /// <summary>
    /// File is unavailable
    /// </summary>
    Unavailable,
    /// <summary>
    /// Unexpected end of file
    /// </summary>
    UnexpectedEnd,
    /// <summary>
    /// Data after end of archive
    /// </summary>
    DataAfterEnd,
    /// <summary>
    /// File is not archive
    /// </summary>
    IsNotArc,
    /// <summary>
    /// Archive headers error
    /// </summary>
    HeadersError,
    /// <summary>
    /// Wrong password
    /// </summary>
    WrongPassword
}

And SetOperationResult with something like:

public void SetOperationResult(OperationResult operationResult)
{
    if (operationResult != OperationResult.Ok && ReportErrors)
    {
        switch (operationResult)
        {
            case OperationResult.CrcError:
                AddException(new ExtractionFailedException("File is corrupted. Crc check has failed."));
                break;
            case OperationResult.DataError:
                AddException(new ExtractionFailedException("File is corrupted. Data error has occured."));
                break;
            case OperationResult.UnsupportedMethod:
                AddException(new ExtractionFailedException("Unsupported method error has occured."));
                break;
            case OperationResult.Unavailable:
                AddException(new ExtractionFailedException("File is unavailable."));
                break;
            case OperationResult.UnexpectedEnd:
                AddException(new ExtractionFailedException("Unexpected end of file."));
                break;
            case OperationResult.DataAfterEnd:
                AddException(new ExtractionFailedException("Data after end of archive."));
                break;
            case OperationResult.IsNotArc:
                AddException(new ExtractionFailedException("File is not archive."));
                break;
            case OperationResult.HeadersError:
                AddException(new ExtractionFailedException("Archive headers error."));
                break;
            case OperationResult.WrongPassword:
                AddException(new ExtractionFailedException("Wrong password."));
                break;
            default:
                AddException(new ExtractionFailedException($"Unexpected operation result: {operationResult}"));
                break;
        }
    }
...

It's a bug and it is not fixed for almost a year :(

About empty files, it is happening because library creates directories and files before extraction when ArchiveExtractCallback.GetStream is called, it would be nice to remove directory creation from this method and make OutStreamWrapper be able to lazily create directory and file on Write call.