Skip to content
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.

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

Fix LastWriteTime and LastAccessTime of a symlink on Windows and Unix #52639

Merged
merged 24 commits into from
Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
270c526
Implement most of the fix for #38824
hamarb123 May 12, 2021
50d0b47
Merge remote-tracking branch 'upstream/main' into fixfor38824
hamarb123 Oct 20, 2021
952a168
Most of the code for PR #52639 to fix #38824
hamarb123 Oct 20, 2021
abdbbcb
Remove additional FILE_FLAG_OPEN_REPARSE_POINT
hamarb123 Oct 21, 2021
7dc1875
Add missing override keywords
hamarb123 Oct 21, 2021
41d23e2
Fix access modifiers
hamarb123 Oct 21, 2021
f603c3c
Merge remote-tracking branch 'upstream/main' into fixfor38824
hamarb123 Oct 21, 2021
0a87835
Add more symlink tests, rearrange files
hamarb123 Oct 26, 2021
606130b
Merge remote-tracking branch 'upstream/main' into fixfor38824
hamarb123 Oct 26, 2021
f8c4fd6
Fix return type of CreateSymlink in File/GetSetTimes.cs
hamarb123 Oct 26, 2021
060bf1c
Remove browser from new symlink tests as it doesn't support creation …
hamarb123 Oct 26, 2021
f4bac0c
Use lutimes, improve code readability, simplify tests
hamarb123 Oct 28, 2021
97d1ed3
Change year in test to 2014 to reduce diff
hamarb123 Oct 28, 2021
fd9d2d5
Rename symlink tests, use 1 core symlink times function, and check th…
hamarb123 Oct 28, 2021
1b7b868
Inline RunSymlinkTestPart 'function'
hamarb123 Oct 28, 2021
b8460ff
Share CreateSymlinkToItem call in tests and update comment for clarity
hamarb123 Oct 28, 2021
9bf86db
Update symlink time tests
hamarb123 Oct 29, 2021
5713e27
Remove unnecessary Assert.All
hamarb123 Oct 29, 2021
d50be1b
Changes to SettingUpdatesPropertiesOnSymlink test
hamarb123 Oct 30, 2021
d61060b
Merge remote-tracking branch 'upstream/main' into fixfor38824
hamarb123 Oct 30, 2021
a894d87
Remove unnecessary fsi.Refresh()
hamarb123 Oct 31, 2021
d8bff21
Updates to test and pal_time.c
hamarb123 Nov 5, 2021
3b37666
Merge remote-tracking branch 'upstream/main' into fixfor38824
hamarb123 Nov 5, 2021
569a24f
Remove trailing space
hamarb123 Nov 5, 2021
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: 2 additions & 0 deletions src/libraries/Common/src/Interop/OSX/Interop.libc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ internal struct AttrList

[DllImport(Libraries.libc, EntryPoint = "setattrlist", SetLastError = true)]
internal static unsafe extern int setattrlist(string path, AttrList* attrList, void* attrBuf, nint attrBufSize, CULong options);

internal const uint FSOPT_NOFOLLOW = 0x00000001;
}
}
2 changes: 1 addition & 1 deletion src/libraries/Native/Unix/System.Native/pal_time.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ int32_t SystemNative_UTimensat(const char* path, TimeSpec* times)

updatedTimes[1].tv_sec = (time_t)times[1].tv_sec;
updatedTimes[1].tv_nsec = (long)times[1].tv_nsec;
while (CheckInterrupted(result = utimensat(AT_FDCWD, path, updatedTimes, 0)));
while (CheckInterrupted(result = utimensat(AT_FDCWD, path, updatedTimes, AT_SYMLINK_NOFOLLOW)));
#else
struct timeval updatedTimes[2];
updatedTimes[0].tv_sec = (long)times[0].tv_sec;
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
35 changes: 35 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public abstract class BaseGetSetTimes<T> : FileSystemTest

protected abstract T GetExistingItem();
protected abstract T GetMissingItem();
protected abstract T CreateSymlinkToItem(T item);

