Skip to content

Commit

Permalink
Force write of local file header when "version needed to extract" cha…
Browse files Browse the repository at this point in the history 
…nges (#112032)

aapt2 creates ZIP entries with a "version needed to extract" of 0.0, but sometimes writes these entries with deflate compression (which requires a "version needed to extract" of 2.0.)

When a ZipArchive is opened in Update mode and a new item is added, the central directory is written with the correct value for this field. However, the local file header for existing entries isn't rewritten (because it hasn't changed.) This leads to a mismatch between the local file header values and the central directory header values, which causes Android app builds to fail.

When an existing entry's "version needed to extract" is changed, we force its local file header to be rewritten.

There's a test to cover this. I've also added an entry to the packaged_resources.zip file from the issue and confirmed that the field matches between the local file header and the CD header for all compressed entries in the resultant file.


* Add failing test
* Force local file headers to be rewritten if the version to extract is changed
* Code review
Adding/correcting comments in test, and slightly reducing the diff in ZipArchiveEntry.

---------

Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>
  • Loading branch information
@edwardneal @carlossanlop
edwardneal and carlossanlop authored Feb 10, 2025
1 parent 9b77dd4 commit 68a88d1
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 7 deletions.
6 changes: 4 additions & 2 deletions src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@ internal ZipArchiveEntry(ZipArchive archive, ZipCentralDirectoryFileHeader cd)
_archive = archive;

_originallyInArchive = true;
Changes = ZipArchive.ChangeState.Unchanged;

_diskNumberStart = cd.DiskNumberStart;
_versionMadeByPlatform = (ZipVersionMadeByPlatform)cd.VersionMadeByCompatibility;
_versionMadeBySpecification = (ZipVersionNeededValues)cd.VersionMadeBySpecification;
_versionToExtract = (ZipVersionNeededValues)cd.VersionNeededToExtract;
_generalPurposeBitFlag = (BitFlagValues)cd.GeneralPurposeBitFlag;
_isEncrypted = (_generalPurposeBitFlag & BitFlagValues.IsEncrypted) != 0;
// Setting CompressionMethod can change the _versionToExtract variable, which can change the value of Changes
CompressionMethod = (CompressionMethodValues)cd.CompressionMethod;
_lastModified = new DateTimeOffset(ZipHelper.DosTimeToDateTime(cd.LastModified));
_compressedSize = cd.CompressedSize;
Expand Down Expand Up @@ -88,8 +90,6 @@ internal ZipArchiveEntry(ZipArchive archive, ZipCentralDirectoryFileHeader cd)
_fileComment = cd.FileComment;

_compressionLevel = MapCompressionLevel(_generalPurposeBitFlag, CompressionMethod);

Changes = ZipArchive.ChangeState.Unchanged;
}

// Initializes a ZipArchiveEntry instance for a new archive entry with a specified compression level.
Expand Down Expand Up @@ -1243,10 +1243,12 @@ private void VersionToExtractAtLeast(ZipVersionNeededValues value)
if (_versionToExtract < value)
{
_versionToExtract = value;
Changes |= ZipArchive.ChangeState.FixedLengthMetadata;
}
if (_versionMadeBySpecification < value)
{
_versionMadeBySpecification = value;
Changes |= ZipArchive.ChangeState.FixedLengthMetadata;
}
}

Expand Down
150 changes: 145 additions & 5 deletions src/libraries/System.IO.Compression/tests/ZipArchive/zip_InvalidParametersAndStrangeFiles.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,146 @@ public static async Task ZipArchive_InvalidHuffmanData()
}
}

[Fact]
public static void ZipArchive_InvalidVersionToExtract()
{
using (MemoryStream updatedStream = new MemoryStream())
{
int originalLocalVersionToExtract = s_inconsistentVersionToExtract[4];
int originalCentralDirectoryVersionToExtract = s_inconsistentVersionToExtract[57];

// The existing archive will have a "version to extract" of 0.0, but will contain entries
// with deflate compression (which has a minimum version to extract of 2.0.)
Assert.Equal(0x00, originalLocalVersionToExtract);
Assert.Equal(0x00, originalCentralDirectoryVersionToExtract);

// Write the example data to the stream. We expect to be able to read it (and the entry contents) successfully.
updatedStream.Write(s_inconsistentVersionToExtract);
updatedStream.Seek(0, SeekOrigin.Begin);

using (ZipArchive originalArchive = new ZipArchive(updatedStream, ZipArchiveMode.Read, true))
{
Assert.Equal(1, originalArchive.Entries.Count);

ZipArchiveEntry firstEntry = originalArchive.Entries[0];

Assert.Equal("first.bin", firstEntry.Name);
Assert.Equal(10, firstEntry.Length);

using (Stream entryStream = firstEntry.Open())
{
Assert.Equal(10, firstEntry.Length);

byte[] uncompressedBytes = new byte[firstEntry.Length];
int bytesRead = entryStream.Read(uncompressedBytes);

Assert.Equal(10, bytesRead);

Assert.Equal(new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09 }, uncompressedBytes);
}
}

updatedStream.Seek(0, SeekOrigin.Begin);

// Create a new entry, forcing the central directory headers to be rewritten. The local file header
// for first.bin would normally be skipped (because it hasn't changed) but it needs to be rewritten
// because the central directory headers will be rewritten with a valid value and the local file header
// needs to match.
using (ZipArchive updatedArchive = new ZipArchive(updatedStream, ZipArchiveMode.Update))
{
ZipArchiveEntry newEntry = updatedArchive.CreateEntry("second.bin", CompressionLevel.NoCompression);
}

byte[] updatedContents = updatedStream.ToArray();
int updatedLocalVersionToExtract = updatedContents[4];
int updatedCentralDirectoryVersionToExtract = updatedContents[97];

Assert.Equal(20, updatedCentralDirectoryVersionToExtract);
Assert.Equal(20, updatedLocalVersionToExtract);
}
}

