Skip to content

Commit b541b87

Browse files
dellis1972dellis1972
authored
Fix the elusive invalid zip archive issue that has been a problem for ages! (#142)
* Fix the elusive invalid zip archive issue that has been a problem for ages! Fixesdotnet/android#8988 We had this odd corrupt zip file issue which kept cropping up on our Azure Pipelines builds. We had no idea what caused it until now. Some of the data for the local headers of an item (not the central directory) would be written incorrectly. This would result in a zip which may or may not be extractable, it would depend on how resilient the software extracting the data would be. So, what was happening here was that (sometimes) libzip would start writing some data (most likely the local file header) using our stream source callback, and it would seek a few bytes into the data and then tried to seek back to the beginning. The latter seek was done by giving the seek operation of the callback an offset of 0 which, unfortunately, was also used by the code as a guard as to whether or not to even perform the seek operation. The effect was that we ignored the seek to 0 and the stream remained at whatever the previous seek location was requested, thus corrupting data. It happened only on the very first entry, since that was the only one which would have position 0 within its range. We discovered that just enabling the strict consistency checks would uncover the issue, so that has been enabled in a number of unit tests. Once we did that it turns out we were writting the corrupt data ALL the TIME!. Fixing up the seeking code to take into account that we might want to see to 0 fixed the issue. * Bump to 3.3.0 due to ABI changes
1 parent c2ae332 commit b541b87

5 files changed

Lines changed: 69 additions & 41 deletions

File tree

‎.vscode/tasks.json‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
{
77
"label": "Build LibZipSharp",
88
"type": "shell",
9-
"command": "msbuild LibZipSharp/libZipSharp.csproj /restore /t:Build",
9+
"command": "dotnet build LibZipSharp/libZipSharp.csproj -t:Build",
1010
"group": {
1111
"kind": "build",
1212
"isDefault": true
@@ -18,7 +18,7 @@
1818
{
1919
"label": "Pack LibZipSharp Nuget",
2020
"type": "shell",
21-
"command": "msbuild LibZipSharp/libZipSharp.csproj /t:Pack",
21+
"command": "dotnet pack LibZipSharp/libZipSharp.csproj",
2222
"group": {
2323
"kind": "build",
2424
"isDefault": true
@@ -30,7 +30,7 @@
3030
{
3131
"label": "Clean LibZipSharp",
3232
"type": "shell",
33-
"command": "msbuild LibZipSharp/libZipSharp.csproj /restore /t:Clean",
33+
"command": "dotnet build LibZipSharp/libZipSharp.csproj -t:Clean",
3434
"group": {
3535
"kind": "build",
3636
"isDefault": true
@@ -42,7 +42,7 @@
4242
{
4343
"label": "Build LibZipSharp Unit Tests",
4444
"type": "shell",
45-
"command": "msbuild LibZipSharp.UnitTest/LibZipSharp.UnitTest.csproj /restore /t:Build /p:ReferenceNuget=False",
45+
"command": "dotnet build LibZipSharp.UnitTest/LibZipSharp.UnitTest.csproj -t:Build -p:ReferenceNuget=False",
4646
"group": {
4747
"kind": "build",
4848
"isDefault": true
@@ -51,17 +51,5 @@
5151
"$msCompile"
5252
]
5353
},
54-
{
55-
"label": "Run LibZipSharp Unit Tests",
56-
"type": "shell",
57-
"command": "msbuild LibZipSharp.UnitTest/LibZipSharp.UnitTest.csproj /restore /t:RunNunitTests /p:ReferenceNuget=False",
58-
"group": {
59-
"kind": "test",
60-
"isDefault": true
61-
},
62-
"problemMatcher": [
63-
"$msCompile"
64-
]
65-
}
6654
]
6755
}

‎LibZipSharp.UnitTest/ZipTests.cs‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ public void InMemoryZipFile ()
345345
}
346346

347347
stream.Position=0;
348-
using(varzip=ZipArchive.Open(stream)){
348+
using(varzip=ZipArchive.Open(stream,strictConsistencyChecks:true)){
349349
Assert.AreEqual(3,zip.EntryCount);
350350
foreach(vareinzip){
351351
Console.WriteLine(e.FullName);
@@ -354,7 +354,7 @@ public void InMemoryZipFile ()
354354
}
355355

356356
stream.Position=0;
357-
using(varzip=ZipArchive.Open(stream)){
357+
using(varzip=ZipArchive.Open(stream,strictConsistencyChecks:true)){
358358
Assert.AreEqual(2,zip.EntryCount);
359359
zip.AddEntry("info.json",File.ReadAllText(filePath),Encoding.UTF8,CompressionMethod.Deflate);
360360
}
@@ -377,6 +377,40 @@ public void InMemoryZipFile ()
377377
}
378378
}
379379

380+
[Test]
381+
publicvoidEnumerateOnEmptyThenAddFiles()
382+
{
383+
File.WriteAllText("file1.txt",TEXT);
384+
File.WriteAllText("file2.txt",TEXT);
385+
stringfilePath=Path.GetFullPath("file1.txt");
386+
if(File.Exists("enumerateonempty.zip"))
387+
File.Delete("enumerateonempty.zip");
388+
389+
390+
using(varstream=File.Create("test-archive-write.zip"))
391+
using(varzip=ZipArchive.Create(stream)){
392+
foreach(varentryinzip){
393+
Console.Write(entry.FullName);
394+
}
395+
ZipEntrye;
396+
e=zip.AddFile(filePath,"/path/ZipTestCopy1.exe");
397+
filePath=Path.GetFullPath("file2.txt");
398+
e=zip.AddFile(filePath,"/path/ZipTestCopy2.exe");
399+
400+
foreach(varentryinzip){
401+
Console.Write(entry.FullName);
402+
}
403+
404+
zip.Close();
405+
}
406+
407+
using(varzip=ZipArchive.Open("test-archive-write.zip",FileMode.Open,strictConsistencyChecks:true)){
408+
foreach(varentryinzip){
409+
Console.Write(entry.FullName);
410+
}
411+
}
412+
}
413+
380414
[TestCase(false)]
381415
[TestCase(true)]
382416
publicvoidEnumerateSkipDeletedEntries(booldeleteFromExistingFile)

‎LibZipSharp.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<ProjectDefaultTargets="Build"ToolsVersion="4.0"xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
22
<PropertyGroup>
33
<_LibZipSharpAssemblyVersionMajor>3</_LibZipSharpAssemblyVersionMajor>
4-
<_LibZipSharpAssemblyVersionMinor>2</_LibZipSharpAssemblyVersionMinor>
4+
<_LibZipSharpAssemblyVersionMinor>3</_LibZipSharpAssemblyVersionMinor>
55
<_LibZipSharpAssemblyVersionPatch>0</_LibZipSharpAssemblyVersionPatch>
66
<_LibZipSharpAssemblyVersion>$(_LibZipSharpAssemblyVersionMajor).$(_LibZipSharpAssemblyVersionMinor).$(_LibZipSharpAssemblyVersionPatch)</_LibZipSharpAssemblyVersion>
77
<_NativeLibraryVersionForName>$(_LibZipSharpAssemblyVersionMajor)-$(_LibZipSharpAssemblyVersionMinor)</_NativeLibraryVersionForName>

‎LibZipSharp/Xamarin.Tools.Zip/ZipArchive.cs‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
usingSystem.Buffers;
2929
usingSystem.Collections;
3030
usingSystem.Collections.Generic;
31+
usingSystem.Diagnostics;
3132
usingSystem.IO;
3233
usingSystem.Runtime.InteropServices;
3334
usingSystem.Text;
@@ -195,19 +196,26 @@ ErrorCode Open (string path, OpenFlags flags)
195196
/// </summary>
196197
/// <param name="stream">The stream to open</param>
197198
/// <param name="options">Platform-specific options</param>
198-
publicstaticZipArchiveOpen(Streamstream,IPlatformOptionsoptions=null)
199+
/// <param name="strictConsistencyChecks"></param>
200+
publicstaticZipArchiveOpen(Streamstream,IPlatformOptionsoptions=null,boolstrictConsistencyChecks=false)
199201
{
200-
returnZipArchive.CreateInstanceFromStream(stream,OpenFlags.None,options);
202+
OpenFlagsflags=OpenFlags.None;
203+
if(strictConsistencyChecks)
204+
flags|=OpenFlags.CheckCons;
205+
returnZipArchive.CreateInstanceFromStream(stream,flags,options);
201206
}
202207

203208
/// <summary>
204209
/// Create a new archive using the Stream provided. The steam should be an empty stream, any existing data will be overwritten.
205210
/// </summary>
206211
/// <param name="stream">The stream to create the arhive in</param>
207212
/// <param name="options">Platform-specific options</param>
208-
publicstaticZipArchiveCreate(Streamstream,IPlatformOptionsoptions=null)
213+
publicstaticZipArchiveCreate(Streamstream,IPlatformOptionsoptions=null,boolstrictConsistencyChecks=false)
209214
{
210-
returnZipArchive.CreateInstanceFromStream(stream,OpenFlags.Create|OpenFlags.Truncate,options);
215+
OpenFlagsflags=OpenFlags.Create|OpenFlags.Truncate;
216+
if(strictConsistencyChecks)
217+
flags|=OpenFlags.CheckCons;
218+
returnZipArchive.CreateInstanceFromStream(stream,flags,options);
211219
}
212220

213221
/// <summary>
@@ -788,6 +796,7 @@ internal ZipException GetErrorException ()
788796
internalstaticunsafeInt64stream_callback(IntPtrstate,IntPtrdata,UInt64len,SourceCommandcmd)
789797
{
790798
byte[]buffer=null;
799+
Native.zip_error_terror;
791800
intlength=(int)len;
792801
varhandle=GCHandle.FromIntPtr(state);
793802
if(!handle.IsAllocated)
@@ -833,30 +842,26 @@ internal static unsafe Int64 stream_callback (IntPtr state, IntPtr data, UInt64
833842
return-1;
834843
}
835844

836-
longfirstOffset,secondOffset;
845+
SeekOriginseek=Native.ConvertWhence(args.whence);
837846
if(args.offset>Int64.MaxValue){
838847
// Stream.Seek uses a signed 64-bit value for the offset, we need to split it up
839-
firstOffset=Int64.MaxValue;
840-
secondOffset=(long)(args.offset-Int64.MaxValue);
848+
if(!Seek(destination,seek,Int64.MaxValue)||!Seek(destination,seek,(long)(args.offset-Int64.MaxValue))){
849+
return-1;
850+
}
841851
}else{
842-
firstOffset=(long)args.offset;
843-
secondOffset=0;
844-
}
845-
846-
SeekOriginseek=Native.ConvertWhence(args.whence);
847-
Native.zip_error_terror;
848-
849-
if(!SeekIfNeeded(destination,seek,firstOffset)||!SeekIfNeeded(destination,seek,secondOffset)){
850-
return-1;
852+
if(!Seek(destination,seek,(long)args.offset)){
853+
return-1;
854+
}
851855
}
856+
852857
break;
853858

854859
caseSourceCommand.Seek:
855860
longoffset=Native.zip_source_seek_compute_offset((UInt64)stream.Position,(UInt64)stream.Length,data,len,outerror);
856861
if(offset<0){
857862
returnoffset;
858863
}
859-
if(offset!=stream.Seek(offset,SeekOrigin.Begin)){
864+
if(!Seek(stream,SeekOrigin.Begin,offset)){
860865
return-1;
861866
}
862867
break;
@@ -962,12 +967,8 @@ internal static unsafe Int64 stream_callback (IntPtr state, IntPtr data, UInt64
962967

963968
return0;
964969

965-
boolSeekIfNeeded(Streams,SeekOriginorigin,longoffset)
970+
boolSeek(Streams,SeekOriginorigin,longoffset)
966971
{
967-
if(offset==0){
968-
returntrue;
969-
}
970-
971972
returns.Seek(offset,origin)==offset;
972973
}
973974
}

‎azure-pipelines.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ extends:
105105
archiveType: 7z
106106
replaceExistingArchive: true
107107
archiveFile: $(Build.ArtifactStagingDirectory)\libzip-windows-arm-x64.7z
108+
- script: |
109+
7z t $(Build.ArtifactStagingDirectory)\libzip-windows-arm-x64.7z
110+
7z t $(Build.ArtifactStagingDirectory)\libzip-windows-x64.7z
111+
7z t $(Build.ArtifactStagingDirectory)\libzip-windows-x86.7z
112+
displayName: Test Archives
108113
109114
- job: buildLinux
110115
pool:

0 commit comments

Comments
 (0)