protected abstract string GetItemPath(T item);

Expand Down Expand Up @@ -127,6 +128,40 @@ public void SettingUpdatesPropertiesAfterAnother()
});
}

[Fact]
[PlatformSpecific(~(TestPlatforms.Browser))] // Browser is excluded as there is only 1 effective time store.
public void SettingUpdatesPropertiesOnSymlink()
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
// This test is in this class since it needs all of the time functions.
// This test makes sure that the times are set on the symlink itself.
// It is needed as on OSX for example, the default for most APIs is
// to follow the symlink to completion and set the time on that entry
// instead (eg. the setattrlist will do this without the flag set).
// It is also the same case on unix, with the utimensat function.
T item = CreateSymlinkToItem(GetExistingItem());

Assert.All(TimeFunctions(requiresRoundtripping: true), (function) =>
{
// Checking that milliseconds are not dropped after setter on supported platforms.
DateTime dt = new DateTime(2004, 12, 9, 12, 3, 3, LowTemporalResolution ? 0 : 321, function.Kind);
function.Setter(item, dt);
DateTime result = function.Getter(item);
Assert.Equal(dt, result);
Assert.Equal(dt.ToLocalTime(), result.ToLocalTime());

// File and Directory UTC APIs treat a DateTimeKind.Unspecified as UTC whereas
// ToUniversalTime treats it as local.
if (function.Kind == DateTimeKind.Unspecified)
{
Assert.Equal(dt, result.ToUniversalTime());
}
else
{
Assert.Equal(dt.ToUniversalTime(), result.ToUniversalTime());
}
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
});
}

[Fact]
public void CanGetAllTimesAfterCreation()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,7 @@ public override IEnumerable<TimeFunction> TimeFunctions(bool requiresRoundtrippi
((path) => Directory.GetLastWriteTimeUtc(path)),
DateTimeKind.Utc);
}

protected override string CreateSymlinkToItem(string item) => Directory.CreateSymbolicLink(item + ".link", item).FullName;
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,7 @@ public override IEnumerable<TimeFunction> TimeFunctions(bool requiresRoundtrippi
((testDir) => testDir.LastWriteTimeUtc),
DateTimeKind.Utc);
}

protected override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) => (DirectoryInfo)Directory.CreateSymbolicLink(item.FullName + ".link", item.FullName);
}
}
2 changes: 2 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ public override IEnumerable<TimeFunction> TimeFunctions(bool requiresRoundtrippi
DateTimeKind.Utc);
}

protected override string CreateSymlinkToItem(string item) => File.CreateSymbolicLink(item + ".link", item).FullName;

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInAppContainer))] // Can't read root in appcontainer
[PlatformSpecific(TestPlatforms.Windows)]
public void PageFileHasTimes()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ public override IEnumerable<TimeFunction> TimeFunctions(bool requiresRoundtrippi
DateTimeKind.Utc);
}

protected override FileInfo CreateSymlinkToItem(FileInfo item) => (FileInfo)File.CreateSymbolicLink(item.FullName + ".link", item.FullName);

[ConditionalFact(nameof(HighTemporalResolution))]
public void CopyToMillisecondPresent()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ private unsafe Interop.Error SetCreationTimeCore(string path, long seconds, long
attrList.commonAttr = Interop.libc.AttrList.ATTR_CMN_CRTIME;

Interop.Error error =
Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong)) == 0 ?
Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), new CULong(Interop.libc.FSOPT_NOFOLLOW)) == 0 ?
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
Interop.Error.SUCCESS :
Interop.Sys.GetLastErrorInfo().Error;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ private static SafeFileHandle OpenHandle(string fullPath, bool asDirectory)
Interop.Kernel32.GenericOperations.GENERIC_WRITE,
FileShare.ReadWrite | FileShare.Delete,
FileMode.Open,
asDirectory ? Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS : 0);
asDirectory ? (Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT) : Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT);
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved

if (handle.IsInvalid)
{
Expand Down