Skip to content

Fix tests and ZipEntry DateTime Kind #502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 8, 2020
Merged

Fix tests and ZipEntry DateTime Kind #502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c027b25
Normalize DateTime kind in ZipEntry
piksel Aug 6, 2020
b9b2a0f
Use Unspecifed DateTime Kind for entries
piksel Aug 6, 2020
69bb9c0
Fix tests
piksel Aug 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ICSharpCode.SharpZipLib/Zip/ZipEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ public long DosTime
uint mon = Math.Max(1, Math.Min(12, ((uint)(value >> 21) & 0xf)));
uint year = ((dosTime >> 25) & 0x7f) + 1980;
int day = Math.Max(1, Math.Min(DateTime.DaysInMonth((int)year, (int)mon), (int)((value >> 16) & 0x1f)));
DateTime = new DateTime((int)year, (int)mon, day, (int)hrs, (int)min, (int)sec, DateTimeKind.Utc);
DateTime = new DateTime((int)year, (int)mon, day, (int)hrs, (int)min, (int)sec, DateTimeKind.Unspecified);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any mention of timezones in the appnote, so if it's unknown then unspecified seems a reasonable choice.

Copy link
Member Author

@piksel piksel Aug 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And more importantly, if you create a DateTime instance and specify that it is in UTC, then the OS will translate it to local time when using it to set the file attributes. If you use Unspecified it will not do any such translation.

(If the appnote DID mention it being in UTC, we could of course use the UTC-versions of the attribute setters instead).

}
}
}
Expand Down
35 changes: 33 additions & 2 deletions test/ICSharpCode.SharpZipLib.Tests/Zip/StreamHandling.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,14 @@ public void WriteZipStreamWithNoCompression([Values(0, 1, 256)] int contentLengt
using (var dummyZip = Utils.GetDummyFile(0))
using (var inputFile = Utils.GetDummyFile(contentLength))
{
// Filename is manually cleaned here to prevent this test from failing while ZipEntry doesn't automatically clean it
var inputFileName = ZipEntry.CleanName(inputFile.Filename);

using (var zipFileStream = File.OpenWrite(dummyZip.Filename))
using (var zipOutputStream = new ZipOutputStream(zipFileStream))
using (var inputFileStream = File.OpenRead(inputFile.Filename))
{
zipOutputStream.PutNextEntry(new ZipEntry(inputFile.Filename)
zipOutputStream.PutNextEntry(new ZipEntry(inputFileName)
{
CompressionMethod = CompressionMethod.Stored,
});
Expand All @@ -260,7 +263,6 @@ public void WriteZipStreamWithNoCompression([Values(0, 1, 256)] int contentLengt
{
var inputBytes = File.ReadAllBytes(inputFile.Filename);

var inputFileName = ZipEntry.CleanName(inputFile.Filename);
var entry = zf.GetEntry(inputFileName);
Assert.IsNotNull(entry, "No entry matching source file \"{0}\" found in archive, found \"{1}\"", inputFileName, zf[0].Name);

Expand All @@ -282,6 +284,35 @@ public void WriteZipStreamWithNoCompression([Values(0, 1, 256)] int contentLengt
}
}

[Test]
[Category("Zip")]
[Category("KnownBugs")]
public void ZipEntryFileNameAutoClean()
{
using (var dummyZip = Utils.GetDummyFile(0))
using (var inputFile = Utils.GetDummyFile()) {
using (var zipFileStream = File.OpenWrite(dummyZip.Filename))
using (var zipOutputStream = new ZipOutputStream(zipFileStream))
using (var inputFileStream = File.OpenRead(inputFile.Filename))
{
zipOutputStream.PutNextEntry(new ZipEntry(inputFile.Filename)
{
CompressionMethod = CompressionMethod.Stored,
});

inputFileStream.CopyTo(zipOutputStream);
}

using (var zf = new ZipFile(dummyZip.Filename))
{
Assert.AreNotEqual(ZipEntry.CleanName(inputFile.Filename), zf[0].Name,
"Entry file name \"{0}\" WAS automatically cleaned, this test should be removed", inputFile.Filename);
}

Assert.Warn("Entry file name \"{0}\" was not automatically cleaned", inputFile.Filename);
}
}

/// <summary>
/// Empty zips can be created and read?
/// </summary>
Expand Down
8 changes: 8 additions & 0 deletions test/ICSharpCode.SharpZipLib.Tests/Zip/WindowsNameTransformHandling.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@
using NUnit.Framework;
using System;
using System.IO;
using System.Runtime.InteropServices;

namespace ICSharpCode.SharpZipLib.Tests.Zip
{
[TestFixture]
public class WindowsNameTransformHandling : TransformBase
{
[OneTimeSetUp]
public void TestInit() {
if (Path.DirectorySeparatorChar != '\\') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth using the [Platform] attribute so that the tests only run on Windows?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, I didn't actually know about that one. I just figured feature detection was actually what was important anyway, instead of guessing based on the OS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what's best, just wondered if tests that involve drive letters might have issues as well as things with separators.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah maybe, it's kind of hard to test until any OS starts using drive letters and forward slashes though 😁

Assert.Inconclusive("WindowsNameTransform will not work on platforms not using '\\' directory separators");
}
}

[Test]
public void BasicFiles()
{
Expand Down
1 change: 1 addition & 0 deletions test/ICSharpCode.SharpZipLib.Tests/Zip/ZipEntryFactoryHandling.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public void CreatedValues()
var lastAccessTime = new DateTime(2050, 11, 3, 0, 42, 12);

string tempFile = Path.Combine(tempDir, "SharpZipTest.Zip");

using (FileStream f = File.Create(tempFile, 1024))
{
f.WriteByte(0);
Expand Down
14 changes: 10 additions & 4 deletions test/ICSharpCode.SharpZipLib.Tests/Zip/ZipNameTransformHandling.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,16 @@ public void NameTransforms()
[Category("Zip")]
public void FilenameCleaning()
{
Assert.AreEqual(0, string.Compare(ZipEntry.CleanName("hello"), "hello", StringComparison.Ordinal));
Assert.AreEqual(0, string.Compare(ZipEntry.CleanName(@"z:\eccles"), "eccles", StringComparison.Ordinal));
Assert.AreEqual(0, string.Compare(ZipEntry.CleanName(@"\\server\share\eccles"), "eccles", StringComparison.Ordinal));
Assert.AreEqual(0, string.Compare(ZipEntry.CleanName(@"\\server\share\dir\eccles"), "dir/eccles", StringComparison.Ordinal));
Assert.AreEqual(ZipEntry.CleanName("hello"), "hello");
if(Environment.OSVersion.Platform == PlatformID.Win32NT)
{
Assert.AreEqual(ZipEntry.CleanName(@"z:\eccles"), "eccles");
Assert.AreEqual(ZipEntry.CleanName(@"\\server\share\eccles"), "eccles");
Assert.AreEqual(ZipEntry.CleanName(@"\\server\share\dir\eccles"), "dir/eccles");
}
else {
Assert.AreEqual(ZipEntry.CleanName(@"/eccles"), "eccles");
}
}

[Test]
Expand Down