Skip to content

Remove validation to stat call for symlinks since is a breaking change #57551

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 17, 2021
Merged

Remove validation to stat call for symlinks since is a breaking change #57551

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a90126b
Remove validation to stat call for symlinks since is a breaking chang…
jozkee Aug 17, 2021
d705eb7
Undo try-catch workaround in PhysicalFileProvider
jozkee Aug 17, 2021
e6c8835
Fix tests that were failing due to changes
jozkee Aug 17, 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
13 changes: 1 addition & 12 deletions src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PollingFileChangeToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,7 @@ private DateTime GetLastWriteTimeUtc()
return DateTime.MinValue;
}

DateTime? lastWriteTimeUtc = FileSystemInfoHelper.GetFileLinkTargetLastWriteTimeUtc(_fileInfo);

if (lastWriteTimeUtc == null)
{
try
{
lastWriteTimeUtc = _fileInfo.LastWriteTimeUtc;
}
catch (IOException) { } // https://github.com/dotnet/runtime/issues/57221
}

return lastWriteTimeUtc ?? DateTime.MinValue;
return FileSystemInfoHelper.GetFileLinkTargetLastWriteTimeUtc(_fileInfo) ?? _fileInfo.LastWriteTimeUtc;
}

/// <summary>
Expand Down
41 changes: 9 additions & 32 deletions src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@ internal struct FileStatus
// Positive number: the error code returned by the lstat call
private int _initializedFileCache;

// The last cached stat information about the file
// Refresh only collects this if lstat determines the path is a symbolic link
private Interop.Sys.FileStatus _symlinkCache;

// -1: if the symlink cache isn't initialized - Refresh only changes this value if lstat determines the path is a symbolic link
// 0: if the symlink cache was initialized with no errors
// Positive number: the error code returned by the stat call
private int _initializedSymlinkCache;

// We track intent of creation to know whether or not we want to (1) create a
// DirectoryInfo around this status struct or (2) actually are part of a DirectoryInfo.
// Set to true during initialization when the DirectoryEntry's INodeType describes a directory
Expand Down Expand Up @@ -113,7 +104,6 @@ private bool HasSymbolicLinkFlag
internal void InvalidateCaches()
{
_initializedFileCache = -1;
_initializedSymlinkCache = -1;
}

internal bool IsReadOnly(ReadOnlySpan<char> path, bool continueOnError = false)
Expand Down Expand Up @@ -340,17 +330,16 @@ internal long GetLength(ReadOnlySpan<char> path, bool continueOnError = false)
return _fileCache.Size;
}

// Tries to refresh the lstat cache (_fileCache) and, if the file is pointing to a symbolic link, then also the stat cache (_symlinkCache)
// Tries to refresh the lstat cache (_fileCache).
// This method should not throw. Instead, we store the results, and we will throw when the user attempts to access any of the properties when there was a failure
internal void RefreshCaches(ReadOnlySpan<char> path)
{
_isDirectory = false;
path = Path.TrimEndingDirectorySeparator(path);

// Retrieve the file cache (lstat) to get the details on the object, without following symlinks.
// If it is a symlink, then subsequently get details on the target of the symlink,
// storing those results separately. We only report failure if the initial
// lstat fails, as a broken symlink should still report info on exists, attributes, etc.
// If it is a symlink, then subsequently get details on the target of the symlink.
// We only report failure if the initial lstat fails, as a broken symlink should still report info on exists, attributes, etc.
if (!TryRefreshFileCache(path))
{
_exists = false;
Expand All @@ -361,11 +350,11 @@ internal void RefreshCaches(ReadOnlySpan<char> path)
_isDirectory = CacheHasDirectoryFlag(_fileCache);

// We also need to check if the main path is a symbolic link,
// in which case, we retrieve the symbolic link data
if (!_isDirectory && HasSymbolicLinkFlag && TryRefreshSymbolicLinkCache(path))
// in which case, we retrieve the symbolic link's target data
if (!_isDirectory && HasSymbolicLinkFlag && Interop.Sys.Stat(path, out Interop.Sys.FileStatus target) == 0)
{
// and check again if the symlink path is a directory
_isDirectory = CacheHasDirectoryFlag(_symlinkCache);
_isDirectory = CacheHasDirectoryFlag(target);
}

#if !TARGET_BROWSER
Expand All @@ -376,16 +365,15 @@ internal void RefreshCaches(ReadOnlySpan<char> path)

_exists = true;

// Checks if the specified cache (lstat=_fileCache, stat=_symlinkCache) has the directory attribute set
// Checks if the specified cache has the directory attribute set
// Only call if Refresh has been successfully called at least once, and you're
// certain the passed-in cache was successfully retrieved
static bool CacheHasDirectoryFlag(Interop.Sys.FileStatus cache) =>
(cache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR;
}

// Checks if the file cache is set to -1 and refreshes it's value.
// Optionally, if the symlink cache is set to -1 and the file cache determined the path is pointing to a symbolic link, this cache is also refreshed.
// If stat or lstat failed, and continueOnError is set to true, this method will throw.
// If it failed, and continueOnError is set to true, this method will throw.
internal void EnsureCachesInitialized(ReadOnlySpan<char> path, bool continueOnError = false)
{
if (_initializedFileCache == -1)
Expand All @@ -402,18 +390,10 @@ internal void EnsureCachesInitialized(ReadOnlySpan<char> path, bool continueOnEr
// Throws if any of the caches has an error number saved in it
private void ThrowOnCacheInitializationError(ReadOnlySpan<char> path)
{
int errno;
// Lstat should always be initialized by Refresh
if (_initializedFileCache != 0)
{
errno = _initializedFileCache;
InvalidateCaches();
throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path));
}
// Stat is optionally initialized when Refresh detects object is a symbolic link
else if (_initializedSymlinkCache != 0 && _initializedSymlinkCache != -1)
{
errno = _initializedSymlinkCache;
int errno = _initializedFileCache;
InvalidateCaches();
throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path));
}
Expand All @@ -422,9 +402,6 @@ private void ThrowOnCacheInitializationError(ReadOnlySpan<char> path)
private bool TryRefreshFileCache(ReadOnlySpan<char> path) =>
VerifyStatCall(Interop.Sys.LStat(path, out _fileCache), out _initializedFileCache);

private bool TryRefreshSymbolicLinkCache(ReadOnlySpan<char> path) =>
VerifyStatCall(Interop.Sys.Stat(path, out _symlinkCache), out _initializedSymlinkCache);

// Receives the return value of a stat or lstat call.
// If the call is unsuccessful, sets the initialized parameter to a positive number representing the last error info.
// If the call is successful, sets the initialized parameter to zero.
Expand Down