Fix tests and ZipEntry DateTime Kind

[piksel]

This should allow all tests to pass on both *nix and windows.

The current outstanding bug with ZipEntry file names not being automatically cleaned has been moved to it's own test, as this is a known (although unwanted) behaviour of the current state the code base is in.

By doing this we should be able to have failing tests prevent PRs from being merged in a more controlled manner.

Fixes #481

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

[Numpsy]

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

[piksel]

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.

[Numpsy]

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

[piksel]

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

[Numpsy]

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

[piksel]

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).

[bastianeicher]

TL;DR: The DateTimeKind.Unspecified change looks good to me.

Adding files to archives

When adding files to ZIP archives using ZipFile.Add() or FastZip the following code handles recording timestamps:

SharpZipLib/src/ICSharpCode.SharpZipLib/Zip/ZipEntryFactory.cs

Lines 221 to 253 in b5898dc

	switch (timeSetting_)
	{
	case TimeSetting.CreateTime:
	result.DateTime = fi.CreationTime;
	break;
	case TimeSetting.CreateTimeUtc:
	result.DateTime = fi.CreationTimeUtc;
	break;
	case TimeSetting.LastAccessTime:
	result.DateTime = fi.LastAccessTime;
	break;
	case TimeSetting.LastAccessTimeUtc:
	result.DateTime = fi.LastAccessTimeUtc;
	break;
	case TimeSetting.LastWriteTime:
	result.DateTime = fi.LastWriteTime;
	break;
	case TimeSetting.LastWriteTimeUtc:
	result.DateTime = fi.LastWriteTimeUtc;
	break;
	case TimeSetting.Fixed:
	result.DateTime = fixedDateTime_;
	break;
	default:
	throw new ZipException("Unhandled time setting in MakeFileEntry");
	}

The default value for the TimeSetting enum is LastWriteTime:

SharpZipLib/src/ICSharpCode.SharpZipLib/Zip/ZipEntryFactory.cs

Lines 17 to 58 in b5898dc

	public enum TimeSetting
	{
	/// <summary>
	/// Use the recorded LastWriteTime value for the file.
	/// </summary>
	LastWriteTime,
	/// <summary>
	/// Use the recorded LastWriteTimeUtc value for the file
	/// </summary>
	LastWriteTimeUtc,
	/// <summary>
	/// Use the recorded CreateTime value for the file.
	/// </summary>
	CreateTime,
	/// <summary>
	/// Use the recorded CreateTimeUtc value for the file.
	/// </summary>
	CreateTimeUtc,
	/// <summary>
	/// Use the recorded LastAccessTime value for the file.
	/// </summary>
	LastAccessTime,
	/// <summary>
	/// Use the recorded LastAccessTimeUtc value for the file.
	/// </summary>
	LastAccessTimeUtc,
	/// <summary>
	/// Use a fixed value.
	/// </summary>
	/// <remarks>The actual <see cref="DateTime"/> value used can be
	/// specified via the <see cref="ZipEntryFactory(DateTime)"/> constructor or
	/// using the <see cref="ZipEntryFactory(TimeSetting)"/> with the setting set
	/// to <see cref="TimeSetting.Fixed"/> which will use the <see cref="DateTime"/> when this class was constructed.
	/// The <see cref="FixedDateTime"/> property can also be used to set this value.</remarks>
	Fixed,
	}

This means by default the FileInfo.CreationTime property is read. This property provides a timestamp in local time (DateTimeKind.Local).

If TimeSetting is set to LastWriteTimeUtc the FileInfo.CreationTimeUtc property is read, which provides a timestamp in UTC time (DateTimeKind.Utc).

Extracting files from archives

When extracting files from ZIP archives using FastZip the following code handles restoring timestamps:

SharpZipLib/src/ICSharpCode.SharpZipLib/Zip/FastZip.cs

Lines 640 to 643 in b5898dc

	if (restoreDateTimeOnExtract_)
	{
	File.SetLastWriteTime(targetName, entry.DateTime);
	}

The following table shows how File.SetLastWriteTime() (used by SharpZipLib) and File.SetLastWriteTimeUtc() (not used by SharpZipLib) interpret DateTimes with various DateTimeKinds:

	File.SetLastWriteTime()	File.SetLastWriteTimeUtc()
DateTimeKind.Unspecified	Local	UTC
DateTimeKind.Local	Local	Local
DateTimeKind.Utc	UTC	UTC

Impact of proposed change

This PR changes the following code to use DateTimeKind.Unspecified instead of DateTimeKind.Utc when reading timestamps from DOS time:

SharpZipLib/src/ICSharpCode.SharpZipLib/Zip/ZipEntry.cs

Line 745 in b5898dc

DateTime = new DateTime((int)year, (int)mon, day, (int)hrs, (int)min, (int)sec, DateTimeKind.Utc);

Because the default behavior of the library is to capture timestamps as local time and DateTimeKind.Unspecified gets treated a s local time when writing files, this change allows timestamps to be properly round-tripped without shifting. This sounds like a good solution to me.

An alternative would be to use DateTimeKind.Local instead to make this more explicit. I would advise against that though, since when reading a ZIP archive its impossible to know whether its timestamps were creating from local or UTC time, so DateTimeKind.Unspecified seems more honest. It also allows consumers of SharpZipLib that don't use FastZip to control which behavior they want by using either File.SetLastWriteTime() or File.SetLastWriteTimeUtc().

[piksel]

Great summary!