Fix CompressionType for WinZip AES encrypted ZIP entries#1213
Fix CompressionType for WinZip AES encrypted ZIP entries#1213adamhathcock merged 5 commits intomasterfrom
Conversation
… Unknown Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
|
There was a problem hiding this comment.
Pull request overview
Updates ZIP entry metadata handling so WinZip AES–encrypted ZIP entries report their actual compression type (e.g., Deflate/LZMA) based on AES extra data, aligning Entry.CompressionType with how extraction already determines the real method.
Changes:
- Add WinZip AES extra-field parsing in
ZipEntryto derive the actual compression method forCompressionType. - Update existing WinZip AES reader tests to assert the real compression type instead of
Unknown. - Add archive-level tests asserting
CompressionTypefor WinZip AES and PKWARE encrypted ZIPs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/SharpCompress/Common/Zip/ZipEntry.cs | Derives CompressionType from WinZip AES extra data when the header method is WinzipAes (0x63). |
| tests/SharpCompress.Test/Zip/ZipReaderTests.cs | Updates WinZip AES reader test expectations from Unknown to LZMA/Deflate. |
| tests/SharpCompress.Test/Zip/ZipReaderAsyncTests.cs | Async equivalents of the WinZip AES test expectation updates. |
| tests/SharpCompress.Test/Zip/ZipArchiveTests.cs | Adds new tests validating CompressionType for WinZip AES and PKWARE encrypted entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (_filePart?.Header.CompressionMethod != ZipCompressionMethod.WinzipAes) | ||
| { | ||
| ZipCompressionMethod.BZip2 => CompressionType.BZip2, | ||
| ZipCompressionMethod.Deflate => CompressionType.Deflate, | ||
| ZipCompressionMethod.Deflate64 => CompressionType.Deflate64, | ||
| ZipCompressionMethod.LZMA => CompressionType.LZMA, | ||
| ZipCompressionMethod.PPMd => CompressionType.PPMd, | ||
| ZipCompressionMethod.None => CompressionType.None, | ||
| ZipCompressionMethod.Shrink => CompressionType.Shrink, | ||
| ZipCompressionMethod.Reduce1 => CompressionType.Reduce1, | ||
| ZipCompressionMethod.Reduce2 => CompressionType.Reduce2, | ||
| ZipCompressionMethod.Reduce3 => CompressionType.Reduce3, | ||
| ZipCompressionMethod.Reduce4 => CompressionType.Reduce4, | ||
| ZipCompressionMethod.Explode => CompressionType.Explode, | ||
| ZipCompressionMethod.ZStandard => CompressionType.ZStandard, | ||
| _ => CompressionType.Unknown, | ||
| }; | ||
| return _filePart?.Header.CompressionMethod ?? ZipCompressionMethod.None; | ||
| } |
There was a problem hiding this comment.
GetActualCompressionMethod() returns ZipCompressionMethod.None when _filePart is null, which changes the previous behavior of reporting CompressionType.Unknown in that case (old code switched on _filePart?.Header.CompressionMethod). If a ZipEntry can ever be constructed without a ZipFilePart, consider returning ZipCompressionMethod.WinzipAes/None only when you can prove the method, otherwise keep the prior Unknown behavior (e.g., return a sentinel that maps to CompressionType.Unknown).
| // For WinZip AES, the actual compression method is stored in the extra data | ||
| var aesExtraData = _filePart.Header.Extra.FirstOrDefault(x => | ||
| x.Type == ExtraDataType.WinZipAes | ||
| ); | ||
|
|
||
| if (aesExtraData is null || aesExtraData.DataBytes.Length < MinimumWinZipAesExtraDataLength) | ||
| { | ||
| return ZipCompressionMethod.WinzipAes; | ||
| } | ||
|
|
||
| // The compression method is at offset 5 in the extra data | ||
| return (ZipCompressionMethod) | ||
| System.Buffers.Binary.BinaryPrimitives.ReadUInt16LittleEndian( | ||
| aesExtraData.DataBytes.AsSpan(WinZipAesCompressionMethodOffset) | ||
| ); |
There was a problem hiding this comment.
For WinZip AES entries, this reads the compression method from the extra field but does not validate the extra data (length == 7, vendor version, vendor ID) and it uses FirstOrDefault rather than SingleOrDefault. This can make Entry.CompressionType disagree with extraction behavior, since ZipFilePart.CreateDecompressionStream validates these fields and will throw on malformed or duplicate AES extra data. To keep behavior consistent, mirror the same validation and selection logic here and fall back to Unknown when the AES metadata is invalid.
| // For WinZip AES, the actual compression method is stored in the extra data | |
| var aesExtraData = _filePart.Header.Extra.FirstOrDefault(x => | |
| x.Type == ExtraDataType.WinZipAes | |
| ); | |
| if (aesExtraData is null || aesExtraData.DataBytes.Length < MinimumWinZipAesExtraDataLength) | |
| { | |
| return ZipCompressionMethod.WinzipAes; | |
| } | |
| // The compression method is at offset 5 in the extra data | |
| return (ZipCompressionMethod) | |
| System.Buffers.Binary.BinaryPrimitives.ReadUInt16LittleEndian( | |
| aesExtraData.DataBytes.AsSpan(WinZipAesCompressionMethodOffset) | |
| ); | |
| // For WinZip AES, the actual compression method is stored in the extra data. | |
| // Mirror the validation logic used during extraction so that behavior is consistent. | |
| var aesExtraDataList = _filePart.Header.Extra | |
| .Where(x => x.Type == ExtraDataType.WinZipAes) | |
| .ToList(); | |
| // If there is not exactly one AES extra data record, treat the metadata as invalid. | |
| if (aesExtraDataList.Count != 1) | |
| { | |
| // We do not know the actual compression method; keep WinzipAes so higher layers | |
| // can treat the compression type as unknown. | |
| return ZipCompressionMethod.WinzipAes; | |
| } | |
| var aesExtraData = aesExtraDataList[0]; | |
| var data = aesExtraData.DataBytes; | |
| // WinZip AES extra data layout (7 bytes total): | |
| // 0-1: Vendor version (1 or 2) | |
| // 2-3: Vendor ID ("AE" -> 0x4541 little-endian) | |
| // 4 : AES strength (ignored here) | |
| // 5-6: Actual compression method | |
| if (data is null || data.Length != MinimumWinZipAesExtraDataLength) | |
| { | |
| return ZipCompressionMethod.WinzipAes; | |
| } | |
| var span = data.AsSpan(); | |
| ushort vendorVersion = | |
| System.Buffers.Binary.BinaryPrimitives.ReadUInt16LittleEndian(span.Slice(0, 2)); | |
| ushort vendorId = | |
| System.Buffers.Binary.BinaryPrimitives.ReadUInt16LittleEndian(span.Slice(2, 2)); | |
| // Only accept known vendor versions and the WinZip AES vendor ID ("AE"). | |
| bool isSupportedVendorVersion = vendorVersion == 1 || vendorVersion == 2; | |
| const ushort WinZipAesVendorId = 0x4541; // "AE" in little-endian | |
| if (!isSupportedVendorVersion || vendorId != WinZipAesVendorId) | |
| { | |
| return ZipCompressionMethod.WinzipAes; | |
| } | |
| // The actual compression method is at offset 5 in the extra data. | |
| ushort method = | |
| System.Buffers.Binary.BinaryPrimitives.ReadUInt16LittleEndian( | |
| span.Slice(WinZipAesCompressionMethodOffset, sizeof(ushort)) | |
| ); | |
| return (ZipCompressionMethod)method; |
WinZip AES encrypted entries report
CompressionType.Unknowninstead of the actual compression method (Deflate, LZMA, etc.). This is because WinZip AES stores the compression method in extra data at offset 5, not in the standard header field (which contains marker 0x63).Changes
GetActualCompressionMethod()to extract compression method from WinZip AES extra data when presentUnknownto assert the actual compression typeTechnical Details
WinZip AES extra data structure (7+ bytes):
For non-WinZip AES entries, behavior is unchanged.
Example
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.