private static readonly byte[] s_inconsistentVersionToExtract =
{
// ===== Local file header signature 0x04034b50
0x50, 0x4b, 0x03, 0x04,
// version to extract 0.0 (invalid - this should be at least 2.0 to make use of deflate compression)
0x00, 0x00,
// general purpose flags
0x02, 0x00, // 0000_0002 'for maximum-compression deflating'
// Deflate
0x08, 0x00,
// Last mod file time
0x3b, 0x33,
// Last mod date
0x3f, 0x5a,
// CRC32
0x46, 0xd7, 0x6c, 0x45,
// compressed size
0x0c, 0x00, 0x00, 0x00,
// uncompressed size
0x0a, 0x00, 0x00, 0x00,
// file name length
0x09, 0x00,
// extra field length
0x00, 0x00,
// filename
0x66, 0x69, 0x72, 0x73, 0x74, 0x2e, 0x62, 0x69, 0x6e,
// -------------
// Data!
0x63, 0x60, 0x64, 0x62, 0x66, 0x61, 0x65, 0x63, 0xe7, 0xe0, 0x04, 0x00,
// -------- Central directory signature 0x02014b50
0x50, 0x4b, 0x01, 0x02,
// version made by 2.0
0x14, 0x00,
// version to extract 0.0 (invalid - this should be at least 2.0 to make use of deflate compression)
0x00, 0x00,
// general purpose flags
0x02, 0x00,
// Deflate
0x08, 0x00,
// Last mod file time
0x3b, 0x33,
// Last mod date
0x3f, 0x5a,
// CRC32
0x46, 0xd7, 0x6c, 0x45,
// compressed size
0x0c, 0x00, 0x00, 0x00,
// uncompressed size
0x0a, 0x00, 0x00, 0x00,
// file name length
0x09, 0x00,
// extra field length
0x00, 0x00,
// file comment length
0x00, 0x00,
// disk number start
0x00, 0x00,
// internal file attributes
0x00, 0x00,
// external file attributes
0x00, 0x00, 0x00, 0x00,
// relative offset of local header
0x00, 0x00, 0x00, 0x00,
// file name
0x66, 0x69, 0x72, 0x73, 0x74, 0x2e, 0x62, 0x69, 0x6e,
// == 'end of CD' signature 0x06054b50
0x50, 0x4b, 0x05, 0x06,
// disk number, disk number with CD
0x00, 0x00,
0x00, 0x00,
// total number of entries in CD on this disk, and overall
0x01, 0x00,
0x01, 0x00,
// size of CD
0x37, 0x00, 0x00, 0x00,
// offset of start of CD wrt start disk
0x33, 0x00, 0x00, 0x00,
// comment length
0x00, 0x00
};

private static readonly byte[] s_slightlyIncorrectZip64 =
{
// ===== Local file header signature 0x04034b50
Expand All @@ -925,7 +1065,7 @@ public static async Task ZipArchive_InvalidHuffmanData()
0x0c, 0x7e, 0x7f, 0xd8,
// compressed size
0xff, 0xff, 0xff, 0xff,
// UNcompressed size
// uncompressed size
0xff, 0xff, 0xff, 0xff,
// file name length
0x08, 0x00,
Expand Down Expand Up @@ -976,7 +1116,7 @@ public static async Task ZipArchive_InvalidHuffmanData()
0x0c, 0x7e, 0x7f, 0xd8,
// 4 byte compressed size, index 120 (-1 indicates refer to Zip64 extra field)
0xff, 0xff, 0xff, 0xff,
// 4 byte UNcompressed size, index 124 (-1 indicates refer to Zip64 extra field)
// 4 byte uncompressed size, index 124 (-1 indicates refer to Zip64 extra field)
0xff, 0xff, 0xff, 0xff,
// file name length
0x08, 0x00,
Expand Down Expand Up @@ -1066,7 +1206,7 @@ public static async Task ZipArchive_InvalidHuffmanData()
0x0c, 0x7e, 0x7f, 0xd8,
// compressed size
0xff, 0xff, 0xff, 0xff,
// UNcompressed size
// uncompressed size
0xff, 0xff, 0xff, 0xff,
// file name length

Expand All @@ -1079,7 +1219,7 @@ public static async Task ZipArchive_InvalidHuffmanData()
0x01, 0x00,
// size of extra field block
0x20, 0x00,
// 8 byte Zip64 UNcompressed size, index 42
// 8 byte Zip64 uncompressed size, index 42
0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
// 8 byte Zip64 compressed size, index 50
0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
Expand Down Expand Up @@ -1122,7 +1262,7 @@ public static async Task ZipArchive_InvalidHuffmanData()
0x0c, 0x7e, 0x7f, 0xd8,
// 4 byte compressed size, index 120 (-1 indicates refer to Zip64 extra field)
0xff, 0xff, 0xff, 0xff,
// 4 byte UNcompressed size, index 124 (-1 indicates refer to Zip64 extra field)
// 4 byte uncompressed size, index 124 (-1 indicates refer to Zip64 extra field)
0xff, 0xff, 0xff, 0xff,
// file name length
0x08, 0x00,
Expand Down

0 comments on commit 68a88d1

Please # to comment.