From 63ff7368c4adc7c9966d2378dade4a67322de366 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 17 Jan 2022 11:36:22 +0000 Subject: [PATCH] Fix corrupt data when Deleting entries from zip files. Fixes #102 The initial report showed the following error ``` Unhandled exception. System.IO.IOException: An attempt was made to move the position before the beginning of the stream. at System.IO.MemoryStream.Seek(Int64 offset, SeekOrigin loc) at Xamarin.Tools.Zip.ZipArchive.stream_callback(IntPtr state, IntPtr data, UInt64 len, SourceCommand cmd) ``` The really odd thing was that this only happened if the example files were added to the archive in a certain order. This pointed to some kind of memory corruption or stream offset issue. Digging into the documentation for [zip_source_function]](https://libzip.org/documentation/zip_source_function.html#DESCRIPTION) we can being to see what the problem is. Our current implementation of this user callback treats the SourceCommand's of `ZIP_SOURCE_BEGIN_WRITE` `ZIP_SOURCE_COMMIT_WRITE`, `ZIP_SOURCE_TELL_WRITE` and `ZIP_SOURCE_SEEK_WRITE` in the same way as their non `WRITE` counter parts. In that when we get a `ZIP_SOURCE_SEEK_WRITE` we seek in the current archive `Stream` to the appropriate location. Similarly `ZIP_SOURCE_BEGIN_WRITE` seeks to the start of the current archive `Stream`. This implementation is incorrect. The documentation for `ZIP_SOURCE_BEGIN_WRITE` is as follows ``` Prepare the source for writing. Use this to create any temporary file(s). ``` This suggests that a new temporary file is expected to be created. Also if we look at the following descriptions ZIP_SOURCE_TELL Return the current read offset in the source, like ftell(3). ZIP_SOURCE_TELL_WRITE Return the current write offset in the source, like ftell(3). We can see that there are supposed to be two different offsets. One for reading and one for writing. This leads us to the reason why this problem was occurring. Because both the `READ` and `WRITE` are operating on the exact same `Stream` were are getting into a position where the original data was being overwritten by us deleting an entry. What we should have been doing was creating a temp `Stream` when we got the `ZIP_SOURCE_BEGIN_WRITE` SourceCommand. Then use this temp `Stream` to write all the required changes to before finally overwriting the original `Stream` when we get the `ZIP_SOURCE_COMMIT_WRITE` SourceCommand. This will ensure that the original archive data will remain intact until all the processing is done. So rather than passing in a `GCHandle` to the `Stream` directly , a new class has been introduced. The `CallbackContext` class will be used to pass data between the managed and unmanaged code. It will contain the properties for the `Source` stream as well as `Destination` stream. The `Source` will always be used for reading, the `Destination` (if present) will be used for writing. Now on `ZIP_SOURCE_BEGIN_WRITE` we create a new temp file stream which we will use to create the updated zip file. This new stream will be stored in the `CallbackContext.Destination` property so that all the other `WRITE` based commands will work on it. Finally when we get the `ZIP_SOURCE_COMMIT_WRITE` SourceCommand, we will copy the data to the `CallbackContext.Source` stream. Then finally we will dispose of the `CallbackContext.Destination`. --- .vscode/settings.json | 1 - .../LibZipSharp.UnitTest.csproj | 9 ++ LibZipSharp.UnitTest/ZipTests.cs | 48 ++++++++++ LibZipSharp.UnitTest/characters_players.json | 3 + LibZipSharp.UnitTest/info.json | 21 +++++ LibZipSharp.UnitTest/object_spawn.json | 30 +++++++ LibZipSharp.props | 2 +- LibZipSharp/Xamarin.Tools.Zip/Native.cs | 2 +- LibZipSharp/Xamarin.Tools.Zip/ZipArchive.cs | 87 ++++++++++++++++--- 9 files changed, 186 insertions(+), 17 deletions(-) create mode 100644 LibZipSharp.UnitTest/characters_players.json create mode 100644 LibZipSharp.UnitTest/info.json create mode 100644 LibZipSharp.UnitTest/object_spawn.json diff --git a/.vscode/settings.json b/.vscode/settings.json index b89b4b1c..bb9e1222 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,4 @@ { - "nxunitExplorer.nunit": "/Users/dean/.nuget/packages/nunit.consolerunner/3.10.0/tools/nunit3-console.exe", "nxunitExplorer.modules": [ "LibZipSharp.UnitTest/bin/Debug/net471/LibZipSharp.UnitTest.dll", ] diff --git a/LibZipSharp.UnitTest/LibZipSharp.UnitTest.csproj b/LibZipSharp.UnitTest/LibZipSharp.UnitTest.csproj index be5bb1d6..18f619f5 100644 --- a/LibZipSharp.UnitTest/LibZipSharp.UnitTest.csproj +++ b/LibZipSharp.UnitTest/LibZipSharp.UnitTest.csproj @@ -42,6 +42,15 @@ PreserveNewest + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + diff --git a/LibZipSharp.UnitTest/ZipTests.cs b/LibZipSharp.UnitTest/ZipTests.cs index 39ea5956..f5b2b2fd 100644 --- a/LibZipSharp.UnitTest/ZipTests.cs +++ b/LibZipSharp.UnitTest/ZipTests.cs @@ -257,6 +257,54 @@ public void CheckForUnknownCompressionMethods () } } + [Test] + public void InMemoryZipFile () + { + string filePath = Path.GetFullPath ("info.json"); + if (!File.Exists (filePath)) { + filePath = Path.GetFullPath (Path.Combine ("LibZipSharp.UnitTest", "info.json")); + } + string fileRoot = Path.GetDirectoryName (filePath); + using (var stream = new MemoryStream ()) { + using (var zip = ZipArchive.Create (stream)) { + zip.AddFile (filePath, "info.json"); + zip.AddFile (Path.Combine (fileRoot, "characters_players.json"), "characters_players.json"); + zip.AddFile (Path.Combine (fileRoot, "object_spawn.json"), "object_spawn.json"); + } + + stream.Position = 0; + using (var zip = ZipArchive.Open (stream)) { + Assert.AreEqual (3, zip.EntryCount); + foreach (var e in zip) { + Console.WriteLine (e.FullName); + } + zip.DeleteEntry ("info.json"); + } + + stream.Position = 0; + using (var zip = ZipArchive.Open (stream)) { + Assert.AreEqual (2, zip.EntryCount); + zip.AddEntry ("info.json", File.ReadAllText (filePath), Encoding.UTF8, CompressionMethod.Deflate); + } + + stream.Position = 0; + using (var zip = ZipArchive.Open (stream)) { + Assert.AreEqual (3, zip.EntryCount); + Assert.IsTrue (zip.ContainsEntry ("info.json")); + var entry1 = zip.ReadEntry ("info.json"); + using (var s = new MemoryStream ()) { + entry1.Extract (s); + s.Position = 0; + using (var sr = new StreamReader (s)) { + var t = sr.ReadToEnd (); + Assert.AreEqual (File.ReadAllText (filePath), t); + } + + } + } + } + } + [TestCase (false)] [TestCase (true)] public void EnumerateSkipDeletedEntries (bool deleteFromExistingFile) diff --git a/LibZipSharp.UnitTest/characters_players.json b/LibZipSharp.UnitTest/characters_players.json new file mode 100644 index 00000000..443d47cf --- /dev/null +++ b/LibZipSharp.UnitTest/characters_players.json @@ -0,0 +1,3 @@ +{ + "players": [] +} \ No newline at end of file diff --git a/LibZipSharp.UnitTest/info.json b/LibZipSharp.UnitTest/info.json new file mode 100644 index 00000000..864a321e --- /dev/null +++ b/LibZipSharp.UnitTest/info.json @@ -0,0 +1,21 @@ +{ + "version": 22, + "type": "Object", + "id": "c9bb1a03-cb8a-4f9f-b298-49ee26fa1fd9", + "ownerId": "47b9d124-6421-4055-abf4-b5fc8fbfe784", + "name": "OBJECT NAME", + "description": "", + "blockCount": 9, + "created": "2019-06-21T14:20:42.000Z", + "lastChanged": "0001-01-01T00:00:00.000Z", + "tags": [ + { + "value": "some_tag", + "origin": "category" + }, + { + "value": "some_other_tag", + "origin": "category" + } + ] +} diff --git a/LibZipSharp.UnitTest/object_spawn.json b/LibZipSharp.UnitTest/object_spawn.json new file mode 100644 index 00000000..940507c0 --- /dev/null +++ b/LibZipSharp.UnitTest/object_spawn.json @@ -0,0 +1,30 @@ +{ + "spawnRotationOffset": + { + "pitch": -16.274194717407227, + "yaw": -34.644023895263672, + "roll": 65.772232055664063 + }, + "spawnScaleOffset": + { + "x": 0.52916890382766724, + "y": 0.52916890382766724, + "z": 0.52916890382766724 + }, + "bounds": + { + "origin": + { + "x": -9086.74609375, + "y": -6475.60498046875, + "z": 1128.5997314453125 + }, + "boxExtent": + { + "x": 400.17755126953125, + "y": 474.9906005859375, + "z": 502.65240478515625 + }, + "sphereRadius": 530.2637939453125 + } +} \ No newline at end of file diff --git a/LibZipSharp.props b/LibZipSharp.props index 22ce3b43..80dc1906 100644 --- a/LibZipSharp.props +++ b/LibZipSharp.props @@ -1,6 +1,6 @@ - <_LibZipSharpAssemblyVersion>2.0.1 + <_LibZipSharpAssemblyVersion>2.0.2