From 0297b2767f0c506761075935e215853b53151bc8 Mon Sep 17 00:00:00 2001 From: Paul Fenwick Date: Fri, 31 Oct 2014 01:25:40 +1100 Subject: [PATCH 1/2] Test case to show the presence of the text vs binary bug. For #205. --- CKAN/CKAN/ModuleInstaller.cs | 5 ++++- CKAN/Tests/CKAN/ModuleInstaller.cs | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CKAN/CKAN/ModuleInstaller.cs b/CKAN/CKAN/ModuleInstaller.cs index b2b1302821..cf6d02adf1 100644 --- a/CKAN/CKAN/ModuleInstaller.cs +++ b/CKAN/CKAN/ModuleInstaller.cs @@ -726,7 +726,10 @@ public static List FindInstallableFiles(CkanModule module, stri } } - private void CopyZipEntry(ZipFile zipfile, ZipEntry entry, string fullPath, bool makeDirs) + /// + /// Copy the entry from the opened zipfile to the path specified. + /// + internal static void CopyZipEntry(ZipFile zipfile, ZipEntry entry, string fullPath, bool makeDirs) { if (entry.IsDirectory) { diff --git a/CKAN/Tests/CKAN/ModuleInstaller.cs b/CKAN/Tests/CKAN/ModuleInstaller.cs index e32f912d08..e3a1ce2d73 100644 --- a/CKAN/Tests/CKAN/ModuleInstaller.cs +++ b/CKAN/Tests/CKAN/ModuleInstaller.cs @@ -96,6 +96,32 @@ public void No_Installable_Files() } } + [Test()] + // GH #205, make sure we write in *binary*, not text. + public void BinaryNotText_205() + { + string dogezip = Tests.TestData.DogeCoinFlagZip(); + ZipFile zipfile = new ZipFile(dogezip); + + ZipEntry entry = zipfile.GetEntry("DogeCoinFlag-1.01/GameData/DogeCoinFlag/Flags/dogecoin.png"); + string tmpfile = Path.GetTempFileName(); + + CKAN.ModuleInstaller.CopyZipEntry(zipfile, entry, tmpfile, false); + + long size = new System.IO.FileInfo(tmpfile).Length; + + try + { + // Compare recorded length against what we expect. + Assert.AreEqual(52043, size); + } + finally + { + // Tidy up. + File.Delete(tmpfile); + } + } + private void TestDogeCoinStanza(ModuleInstallDescriptor stanza) { Assert.AreEqual("GameData", stanza.install_to); From ee84dbd1868772d0db7d2edea14c487a08696727 Mon Sep 17 00:00:00 2001 From: Paul Fenwick Date: Fri, 31 Oct 2014 01:47:53 +1100 Subject: [PATCH 2/2] Make sure we write all our files in binary mode. - Supports transactions. - Includes tests. - Closes #205 --- CKAN/CKAN/ModuleInstaller.cs | 11 ++++++-- CKAN/Tests/CKAN/ModuleInstaller.cs | 44 ++++++++++++++++++++++++------ CKAN/Tests/Tests.csproj | 1 + 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/CKAN/CKAN/ModuleInstaller.cs b/CKAN/CKAN/ModuleInstaller.cs index cf6d02adf1..f3608aa63a 100644 --- a/CKAN/CKAN/ModuleInstaller.cs +++ b/CKAN/CKAN/ModuleInstaller.cs @@ -6,6 +6,7 @@ using System.Security.Cryptography; using System.Text.RegularExpressions; using System.Threading; +using ICSharpCode.SharpZipLib.Core; using ICSharpCode.SharpZipLib.Zip; using System.Transactions; using ChinhDo.Transactions; @@ -755,11 +756,17 @@ internal static void CopyZipEntry(ZipFile zipfile, ZipEntry entry, string fullPa file_transaction.CreateDirectory(directory); } + // Snapshot whatever was there before. If there's nothing, this will just + // remove our file on rollback. + file_transaction.Snapshot(fullPath); + // It's a file! Prepare the streams using (Stream zipStream = zipfile.GetInputStream(entry)) - using (StreamReader reader = new StreamReader(zipStream)) + using (FileStream writer = File.Create(fullPath)) { - file_transaction.WriteAllText(fullPath, reader.ReadToEnd()); + // 4k is the block size on practically every disk and OS. + byte[] buffer = new byte[4096]; + StreamUtils.Copy(zipStream, writer, buffer); } } diff --git a/CKAN/Tests/CKAN/ModuleInstaller.cs b/CKAN/Tests/CKAN/ModuleInstaller.cs index e3a1ce2d73..4547bce264 100644 --- a/CKAN/Tests/CKAN/ModuleInstaller.cs +++ b/CKAN/Tests/CKAN/ModuleInstaller.cs @@ -1,6 +1,7 @@ using NUnit.Framework; using System; using System.IO; +using System.Transactions; using System.Collections.Generic; using ICSharpCode.SharpZipLib.Zip; using CKAN; @@ -100,14 +101,9 @@ public void No_Installable_Files() // GH #205, make sure we write in *binary*, not text. public void BinaryNotText_205() { - string dogezip = Tests.TestData.DogeCoinFlagZip(); - ZipFile zipfile = new ZipFile(dogezip); - - ZipEntry entry = zipfile.GetEntry("DogeCoinFlag-1.01/GameData/DogeCoinFlag/Flags/dogecoin.png"); - string tmpfile = Path.GetTempFileName(); - - CKAN.ModuleInstaller.CopyZipEntry(zipfile, entry, tmpfile, false); - + // Use CopyZipEntry (via CopyDogeFromZip) and make sure it + // comes out the right size. + string tmpfile = CopyDogeFromZip(); long size = new System.IO.FileInfo(tmpfile).Length; try @@ -122,6 +118,38 @@ public void BinaryNotText_205() } } + [Test()] + // Make sure when we roll-back a transaction, files written with CopyZipEntry go + // back to their pre-transaction state. + public void FileSysRollBack() + { + string file; + + using (var scope = new TransactionScope()) + { + file = CopyDogeFromZip(); + Assert.IsTrue(new System.IO.FileInfo(file).Length > 0); + scope.Dispose(); // Rollback + } + + // CopyDogeFromZip creates a tempfile, so we check to make sure it's empty + // again on transaction rollback. + Assert.AreEqual(0, new System.IO.FileInfo(file).Length); + } + + private static string CopyDogeFromZip() + { + string dogezip = Tests.TestData.DogeCoinFlagZip(); + ZipFile zipfile = new ZipFile(dogezip); + + ZipEntry entry = zipfile.GetEntry("DogeCoinFlag-1.01/GameData/DogeCoinFlag/Flags/dogecoin.png"); + string tmpfile = Path.GetTempFileName(); + + CKAN.ModuleInstaller.CopyZipEntry(zipfile, entry, tmpfile, false); + + return tmpfile; + } + private void TestDogeCoinStanza(ModuleInstallDescriptor stanza) { Assert.AreEqual("GameData", stanza.install_to); diff --git a/CKAN/Tests/Tests.csproj b/CKAN/Tests/Tests.csproj index e560f9fcd0..e70028958a 100644 --- a/CKAN/Tests/Tests.csproj +++ b/CKAN/Tests/Tests.csproj @@ -39,6 +39,7 @@ ..\packages\SharpZipLib.0.86.0\lib\20\ICSharpCode.SharpZipLib.dll +