Skip to content

Conversation

@menees
Copy link
Owner

@menees menees commented Dec 13, 2017

I changed SftpFileAttributes to preserve the last access and modified times as UTC values rather than always forcing a local conversion. This primarily affected the FromBytes(SshDataStream) method, but the changes cascade out from there to make sure that SftpFileAttributes and SftpFile can be used without ever forcing a (potentially lossy) conversion to local time. In the SSH data stream the atime and mtime values are "Unix times" in seconds, so they're already in UTC. We need to preserve that fact by returning and storing DateTime values with Kind == Utc.

Previously, the code called DateTime.FromFileTime. Internally, FromFileTime calls DateTime.ToLocalTime(), which is a lossy function. For example, any local time >= 1am and < 2am during the "fall back" hour of a daylight saving time shift in the fall could have come from two different UTC times. Avoiding this ambiguity is why file systems store times in UTC, so the SFTP classes need to preserve UTC times too. Here's a simple example to see this in VS's C# interactive window:

TimeZoneInfo central = TimeZoneInfo.FindSystemTimeZoneById("Central Standard Time");
DateTime first = new DateTime(2017, 11, 5, 6, 0, 0, DateTimeKind.Utc);
DateTime second = new DateTime(2017, 11, 5, 7, 0, 0, DateTimeKind.Utc);
Console.WriteLine(TimeZoneInfo.ConvertTimeFromUtc(first, central));
Console.WriteLine(TimeZoneInfo.ConvertTimeFromUtc(second, central));
// Both UTC times convert to the same local time: 11/5/2017 1:00:00 AM

.NET also has an issue where it caches the current local time zone offset, so it can incorrectly calculate local times after a DST shift if nothing has called CultureInfo.ClearCachedData or TimeZoneInfo.ClearCachedData. This is discussed more at https://stackoverflow.com/a/297189/1882616, and it can be a big problem in long-running services if not handled correctly. By making SSH.NET internally preserve UTC times correctly, the library can be immune to problems with DST shifts for callers that only work with UTC times.

I also cleaned up some warnings in the unit tests and added an .editorconfig file so VS 2017 will know to use spaces for indentation in this solution (even though my global preference is set to tabs).

@menees menees merged commit 8d24450 into develop Dec 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants