DST handling in client

[btriffles]

I believe it is known that FileZilla Client has problems with file times being shown with an incorrect 1-hour offset at times due to DST (at least on Windows). After doing some research, I think I can help solve the problem.

As best I understand, the use of the Windows API function FileTimeToLocalFileTime leads to the bug when the current DST state doesn't match the DST state at the time the file was modified. The documentation for FileTimeToLocalFileTime says to use three different functions (FileTimeToSystemTime, SystemTimeToTzSpecificLocalTime, SystemTimeToFileTime) to avoid this problem. I believe this is done in Windows Explorer for Windows 7 and up.

It looks like FileZilla uses this function in /src/engine/local_filesys.cpp. In addition, the code here appears to be mostly copied from wxWidgets. There was a patch to the corresponding code in wxWidgets that supposedly solves the problem by avoiding the use of FileTimeToLocalFileTime entirely. If you can apply an equivalent change to FileZilla, I think it might fix the bug. The change, which looks rather simple, is described in the following wxWidgets ticket and changeset:
http://trac.wxwidgets.org/ticket/13098
http://trac.wxwidgets.org/changeset/74423

I can't test this fix myself, but I'm hoping that it solves the problem without requiring wxWidgets to be updated or figuring out how to call more Windows API functions. I didn't see any other places in FileZilla's source where FileTimeToLocalFileTime was used.

[botg]

Thanks, I'll have a look.

I wonder, why did I copy that code from wx in the first place?

Edit: I remember now, wx' own ConvertWxToFileTime lacks error handling and does this slow wxLogLastError instead.

[botg]

A fix has been applied.

[btriffles]

Thanks for the quick response and potential fix! I'll give it a try once there is a working build. Right now, it looks like the nightly build failed due to a type conversion issue in the modified code.

[btriffles]

After testing the 2013-12-02 nightly build, it appears that FileZilla Client now shows the correct time for the local file pane. Hooray!

Unfortunately, the beast that is DST is not yet slain. When attempting to upload a file (to FileZilla Server) using the MFMT command, the wrong DST setting is still used. In particular:
-The "target file already exists" dialog shows the old, incorrect time
-The MFMT command in the client message log shows the incorrect time
-The time is thus set wrong on the server

I don't have a development environment, but I suspect this is at least partially caused by the use of wxFileName::GetModificationTime in ftpcontrolsocket.cpp. GetModificationTime calls the code in wxWidgets that the earlier changeset fixed, so it has the same DST bug. There appears to be a few other uses of GetModificationTime in FileZilla, so it would be a good idea to update your copy of wxWidgets if/when possible (at least with the changeset). Alternatively, you could avoid the use of wxWidgets here as you did for the local file pane.

[botg]

Fixed in the repository, will be in the next nightly.

[btriffles]

Thanks for the continuing fast and amazing work!

When uploading a file, the correct date now appears to be sent to the server. Great!
However, I notice that the "target file already exists" dialog still shows the incorrect time (a cosmetic issue). I think if you update the file exists dialog to use the same local format function (CTimeFormat::FormatDateTime?) as the local list view, this may fix the problem. In addition, I think this will have the added bonus of making the dialog respect the date/time format specified in FileZilla's interface settings.

Continuing my testing, I see that the file date appears to be set incorrectly when downloading a file to the local file system. I think FileZilla is using wxFileName::SetTimes to set the modification date/time, which suffers from the same wxWidgets bug (or rather a complementary version) that was fixed by the originally mentioned changeset. Like before, you can update or apply the changeset to your copy of wxWidgets. Alternatively, you can make your own version of ConvertWxToFileTime and use the Windows SetFileTime function to set the date.

[botg]

Does your server support MLSD, or is the fallback to the old LIST command being used?

Like before, you can update or apply the changeset to your copy of wxWidgets.

Updating to wx 3 requires major refactoring and testing, the changes are non-trivial. Applying the changeset to my copy of wx doesn't quite help others building FileZilla

[botg]

Fixed setting the local timestamps.

As for the file exists dialog, that one I cannot reproduce. (LIST vs MLSD?)

[btriffles]

I can understand why upgrading or patching wxWidgets would be problematic, and I think your solution is completely reasonable.

My message log indicates that MLSD is being used, but since the server is FileZilla Server (0.9.36), you should know better than me
Regardless, the incorrect time is only shown for the local file, so I don't think it involves the server. I apologize for not being clearer.

Like the other DST bugs, the file exists dialog only shows an incorrect hour offset for a local file under certain conditions. I think the following conditions will help you replicate the bug:
-Windows is set to a time zone that uses DST.
-The local file was modified on a date on which the DST status was different from the current DST status. For example, I see the bug with a file modified on 2013-10-09, which is on the other side of my locale's DST status transition on 2013-11-03.

Looking deeper into the issue, I think the bug may be caused by Microsoft's implementation of stat(). Assuming I am understanding the code correctly (in my trusty copy of wordpad.exe), it looks like FileZilla gets the local date for the dialog by calling wxStat() in ControlSocket.cpp. Further assuming that wxStat() calls Microsoft's implementation of stat(), Microsoft's function supposedly handles DST incorrectly:
http://stackoverflow.com/questions/1980 ... ht-savings
If all this is true, I suppose a solution would be to use something like your CLocalFileSystem::GetModificationTime function instead of wxStat() to obtain the modification date.

[botg]

since the server is FileZilla Server (0.9.36), you should know better than me

Dunno, it's an old version I know nothing about anymore.

Like the other DST bugs, the file exists dialog only shows an incorrect hour offset for a local file under certain conditions. I think the following conditions will help you replicate the bug:
-Windows is set to a time zone that uses DST.
-The local file was modified on a date on which the DST status was different from the current DST status. For example, I see the bug with a file modified on 2013-10-09, which is on the other side of my locale's DST status transition on 2013-11-03.

There must be yet another condition, I'm still not seeing it with those two met.

Looking deeper into the issue, I think the bug may be caused by Microsoft's implementation of stat(). Assuming I am understanding the code correctly (in my trusty copy of wordpad.exe), it looks like FileZilla gets the local date for the dialog by calling wxStat() in ControlSocket.cpp. Further assuming that wxStat() calls Microsoft's implementation of stat(), Microsoft's function supposedly handles DST incorrectly:
http://stackoverflow.com/questions/1980 ... ht-savings
If all this is true, I suppose a solution would be to use something like your CLocalFileSystem::GetModificationTime function instead of wxStat() to obtain the modification date.[/quote]

Looks like stat() also behaves differently depending on the used mingw version.

Edit: Got rid of wxStat, try the next nightly.

[btriffles]

Apparently the bug in Microsoft's stat() also depends on other conditions, such as the file system being used. There may also be differences in the version of the C run-time library being used on your computer. I think MinGW just calls Microsoft's C run-time library, and it's probably dynamically linked to whatever version is on your computer.

Anyways, your latest changes appear to be very effective. The file exists dialog shows the correct time, and downloaded files are now set to the correct time. Huzzah!

I need more time for testing, but I did notice a remaining bug. I have a few files modified in 2004-2006 (and near the dates of DST transitions) for which FileZilla is showing an hour offset (both locally and remotely) from Windows Explorer. However, the files show the correct dates in Windows Explorer after being uploaded or downloaded, which suggests that FileZilla is getting/setting the correct time (in UTC). There just seems to be some difference in the methods that FileZilla uses to convert UTC times for local display and the methods that Windows (7 and Server 2008 R2) uses. I'll look into this issue when I have more time.

Thanks again for all your excellent work! And for indulging my unhealthy obsession with metadata!

[btriffles]

It looks like FileZilla is applying the current rules for the DST transition dates, even if the file was modified at a time during which the DST transition dates were different. For example, US law in 2006 stated that DST begins on the first Sunday of April. However, current US law states that DST begins on the second Sunday of March. If I date a file to 2006, FileZilla displays the DST transition on the second Sunday of March (March 12), not on the first Sunday of April (April 2).

The issue is extremely hard to trace, but my current belief is that it's caused by a bug in Microsoft's implementation of localtime():
http://www.nntp.perl.org/group/perl.per ... 21888.html
It looks like FileZilla calls wxDateTime::Format(), which appears to use localtime() to convert from UTC to the local time zone.

If my assessment is correct, the bug should probably be reported for fixing to wxWidgets. However, a potential immediate solution for FileZilla would be to once again avoid the use of wxWidgets. In particular, I think you could call the three functions in the Remarks section of the following page to perform the conversion from UTC to local time:
http://msdn.microsoft.com/en-us/library ... s.85).aspx
Then you can send the output (after conversion) to strftime() to mimic wxDateTime::Format().

I hope that's not too messy for you. With luck, this may be the last DST bug in FileZilla (although it's really Microsoft's fault!).

[boco]

DST = BS

[botg]

Indeed. Only the general concept of timezones is worse.