From 37418eadfc43fb12655c71dddc78ae368f4f9497 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Mon, 14 Aug 2023 12:34:07 -0700 Subject: [PATCH 1/2] [DRAFT] Consider symlinks when watching certificate file paths Also watch resolved targets of symlinks. There are still some rough edges around directory symlinks, since non-polling watchers tend not to notify us when they change. I think there might be something we can do, but I have yet to investigate. This may or may not work for the k8s case, depending on what events are produced when the `..data` symlink is updated (and whether polling is enabled). --- .../src/Internal/CertificatePathWatcher.cs | 225 +++++++--- .../CertificatePathWatcherLoggerExtensions.cs | 1 + .../Infrastructure/IFileInfoWithLinkInfo.cs | 17 + .../IFileProviderWithLinkInfo.cs | 22 + .../PhysicalFileProviderWithLinkInfo.cs | 120 ++++++ .../Core/test/CertificatePathWatcherTests.cs | 395 ++++++++++++++++-- 6 files changed, 687 insertions(+), 93 deletions(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileInfoWithLinkInfo.cs create mode 100644 src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileProviderWithLinkInfo.cs create mode 100644 src/Servers/Kestrel/Core/src/Internal/Infrastructure/PhysicalFileProviderWithLinkInfo.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs b/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs index 7cb7234ac0a8..fcb0a17a330c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs +++ b/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs @@ -2,8 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.FileProviders.Physical; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal; internal sealed partial class CertificatePathWatcher : IDisposable { - private readonly Func _fileProviderFactory; + private readonly Func _fileProviderFactory; private readonly string _contentRootDir; private readonly ILogger _logger; @@ -31,14 +31,14 @@ public CertificatePathWatcher(IHostEnvironment hostEnvironment, ILogger Directory.Exists(dir) ? new PhysicalFileProvider(dir, ExclusionFilters.None) : null) + dir => Directory.Exists(dir) ? new PhysicalFileProviderWithLinkInfo(dir, ExclusionFilters.None) : null) { } /// /// For testing. /// - internal CertificatePathWatcher(string contentRootPath, ILogger logger, Func fileProviderFactory) + internal CertificatePathWatcher(string contentRootPath, ILogger logger, Func fileProviderFactory) { _contentRootDir = contentRootPath; _logger = logger; @@ -102,9 +102,18 @@ public void UpdateWatches(List certificateConfigsToRemove, Li /// internal void AddWatchUnsynchronized(CertificateConfig certificateConfig) { - Debug.Assert(certificateConfig.IsFileCert, "AddWatch called on non-file cert"); + Debug.Assert(certificateConfig.IsFileCert, $"{nameof(AddWatchUnsynchronized)} called on non-file cert"); var path = Path.Combine(_contentRootDir, certificateConfig.Path); + + AddWatchWorkerUnsynchronized(certificateConfig, path, logSummary: true); + } + + /// + /// The patch of may not match (e.g. in the presence of symlinks). + /// + private void AddWatchWorkerUnsynchronized(CertificateConfig certificateConfig, string path, bool logSummary) + { var dir = Path.GetDirectoryName(path)!; if (!_metadataForDirectory.TryGetValue(dir, out var dirMetadata)) @@ -131,100 +140,175 @@ internal void AddWatchUnsynchronized(CertificateConfig certificateConfig) var disposable = ChangeToken.OnChange( () => dirMetadata.FileProvider.Watch(Path.GetFileName(path)), - static tuple => tuple.Item1.OnChange(tuple.Item2), - ValueTuple.Create(this, path)); + static tuple => tuple.Item1.OnChange(tuple.Item2, tuple.Item3), + ValueTuple.Create(this, certificateConfig, path)); fileMetadata = new FileWatchMetadata(disposable); _metadataForFile.Add(path, fileMetadata); dirMetadata.FileWatchCount++; // We actually don't care if the file doesn't exist - we'll watch in case it is created - fileMetadata.LastModifiedTime = GetLastModifiedTimeOrMinimum(path, dirMetadata.FileProvider); + fileMetadata.LastModifiedTime = GetLastModifiedTimeAndLinkTargetPath(path, dirMetadata.FileProvider, out fileMetadata.LinkTargetPath); _logger.CreatedFileWatcher(path); } if (!fileMetadata.Configs.Add(certificateConfig)) { + // Note: this also prevents us from getting stuck in symlink cycles _logger.ReusedObserver(path); return; } _logger.AddedObserver(path); - _logger.ObserverCount(path, fileMetadata.Configs.Count); - _logger.FileCount(dir, dirMetadata.FileWatchCount); - } + // If the directory itself is a link, resolve the link and recurse + // (Note that this will cover the case where the file is also a link) + if (dirMetadata.FileProvider.ResolveLinkTarget(returnFinalTarget: false) is IFileInfoWithLinkInfo dirLinkTarget) + { + if (dirLinkTarget.PhysicalPath is string dirLinkTargetPath) + { + // It may not exist, but we'll let AddSymlinkWatch deal with that and log something sensible + var linkTargetPath = Path.Combine(dirLinkTargetPath, Path.GetFileName(path)); + AddWatchWorkerUnsynchronized(certificateConfig, linkTargetPath, logSummary: false); + } + } + // Otherwise, if the file is a link, resolve the link and recurse + else if (fileMetadata.LinkTargetPath is string linkTargetPath) + { + AddWatchWorkerUnsynchronized(certificateConfig, linkTargetPath, logSummary: false); + } - private DateTimeOffset GetLastModifiedTimeOrMinimum(string path, IFileProvider fileProvider) - { - try + if (logSummary) { - return fileProvider.GetFileInfo(Path.GetFileName(path)).LastModified; + _logger.ObserverCount(path, fileMetadata.Configs.Count); + _logger.FileCount(dir, dirMetadata.FileWatchCount); } - catch (Exception e) + } + + /// + /// Returns if no last modified time is available (e.g. because the file doesn't exist). + /// + private static DateTimeOffset GetLastModifiedTimeAndLinkTargetPath(string path, IFileProviderWithLinkInfo fileProvider, out string? linkTargetPath) + { + var fileInfo = fileProvider.GetFileInfo(Path.GetFileName(path)); + if (!fileInfo.Exists) { - _logger.LastModifiedTimeError(path, e); + linkTargetPath = null; + return DateTimeOffset.MinValue; } - return DateTimeOffset.MinValue; + linkTargetPath = fileInfo.ResolveLinkTarget(returnFinalTarget: false)?.PhysicalPath; // We need to add a watch at every link in the chain + return fileInfo.LastModified; } - private void OnChange(string path) + private void OnChange(CertificateConfig certificateConfig, string path) { // Block until any in-progress updates are complete lock (_metadataLock) { - if (!_metadataForFile.TryGetValue(path, out var fileMetadata)) + if (!ShouldFireChangeUnsynchronized(certificateConfig, path)) { - _logger.UntrackedFileEvent(path); return; } + } + + // AddWatch and RemoveWatch don't affect the token, so this doesn't need to be under the semaphore. + // It does however need to be synchronized, since there could be multiple concurrent events. + var previousToken = Interlocked.Exchange(ref _reloadToken, new()); + previousToken.OnReload(); + } + + private bool ShouldFireChangeUnsynchronized(CertificateConfig certificateConfig, string path) + { + if (!_metadataForFile.TryGetValue(path, out var fileMetadata)) + { + _logger.UntrackedFileEvent(path); + return false; + } - // Existence implied by the fact that we're tracking the file - var dirMetadata = _metadataForDirectory[Path.GetDirectoryName(path)!]; - - // We ignore file changes that don't advance the last modified time. - // For example, if we lose access to the network share the file is - // stored on, we don't notify our listeners because no one wants - // their endpoint/server to shutdown when that happens. - // We also anticipate that a cert file might be renamed to cert.bak - // before a new cert is introduced with the old name. - // This also helps us in scenarios where the underlying file system - // reports more than one change for a single logical operation. - var lastModifiedTime = GetLastModifiedTimeOrMinimum(path, dirMetadata.FileProvider); - if (lastModifiedTime > fileMetadata.LastModifiedTime) + // Existence implied by the fact that we're tracking the file + var dirMetadata = _metadataForDirectory[Path.GetDirectoryName(path)!]; + + // We ignore file changes that don't advance the last modified time. + // For example, if we lose access to the network share the file is + // stored on, we don't notify our listeners because no one wants + // their endpoint/server to shutdown when that happens. + // We also anticipate that a cert file might be renamed to cert.bak + // before a new cert is introduced with the old name. + // This also helps us in scenarios where the underlying file system + // reports more than one change for a single logical operation. + var lastModifiedTime = GetLastModifiedTimeAndLinkTargetPath(path, dirMetadata.FileProvider, out var newLinkTargetPath); + if (lastModifiedTime > fileMetadata.LastModifiedTime) + { + fileMetadata.LastModifiedTime = lastModifiedTime; + } + else + { + if (lastModifiedTime == DateTimeOffset.MinValue) + { + _logger.EventWithoutLastModifiedTime(path); + } + else if (lastModifiedTime == fileMetadata.LastModifiedTime) { - fileMetadata.LastModifiedTime = lastModifiedTime; + Debug.Assert(newLinkTargetPath == fileMetadata.LinkTargetPath, "Link target changed without timestamp changing"); + + _logger.RedundantEvent(path); } else { - if (lastModifiedTime == DateTimeOffset.MinValue) - { - _logger.EventWithoutLastModifiedTime(path); - } - else if (lastModifiedTime == fileMetadata.LastModifiedTime) - { - _logger.RedundantEvent(path); - } - else - { - _logger.OutOfOrderEvent(path); - } - return; + _logger.OutOfOrderEvent(path); + } + + return false; + } + + var configs = fileMetadata.Configs; + foreach (var config in configs) + { + config.FileHasChanged = true; + } + + if (newLinkTargetPath != fileMetadata.LinkTargetPath) + { + // For correctness, we need to do the removal first, even though that might + // cause us to tear down some nodes that we will immediately re-add. + // For example, suppose you had a file A pointing to a file B, pointing to a file C: + // A -> B -> C + // If file A were updated to point to a new file D, which also pointed to C: + // A -> B -> C + // -> D -> + // Then adding a watch on D would no-op on recursively adding a watch on C, since its + // already monitored for A's CertificateConfig. + // Then removing a watch on B would tear down both B and C, even though D should still + // be keeping C alive. + // A -> D -> ?? + // Doing the removal first produces the correct result: + // A -> D -> C + // But it requires creating a watcher for C that is the same as the one that was removed + // (possibly including deleting and re-creating the directory watcher). Hopefully, + // such diamonds will be rare, in practice. + + // Also note that these updates are almost certainly redundant, since the change event + // we're about to fire will cause everything linked to, directly or indirectly, from + // certificateConfig.path to be updated in the next call to UpdateWatches. However, + // baking in the assumption as part of the contract for consuming this type would be + // unreasonable - it should make sense in its own right. + + if (fileMetadata.LinkTargetPath is string oldLinkTargetPath) + { + // We already have a rooted path, so call the worker + RemoveWatchWorkerUnsynchronized(certificateConfig, oldLinkTargetPath, logSummary: false); } - var configs = fileMetadata.Configs; - foreach (var config in configs) + if (newLinkTargetPath is not null) { - config.FileHasChanged = true; + // We already have a rooted path, so call the worker + AddWatchWorkerUnsynchronized(certificateConfig, newLinkTargetPath, logSummary: false); } } - // AddWatch and RemoveWatch don't affect the token, so this doesn't need to be under the semaphore. - // It does however need to be synchronized, since there could be multiple concurrent events. - var previousToken = Interlocked.Exchange(ref _reloadToken, new()); - previousToken.OnReload(); + return true; } /// @@ -236,9 +320,17 @@ private void OnChange(string path) /// internal void RemoveWatchUnsynchronized(CertificateConfig certificateConfig) { - Debug.Assert(certificateConfig.IsFileCert, "RemoveWatch called on non-file cert"); - + Debug.Assert(certificateConfig.IsFileCert, $"{nameof(RemoveWatchUnsynchronized)} called on non-file cert"); var path = Path.Combine(_contentRootDir, certificateConfig.Path); + + RemoveWatchWorkerUnsynchronized(certificateConfig, path, logSummary: true); + } + + /// + /// The patch of may not match (e.g. in the presence of symlinks). + /// + private void RemoveWatchWorkerUnsynchronized(CertificateConfig certificateConfig, string path, bool logSummary) + { var dir = Path.GetDirectoryName(path)!; if (!_metadataForFile.TryGetValue(path, out var fileMetadata)) @@ -257,6 +349,13 @@ internal void RemoveWatchUnsynchronized(CertificateConfig certificateConfig) _logger.RemovedObserver(path); + if (fileMetadata.LinkTargetPath is string linkTargetPath) + { + // We built this graph, so we already know there are no cycles + // Never log summary messages from recursive calls + RemoveWatchWorkerUnsynchronized(certificateConfig, linkTargetPath, logSummary: false); + } + // If we found fileMetadata, there must be a containing/corresponding dirMetadata var dirMetadata = _metadataForDirectory[dir]; @@ -277,8 +376,11 @@ internal void RemoveWatchUnsynchronized(CertificateConfig certificateConfig) } } - _logger.ObserverCount(path, configs.Count); - _logger.FileCount(dir, dirMetadata.FileWatchCount); + if (logSummary) + { + _logger.ObserverCount(path, configs.Count); + _logger.FileCount(dir, dirMetadata.FileWatchCount); + } } /// Test hook @@ -309,9 +411,9 @@ void IDisposable.Dispose() } } - private sealed class DirectoryWatchMetadata(IFileProvider fileProvider) : IDisposable + private sealed class DirectoryWatchMetadata(IFileProviderWithLinkInfo fileProvider) : IDisposable { - public readonly IFileProvider FileProvider = fileProvider; + public readonly IFileProviderWithLinkInfo FileProvider = fileProvider; public int FileWatchCount; public void Dispose() => (FileProvider as IDisposable)?.Dispose(); @@ -322,6 +424,7 @@ private sealed class FileWatchMetadata(IDisposable disposable) : IDisposable public readonly IDisposable Disposable = disposable; public readonly HashSet Configs = new(ReferenceEqualityComparer.Instance); public DateTimeOffset LastModifiedTime = DateTimeOffset.MinValue; + public string? LinkTargetPath; public void Dispose() => Disposable.Dispose(); } diff --git a/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcherLoggerExtensions.cs b/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcherLoggerExtensions.cs index d41543a342a6..afbb7b71a27e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcherLoggerExtensions.cs +++ b/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcherLoggerExtensions.cs @@ -28,6 +28,7 @@ internal static partial class CertificatePathWatcherLoggerExtensions [LoggerMessage(7, LogLevel.Debug, "Removed file watcher for '{Path}'.", EventName = "RemovedFileWatcher")] public static partial void RemovedFileWatcher(this ILogger logger, string path); + [Obsolete("This event will no longer be reported")] [LoggerMessage(8, LogLevel.Debug, "Error retrieving last modified time for '{Path}'.", EventName = "LastModifiedTimeError")] public static partial void LastModifiedTimeError(this ILogger logger, string path, Exception e); diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileInfoWithLinkInfo.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileInfoWithLinkInfo.cs new file mode 100644 index 000000000000..6822b79078bf --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileInfoWithLinkInfo.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.FileProviders; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; + +/// +/// Extends with the ability to resolve symlink targets. +/// +internal interface IFileInfoWithLinkInfo : IFileInfo +{ + /// + /// equivalent of . + /// + IFileInfoWithLinkInfo? ResolveLinkTarget(bool returnFinalTarget); +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileProviderWithLinkInfo.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileProviderWithLinkInfo.cs new file mode 100644 index 000000000000..e3cf2d78fd56 --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileProviderWithLinkInfo.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.FileProviders; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; + +/// +/// Extends with the ability to resolve symlink targets of the provided directory. +/// +internal interface IFileProviderWithLinkInfo : IFileProvider +{ + /// + /// Returns the link target of the 's root directory. + /// + /// + /// Effectively . + /// + IFileInfoWithLinkInfo? ResolveLinkTarget(bool returnFinalTarget); + + public new IFileInfoWithLinkInfo GetFileInfo(string subpath); +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PhysicalFileProviderWithLinkInfo.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PhysicalFileProviderWithLinkInfo.cs new file mode 100644 index 000000000000..7de987c7f65b --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PhysicalFileProviderWithLinkInfo.cs @@ -0,0 +1,120 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.FileProviders; +using Microsoft.Extensions.FileProviders.Physical; +using Microsoft.Extensions.Primitives; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; + +/// +/// A that returns , rather than . +/// +internal sealed class PhysicalFileProviderWithLinkInfo : IFileProviderWithLinkInfo +{ + private readonly PhysicalFileProvider _inner; + + public PhysicalFileProviderWithLinkInfo(string root, ExclusionFilters filters) + { + _inner = new PhysicalFileProvider(root, filters); + } + + IDirectoryContents IFileProvider.GetDirectoryContents(string subpath) => _inner.GetDirectoryContents(subpath); + IChangeToken IFileProvider.Watch(string filter) => _inner.Watch(filter); + + IFileInfo IFileProvider.GetFileInfo(string subpath) + { + return ((IFileProviderWithLinkInfo)this).GetFileInfo(subpath); + } + + IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string subpath) + { + return new FileInfoWithLinkInfo(_inner.GetFileInfo(subpath)); + } + + IFileInfoWithLinkInfo? IFileProviderWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) + { + IFileInfoWithLinkInfo dirInfo = new DirectoryInfoWithLinkInfo(new DirectoryInfo(_inner.Root)); + return dirInfo.ResolveLinkTarget(returnFinalTarget); + } + + private sealed class FileInfoWithLinkInfo : IFileInfoWithLinkInfo + { + private readonly IFileInfo _inner; + + public FileInfoWithLinkInfo(IFileInfo inner) + { + _inner = inner; + } + + bool IFileInfo.Exists => _inner.Exists; + bool IFileInfo.IsDirectory => _inner.IsDirectory; + DateTimeOffset IFileInfo.LastModified => _inner.LastModified; + long IFileInfo.Length => _inner.Length; + string IFileInfo.Name => _inner.Name; + string? IFileInfo.PhysicalPath => _inner.PhysicalPath; + Stream IFileInfo.CreateReadStream() => _inner.CreateReadStream(); + + IFileInfoWithLinkInfo? IFileInfoWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) + { + if (!_inner.Exists) + { + return null; + } + + var path = _inner.PhysicalPath; + + if (path is null) + { + return null; + } + + var fileInfo = new FileInfo(path); + + var linkFileSystemInfo = fileInfo.ResolveLinkTarget(returnFinalTarget); + + if (linkFileSystemInfo is not FileInfo linkInfo) + { + return null; + } + + return new FileInfoWithLinkInfo(new PhysicalFileInfo(linkInfo)); + } + } + private sealed class DirectoryInfoWithLinkInfo : IFileInfoWithLinkInfo + { + private readonly DirectoryInfo _directoryInfo; + + public DirectoryInfoWithLinkInfo(DirectoryInfo directoryInfo) + { + _directoryInfo = directoryInfo; + } + + bool IFileInfo.IsDirectory => true; + + bool IFileInfo.Exists => _directoryInfo.Exists; + DateTimeOffset IFileInfo.LastModified => _directoryInfo.LastWriteTimeUtc; + string IFileInfo.Name => _directoryInfo.Name; + string? IFileInfo.PhysicalPath => _directoryInfo.FullName; + + long IFileInfo.Length => throw new NotSupportedException(); // We could probably just return 0, though that's not strictly accurate + Stream IFileInfo.CreateReadStream() => throw new NotSupportedException(); + + IFileInfoWithLinkInfo? IFileInfoWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) + { + if (!_directoryInfo.Exists) + { + return null; + } + + var linkFileSystemInfo = _directoryInfo.ResolveLinkTarget(returnFinalTarget); + + if (linkFileSystemInfo is not DirectoryInfo linkInfo) + { + return null; + } + + return new DirectoryInfoWithLinkInfo(linkInfo); + } + } +} diff --git a/src/Servers/Kestrel/Core/test/CertificatePathWatcherTests.cs b/src/Servers/Kestrel/Core/test/CertificatePathWatcherTests.cs index b4be5350f81c..c6bdd88e07a7 100644 --- a/src/Servers/Kestrel/Core/test/CertificatePathWatcherTests.cs +++ b/src/Servers/Kestrel/Core/test/CertificatePathWatcherTests.cs @@ -1,7 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.FileProviders; @@ -124,7 +126,7 @@ public async Task FileChanged(int observerCount) var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(); + var fileProvider = new MockFileProvider(dir); var fileLastModifiedTime = DateTimeOffset.UtcNow; fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); @@ -165,6 +167,258 @@ public async Task FileChanged(int observerCount) Assert.All(certificateConfigs, cc => Assert.True(cc.FileHasChanged)); } + [Fact] + public async Task FileReplacedWithLink() + { + var dir = Directory.GetCurrentDirectory(); + + var fileName = Path.GetRandomFileName(); + var filePath = Path.Combine(dir, fileName); + + var linkTargetName = Path.GetRandomFileName(); + var linkTargetPath = Path.Combine(dir, linkTargetName); + + var logger = LoggerFactory.CreateLogger(); + + var fileProvider = new MockFileProvider(dir); + var fileLastModifiedTime = DateTimeOffset.UtcNow; + var linkTargetLastModifiedTime = fileLastModifiedTime.AddSeconds(-1); // target is *older* than original file + + fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(linkTargetName, linkTargetLastModifiedTime); + + using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); + + var signalTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var oldChangeToken = watcher.GetChangeToken(); + oldChangeToken.RegisterChangeCallback(_ => signalTcs.SetResult(), state: null); + + var certificateConfig = new CertificateConfig + { + Path = filePath, + }; + + watcher.AddWatchUnsynchronized(certificateConfig); + + Assert.Equal(1, watcher.TestGetDirectoryWatchCountUnsynchronized()); + Assert.Equal(1, watcher.TestGetFileWatchCountUnsynchronized(dir)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(filePath)); + Assert.Equal(0, watcher.TestGetObserverCountUnsynchronized(linkTargetPath)); + + // Simulate file change on disk + fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime.AddSeconds(1)); + fileProvider.SetLinkTarget(fileName, linkTargetName, fileProvider); + fileProvider.FireChangeToken(fileName); + + await signalTcs.Task.DefaultTimeout(); + + Assert.Equal(1, watcher.TestGetDirectoryWatchCountUnsynchronized()); + Assert.Equal(2, watcher.TestGetFileWatchCountUnsynchronized(dir)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(filePath)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(linkTargetPath)); + + var newChangeToken = watcher.GetChangeToken(); + + Assert.NotSame(oldChangeToken, newChangeToken); + Assert.True(oldChangeToken.HasChanged); + Assert.False(newChangeToken.HasChanged); + + Assert.True(certificateConfig.FileHasChanged); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task FileLinkChanged(bool updatedFileIsLink) + { + var dir = Directory.GetCurrentDirectory(); + + var fileName = Path.GetRandomFileName(); + var filePath = Path.Combine(dir, fileName); + + var oldLinkTargetName = Path.GetRandomFileName(); + var oldLinkTargetPath = Path.Combine(dir, oldLinkTargetName); + + var newLinkTargetName = Path.GetRandomFileName(); + var newLinkTargetPath = Path.Combine(dir, newLinkTargetName); + + var logger = LoggerFactory.CreateLogger(); + + var fileProvider = new MockFileProvider(dir); + var fileLastModifiedTime = DateTimeOffset.UtcNow; + + fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(oldLinkTargetName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(newLinkTargetName, fileLastModifiedTime); + + fileProvider.SetLinkTarget(fileName, oldLinkTargetName, fileProvider); + + using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); + + var signalTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var oldChangeToken = watcher.GetChangeToken(); + oldChangeToken.RegisterChangeCallback(_ => signalTcs.SetResult(), state: null); + + var certificateConfig = new CertificateConfig + { + Path = filePath, + }; + + watcher.AddWatchUnsynchronized(certificateConfig); + + Assert.Equal(1, watcher.TestGetDirectoryWatchCountUnsynchronized()); + Assert.Equal(2, watcher.TestGetFileWatchCountUnsynchronized(dir)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(filePath)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(oldLinkTargetPath)); + Assert.Equal(0, watcher.TestGetObserverCountUnsynchronized(newLinkTargetPath)); + + // Simulate file change on disk + fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime.AddSeconds(1)); + if (updatedFileIsLink) + { + fileProvider.SetLinkTarget(fileName, newLinkTargetName, fileProvider); + } + else + { + fileProvider.RemoveLinkTarget(fileName); + } + fileProvider.FireChangeToken(fileName); + + await signalTcs.Task.DefaultTimeout(); + + var newChangeToken = watcher.GetChangeToken(); + + Assert.Equal(1, watcher.TestGetDirectoryWatchCountUnsynchronized()); + Assert.Equal(updatedFileIsLink ? 2 : 1, watcher.TestGetFileWatchCountUnsynchronized(dir)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(filePath)); + Assert.Equal(0, watcher.TestGetObserverCountUnsynchronized(oldLinkTargetPath)); + Assert.Equal(updatedFileIsLink ? 1 : 0, watcher.TestGetObserverCountUnsynchronized(newLinkTargetPath)); + + Assert.NotSame(oldChangeToken, newChangeToken); + Assert.True(oldChangeToken.HasChanged); + Assert.False(newChangeToken.HasChanged); + + Assert.True(certificateConfig.FileHasChanged); + } + + [Fact] + public async Task FileLinkTargetChanged() + { + var dir = Directory.GetCurrentDirectory(); + + var fileName = Path.GetRandomFileName(); + var filePath = Path.Combine(dir, fileName); + + var linkTargetName = Path.GetRandomFileName(); + var linkTargetPath = Path.Combine(dir, linkTargetName); + + var logger = LoggerFactory.CreateLogger(); + + var fileProvider = new MockFileProvider(dir); + var fileLastModifiedTime = DateTimeOffset.UtcNow; + + fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(linkTargetName, fileLastModifiedTime); + + fileProvider.SetLinkTarget(fileName, linkTargetName, fileProvider); + + using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); + + var signalTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var oldChangeToken = watcher.GetChangeToken(); + oldChangeToken.RegisterChangeCallback(_ => signalTcs.SetResult(), state: null); + + var certificateConfig = new CertificateConfig + { + Path = filePath, + }; + + watcher.AddWatchUnsynchronized(certificateConfig); + + Assert.Equal(1, watcher.TestGetDirectoryWatchCountUnsynchronized()); + Assert.Equal(2, watcher.TestGetFileWatchCountUnsynchronized(dir)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(filePath)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(linkTargetPath)); + + // Simulate file change on disk + fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime.AddSeconds(1)); + fileProvider.SetLinkTarget(fileName, linkTargetName, fileProvider); + fileProvider.FireChangeToken(fileName); + + await signalTcs.Task.DefaultTimeout(); + + var newChangeToken = watcher.GetChangeToken(); + + Assert.Equal(1, watcher.TestGetDirectoryWatchCountUnsynchronized()); + Assert.Equal(2, watcher.TestGetFileWatchCountUnsynchronized(dir)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(filePath)); + Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(linkTargetPath)); + + Assert.NotSame(oldChangeToken, newChangeToken); + Assert.True(oldChangeToken.HasChanged); + Assert.False(newChangeToken.HasChanged); + + Assert.True(certificateConfig.FileHasChanged); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(5)] + [LogLevel(LogLevel.Trace)] + public void FileLinkCycle(int cycleLength) + { + var dir = Directory.GetCurrentDirectory(); + + var fileNames = new string[cycleLength]; + var filePaths = new string[cycleLength]; + + var logger = LoggerFactory.CreateLogger(); + + var fileProvider = new MockFileProvider(dir); + var fileLastModifiedTime = DateTimeOffset.UtcNow; + + for (int i = 0; i < cycleLength; i++) + { + fileNames[i] = Path.GetRandomFileName(); + filePaths[i] = Path.Combine(dir, fileNames[i]); + fileProvider.SetLastModifiedTime(fileNames[i], fileLastModifiedTime); + } + + for (int i = 0; i < cycleLength; i++) + { + fileProvider.SetLinkTarget(fileNames[i], fileNames[(i + 1) % cycleLength], fileProvider); + } + + using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); + + var logTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + TestSink.MessageLogged += writeContext => + { + if (writeContext.EventId.Name == "ReusedObserver") + { + logTcs.SetResult(); + } + }; + + var certificateConfig = new CertificateConfig + { + Path = filePaths[0], + }; + + watcher.AddWatchUnsynchronized(certificateConfig); + + Assert.Equal(1, watcher.TestGetDirectoryWatchCountUnsynchronized()); + Assert.Equal(cycleLength, watcher.TestGetFileWatchCountUnsynchronized(dir)); + Assert.All(filePaths, filePath => Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(filePath))); + + Assert.True(logTcs.Task.IsCompleted); + } + [Fact] public async Task OutOfOrderLastModifiedTime() { @@ -174,7 +428,7 @@ public async Task OutOfOrderLastModifiedTime() var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(); + var fileProvider = new MockFileProvider(dir); var fileLastModifiedTime = DateTimeOffset.UtcNow; fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); @@ -355,7 +609,7 @@ public async Task IgnoreDeletion(bool seeChangeForDeletion, bool restoredWithNew var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(); + var fileProvider = new MockFileProvider(dir); var fileLastModifiedTime = DateTimeOffset.UtcNow; fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); @@ -504,18 +758,21 @@ private static IDictionary GetLogMessageProperties(ITestSink tes return dict; } - private sealed class NoChangeFileProvider : IFileProvider + private sealed class NoChangeFileProvider : IFileProviderWithLinkInfo { - public static readonly IFileProvider Instance = new NoChangeFileProvider(); + public static readonly IFileProviderWithLinkInfo Instance = new NoChangeFileProvider(); private NoChangeFileProvider() { } IDirectoryContents IFileProvider.GetDirectoryContents(string subpath) => throw new NotSupportedException(); - IFileInfo IFileProvider.GetFileInfo(string subpath) => throw new NotSupportedException(); + IFileInfo IFileProvider.GetFileInfo(string subpath) => FixedTimeFileInfoWithLinkInfo.Instance; IChangeToken IFileProvider.Watch(string filter) => NoChangeChangeToken.Instance; + IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string subpath) => FixedTimeFileInfoWithLinkInfo.Instance; + IFileInfoWithLinkInfo IFileProviderWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) => null; + private sealed class NoChangeChangeToken : IChangeToken { public static readonly IChangeToken Instance = new NoChangeChangeToken(); @@ -528,6 +785,28 @@ private NoChangeChangeToken() bool IChangeToken.ActiveChangeCallbacks => true; IDisposable IChangeToken.RegisterChangeCallback(Action callback, object state) => DummyDisposable.Instance; } + + private sealed class FixedTimeFileInfoWithLinkInfo : IFileInfoWithLinkInfo + { + public static readonly IFileInfoWithLinkInfo Instance = new FixedTimeFileInfoWithLinkInfo(); + + private readonly DateTimeOffset _lastModified = DateTimeOffset.UtcNow; + + private FixedTimeFileInfoWithLinkInfo() + { + } + + DateTimeOffset IFileInfo.LastModified => _lastModified; + bool IFileInfo.Exists => true; + bool IFileInfo.IsDirectory => false; + + long IFileInfo.Length => throw new NotSupportedException(); + string IFileInfo.PhysicalPath => throw new NotSupportedException(); + string IFileInfo.Name => throw new NotSupportedException(); + Stream IFileInfo.CreateReadStream() => throw new NotSupportedException(); + + IFileInfoWithLinkInfo IFileInfoWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) => null; + } } private sealed class DummyDisposable : IDisposable @@ -543,60 +822,112 @@ void IDisposable.Dispose() } } - private sealed class MockFileProvider : IFileProvider + private sealed class MockFileProvider : IFileProviderWithLinkInfo { private readonly Dictionary _changeTokens = new(); private readonly Dictionary _lastModifiedTimes = new(); + private readonly Dictionary _linkTargets = new(); + + private readonly string _rootPath; - public void FireChangeToken(string path) + public MockFileProvider(string rootPath) { - var oldChangeToken = _changeTokens[path]; - _changeTokens[path] = new ConfigurationReloadToken(); + _rootPath = rootPath; + } + + public void FireChangeToken(string fileName) + { + var oldChangeToken = _changeTokens[fileName]; + _changeTokens[fileName] = new ConfigurationReloadToken(); oldChangeToken.OnReload(); } - public void SetLastModifiedTime(string path, DateTimeOffset? lastModifiedTime) + public void SetLastModifiedTime(string fileName, DateTimeOffset? lastModifiedTime) { - _lastModifiedTimes[path] = lastModifiedTime; + _lastModifiedTimes[fileName] = lastModifiedTime; } - IDirectoryContents IFileProvider.GetDirectoryContents(string subpath) + public void RemoveLinkTarget(string fileName) { - throw new NotSupportedException(); + _linkTargets.Remove(fileName); + } + + public void SetLinkTarget(string fileName, string linkTargetName, IFileProviderWithLinkInfo linkTargetFileProvider) + { + var targetInfo = linkTargetFileProvider.GetFileInfo(linkTargetName); + _linkTargets[fileName] = targetInfo; } - IFileInfo IFileProvider.GetFileInfo(string subpath) + IDirectoryContents IFileProvider.GetDirectoryContents(string subpath) { - return new MockFileInfo(_lastModifiedTimes[subpath]); + throw new NotSupportedException(); } - IChangeToken IFileProvider.Watch(string path) + IChangeToken IFileProvider.Watch(string fileName) { - if (!_changeTokens.TryGetValue(path, out var changeToken)) + if (!_changeTokens.TryGetValue(fileName, out var changeToken)) { - _changeTokens[path] = changeToken = new ConfigurationReloadToken(); + _changeTokens[fileName] = changeToken = new ConfigurationReloadToken(); } return changeToken; } - private sealed class MockFileInfo : IFileInfo + IFileInfo IFileProvider.GetFileInfo(string fileName) { - private readonly DateTimeOffset? _lastModifiedTime; + return ((IFileProviderWithLinkInfo)this).GetFileInfo(fileName); + } - public MockFileInfo(DateTimeOffset? lastModifiedTime) - { - _lastModifiedTime = lastModifiedTime; - } + IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string fileName) + { + return new MockFileInfoWithLinkInfo(Path.Combine(_rootPath, fileName), _lastModifiedTimes[fileName], _linkTargets.TryGetValue(fileName, out var target) ? target : null); + } - bool IFileInfo.Exists => _lastModifiedTime.HasValue; - DateTimeOffset IFileInfo.LastModified => _lastModifiedTime.GetValueOrDefault(); + IFileInfoWithLinkInfo IFileProviderWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) + { + return null; + } + } - long IFileInfo.Length => throw new NotSupportedException(); - string IFileInfo.PhysicalPath => throw new NotSupportedException(); - string IFileInfo.Name => throw new NotSupportedException(); - bool IFileInfo.IsDirectory => throw new NotSupportedException(); - Stream IFileInfo.CreateReadStream() => throw new NotSupportedException(); + private sealed class MockFileInfoWithLinkInfo : IFileInfoWithLinkInfo + { + private readonly string _path; + private readonly DateTimeOffset? _lastModifiedTime; + private readonly IFileInfoWithLinkInfo _linkTarget; + + public MockFileInfoWithLinkInfo(string path, DateTimeOffset? lastModifiedTime, IFileInfoWithLinkInfo linkTarget) + { + _path = path; + _lastModifiedTime = lastModifiedTime; + _linkTarget = linkTarget; } + + bool IFileInfo.Exists => _lastModifiedTime.HasValue; + DateTimeOffset IFileInfo.LastModified => _lastModifiedTime.GetValueOrDefault(); + string IFileInfo.PhysicalPath => _path; + + long IFileInfo.Length => throw new NotSupportedException(); + string IFileInfo.Name => throw new NotSupportedException(); + bool IFileInfo.IsDirectory => throw new NotSupportedException(); + Stream IFileInfo.CreateReadStream() => throw new NotSupportedException(); + + IFileInfoWithLinkInfo IFileInfoWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) => _linkTarget; + } + + private sealed class MockLinkFileProvider : IFileProviderWithLinkInfo + { + private readonly IFileInfoWithLinkInfo _linkTarget; + + public MockLinkFileProvider(IFileInfoWithLinkInfo linkTarget) + { + _linkTarget = linkTarget; + } + + IChangeToken IFileProvider.Watch(string filter) => throw new NotSupportedException(); + IDirectoryContents IFileProvider.GetDirectoryContents(string subpath) => throw new NotSupportedException(); + IFileInfo IFileProvider.GetFileInfo(string subpath) => throw new NotSupportedException(); + IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string subpath) => throw new NotSupportedException(); + + IFileInfoWithLinkInfo IFileProviderWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) => _linkTarget; } } From 1e5d35ba40e3c7737f1c238217e5725702efa34b Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 15 Aug 2023 19:25:26 -0700 Subject: [PATCH 2/2] Handle directory symlinks Specifically, move the file watcher up one level so that we can also see changes to the parent directory of a cert, even if it's a symlink. Note that this approach still won't handle changes to directories further up the hierarchy. It also does nothing for files directly in the root directory which, presumably, can't be modified anyway. The tests are failing because they're testing for things that are no longer intended to be true. --- .../src/Internal/CertificatePathWatcher.cs | 287 ++++++++++-------- .../CertificatePathWatcherLoggerExtensions.cs | 3 + .../IFileProviderWithLinkInfo.cs | 8 - .../PhysicalFileProviderWithLinkInfo.cs | 61 +--- .../Core/test/CertificatePathWatcherTests.cs | 263 ++++++++-------- 5 files changed, 319 insertions(+), 303 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs b/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs index fcb0a17a330c..3a1274aaccb4 100644 --- a/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs +++ b/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs @@ -79,17 +79,82 @@ public void UpdateWatches(List certificateConfigsToRemove, Li lock (_metadataLock) { - // Adds before removes to increase the chances of watcher reuse. - // Since removeSet doesn't contain any of these configs, this won't change the semantics. + foreach (var certificateConfig in removeSet) + { + RemoveWatchUnsynchronized(certificateConfig); + } + foreach (var certificateConfig in addSet) { AddWatchUnsynchronized(certificateConfig); } - foreach (var certificateConfig in removeSet) + // We don't clean up unused watchers until the Adds have been processed to maximize reuse + if (removeSet.Count > 0) { - RemoveWatchUnsynchronized(certificateConfig); + // We could conceivably save time by propagating a list of files affected out of RemoveWatchUnsynchronized + + List? filePathsToRemove = null; + List? dirPathsToRemove = null; + foreach (var (path, fileMetadata) in _metadataForFile) + { + if (fileMetadata.Configs.Count == 0) + { + (filePathsToRemove ??= new()).Add(path); + + var dir = Path.GetDirectoryName(path)!; + + // If we found fileMetadata, there must be a containing/corresponding dirMetadata + var dirMetadata = _metadataForDirectory[dir]; + + dirMetadata.FileWatchCount--; + if (dirMetadata.FileWatchCount == 0) + { + (dirPathsToRemove ??= new()).Add(dir); + } + } + } + + if (filePathsToRemove is not null) + { + foreach (var path in filePathsToRemove) + { + _metadataForFile.Remove(path, out var fileMetadata); + fileMetadata!.Dispose(); + _logger.RemovedFileWatcher(path); + } + } + + if (dirPathsToRemove is not null) + { + foreach (var dir in dirPathsToRemove) + { + _metadataForDirectory.Remove(dir, out var dirMetadata); + dirMetadata!.Dispose(); + _logger.RemovedDirectoryWatcher(dir); + } + } } + + LogStateUnsynchronized(); + } + } + + private void LogStateUnsynchronized() + { + if (!_logger.IsEnabled(LogLevel.Trace)) + { + return; + } + + foreach (var (dir, dirMetadata) in _metadataForDirectory) + { + _logger.FileCount(dir, dirMetadata.FileWatchCount); + } + + foreach (var (path, fileMetadata) in _metadataForFile) + { + _logger.ObserverCount(path, fileMetadata.Configs.Count); } } @@ -106,13 +171,13 @@ internal void AddWatchUnsynchronized(CertificateConfig certificateConfig) var path = Path.Combine(_contentRootDir, certificateConfig.Path); - AddWatchWorkerUnsynchronized(certificateConfig, path, logSummary: true); + AddWatchWorkerUnsynchronized(certificateConfig, path); } /// /// The patch of may not match (e.g. in the presence of symlinks). /// - private void AddWatchWorkerUnsynchronized(CertificateConfig certificateConfig, string path, bool logSummary) + private void AddWatchWorkerUnsynchronized(CertificateConfig certificateConfig, string path) { var dir = Path.GetDirectoryName(path)!; @@ -121,14 +186,17 @@ private void AddWatchWorkerUnsynchronized(CertificateConfig certificateConfig, s // If we wanted to detected deletions of this whole directory (which we don't since we ignore deletions), // we'd probably need to watch the whole directory hierarchy - var fileProvider = _fileProviderFactory(dir); + var parentDir = Path.GetDirectoryName(dir); + var directoryName = parentDir is null ? null : Path.GetFileName(dir); + + var fileProvider = _fileProviderFactory(parentDir ?? dir); if (fileProvider is null) { _logger.DirectoryDoesNotExist(dir, path); return; } - dirMetadata = new DirectoryWatchMetadata(fileProvider); + dirMetadata = new DirectoryWatchMetadata(fileProvider, directoryName); _metadataForDirectory.Add(dir, dirMetadata); _logger.CreatedDirectoryWatcher(dir); @@ -139,16 +207,30 @@ private void AddWatchWorkerUnsynchronized(CertificateConfig certificateConfig, s // PhysicalFileProvider appears to be able to tolerate non-existent files, as long as the directory exists var disposable = ChangeToken.OnChange( - () => dirMetadata.FileProvider.Watch(Path.GetFileName(path)), + () => + { + var fileProvider = dirMetadata.FileProvider; + + if (dirMetadata.DirectoryName is not string dirName) + { + return fileProvider.Watch(Path.GetFileName(path)); + } + + return new CompositeChangeToken(new[] + { + fileProvider.Watch(dirName), + fileProvider.Watch(Path.Combine(dirName, Path.GetFileName(path))), + }); + }, static tuple => tuple.Item1.OnChange(tuple.Item2, tuple.Item3), - ValueTuple.Create(this, certificateConfig, path)); + ValueTuple.Create(this, path, Path.Combine(_contentRootDir, certificateConfig.Path!))); fileMetadata = new FileWatchMetadata(disposable); _metadataForFile.Add(path, fileMetadata); dirMetadata.FileWatchCount++; // We actually don't care if the file doesn't exist - we'll watch in case it is created - fileMetadata.LastModifiedTime = GetLastModifiedTimeAndLinkTargetPath(path, dirMetadata.FileProvider, out fileMetadata.LinkTargetPath); + fileMetadata.LastModifiedTime = GetLastModifiedTimeAndLinkTargetPath(path, dirMetadata, out fileMetadata.LinkTargetPath); _logger.CreatedFileWatcher(path); } @@ -162,52 +244,54 @@ private void AddWatchWorkerUnsynchronized(CertificateConfig certificateConfig, s _logger.AddedObserver(path); - // If the directory itself is a link, resolve the link and recurse - // (Note that this will cover the case where the file is also a link) - if (dirMetadata.FileProvider.ResolveLinkTarget(returnFinalTarget: false) is IFileInfoWithLinkInfo dirLinkTarget) - { - if (dirLinkTarget.PhysicalPath is string dirLinkTargetPath) - { - // It may not exist, but we'll let AddSymlinkWatch deal with that and log something sensible - var linkTargetPath = Path.Combine(dirLinkTargetPath, Path.GetFileName(path)); - AddWatchWorkerUnsynchronized(certificateConfig, linkTargetPath, logSummary: false); - } - } - // Otherwise, if the file is a link, resolve the link and recurse - else if (fileMetadata.LinkTargetPath is string linkTargetPath) - { - AddWatchWorkerUnsynchronized(certificateConfig, linkTargetPath, logSummary: false); - } - - if (logSummary) + if (fileMetadata.LinkTargetPath is string linkTargetPath) { - _logger.ObserverCount(path, fileMetadata.Configs.Count); - _logger.FileCount(dir, dirMetadata.FileWatchCount); + // The link target may not exist, but we'll let AddWatchWorkerUnsynchronized deal with that and log something sensible + AddWatchWorkerUnsynchronized(certificateConfig, linkTargetPath); } } /// /// Returns if no last modified time is available (e.g. because the file doesn't exist). /// - private static DateTimeOffset GetLastModifiedTimeAndLinkTargetPath(string path, IFileProviderWithLinkInfo fileProvider, out string? linkTargetPath) + private static DateTimeOffset GetLastModifiedTimeAndLinkTargetPath(string path, DirectoryWatchMetadata dirMetadata, out string? linkTargetPath) { - var fileInfo = fileProvider.GetFileInfo(Path.GetFileName(path)); + var fileProvider = dirMetadata.FileProvider; + var subpath = Path.GetFileName(path)!; + var dirName = dirMetadata.DirectoryName; + if (dirName is not null) + { + subpath = Path.Combine(dirName, subpath); + } + + var fileInfo = fileProvider.GetFileInfo(subpath); if (!fileInfo.Exists) { linkTargetPath = null; return DateTimeOffset.MinValue; } - linkTargetPath = fileInfo.ResolveLinkTarget(returnFinalTarget: false)?.PhysicalPath; // We need to add a watch at every link in the chain + // If the directory itself is a link, resolve that link. We prefer to resolve the + // directory link before the file link (if any) because the file will still be a link + // after resolving the directory link but the reverse isn't true. + var dirLinkTargetPath = dirName is not null + ? dirMetadata.FileProvider.GetFileInfo(dirName).ResolveLinkTarget(returnFinalTarget: false)?.PhysicalPath + : null; + linkTargetPath = dirLinkTargetPath is not null + ? Path.Combine(dirLinkTargetPath, Path.GetFileName(path)) + : fileInfo.ResolveLinkTarget(returnFinalTarget: false)?.PhysicalPath; + return fileInfo.LastModified; } - private void OnChange(CertificateConfig certificateConfig, string path) + private void OnChange(string path, string originalPath) { + _logger.FileEventReceived(path, originalPath); + // Block until any in-progress updates are complete lock (_metadataLock) { - if (!ShouldFireChangeUnsynchronized(certificateConfig, path)) + if (!ShouldFireChangeUnsynchronized(path)) { return; } @@ -219,7 +303,7 @@ private void OnChange(CertificateConfig certificateConfig, string path) previousToken.OnReload(); } - private bool ShouldFireChangeUnsynchronized(CertificateConfig certificateConfig, string path) + private bool ShouldFireChangeUnsynchronized(string path) { if (!_metadataForFile.TryGetValue(path, out var fileMetadata)) { @@ -227,9 +311,21 @@ private bool ShouldFireChangeUnsynchronized(CertificateConfig certificateConfig, return false; } - // Existence implied by the fact that we're tracking the file + // If we found fileMetadata, there must be a containing/corresponding dirMetadata var dirMetadata = _metadataForDirectory[Path.GetDirectoryName(path)!]; + var lastModifiedTime = GetLastModifiedTimeAndLinkTargetPath(path, dirMetadata, out var linkTargetPath); + + // This usually indicate that we can't find the file, so we'll keep using the in-memory certificate + // rather than trying to reload and probably crashing + if (lastModifiedTime == DateTimeOffset.MinValue) + { + _logger.EventWithoutLastModifiedTime(path); + return false; + } + + var linkTargetPathChanged = linkTargetPath != fileMetadata.LinkTargetPath; + // We ignore file changes that don't advance the last modified time. // For example, if we lose access to the network share the file is // stored on, we don't notify our listeners because no one wants @@ -238,21 +334,24 @@ private bool ShouldFireChangeUnsynchronized(CertificateConfig certificateConfig, // before a new cert is introduced with the old name. // This also helps us in scenarios where the underlying file system // reports more than one change for a single logical operation. - var lastModifiedTime = GetLastModifiedTimeAndLinkTargetPath(path, dirMetadata.FileProvider, out var newLinkTargetPath); - if (lastModifiedTime > fileMetadata.LastModifiedTime) + // We make an exception if the link target has changed. + if (lastModifiedTime > fileMetadata.LastModifiedTime || linkTargetPathChanged) { + // Note that this might move backwards if the link target changed fileMetadata.LastModifiedTime = lastModifiedTime; + if (linkTargetPathChanged) + { + if (fileMetadata.LinkTargetPath is string oldLinkTargetPath) + { + fileMetadata.OldLinkTargetPaths.Add(oldLinkTargetPath); + } + fileMetadata.LinkTargetPath = linkTargetPath; + } } else { - if (lastModifiedTime == DateTimeOffset.MinValue) - { - _logger.EventWithoutLastModifiedTime(path); - } - else if (lastModifiedTime == fileMetadata.LastModifiedTime) + if (lastModifiedTime == fileMetadata.LastModifiedTime) { - Debug.Assert(newLinkTargetPath == fileMetadata.LinkTargetPath, "Link target changed without timestamp changing"); - _logger.RedundantEvent(path); } else @@ -263,51 +362,11 @@ private bool ShouldFireChangeUnsynchronized(CertificateConfig certificateConfig, return false; } - var configs = fileMetadata.Configs; - foreach (var config in configs) + foreach (var config in fileMetadata.Configs) { config.FileHasChanged = true; } - if (newLinkTargetPath != fileMetadata.LinkTargetPath) - { - // For correctness, we need to do the removal first, even though that might - // cause us to tear down some nodes that we will immediately re-add. - // For example, suppose you had a file A pointing to a file B, pointing to a file C: - // A -> B -> C - // If file A were updated to point to a new file D, which also pointed to C: - // A -> B -> C - // -> D -> - // Then adding a watch on D would no-op on recursively adding a watch on C, since its - // already monitored for A's CertificateConfig. - // Then removing a watch on B would tear down both B and C, even though D should still - // be keeping C alive. - // A -> D -> ?? - // Doing the removal first produces the correct result: - // A -> D -> C - // But it requires creating a watcher for C that is the same as the one that was removed - // (possibly including deleting and re-creating the directory watcher). Hopefully, - // such diamonds will be rare, in practice. - - // Also note that these updates are almost certainly redundant, since the change event - // we're about to fire will cause everything linked to, directly or indirectly, from - // certificateConfig.path to be updated in the next call to UpdateWatches. However, - // baking in the assumption as part of the contract for consuming this type would be - // unreasonable - it should make sense in its own right. - - if (fileMetadata.LinkTargetPath is string oldLinkTargetPath) - { - // We already have a rooted path, so call the worker - RemoveWatchWorkerUnsynchronized(certificateConfig, oldLinkTargetPath, logSummary: false); - } - - if (newLinkTargetPath is not null) - { - // We already have a rooted path, so call the worker - AddWatchWorkerUnsynchronized(certificateConfig, newLinkTargetPath, logSummary: false); - } - } - return true; } @@ -323,25 +382,21 @@ internal void RemoveWatchUnsynchronized(CertificateConfig certificateConfig) Debug.Assert(certificateConfig.IsFileCert, $"{nameof(RemoveWatchUnsynchronized)} called on non-file cert"); var path = Path.Combine(_contentRootDir, certificateConfig.Path); - RemoveWatchWorkerUnsynchronized(certificateConfig, path, logSummary: true); + RemoveWatchWorkerUnsynchronized(certificateConfig, path); } /// /// The patch of may not match (e.g. in the presence of symlinks). /// - private void RemoveWatchWorkerUnsynchronized(CertificateConfig certificateConfig, string path, bool logSummary) + private void RemoveWatchWorkerUnsynchronized(CertificateConfig certificateConfig, string path) { - var dir = Path.GetDirectoryName(path)!; - if (!_metadataForFile.TryGetValue(path, out var fileMetadata)) { _logger.UnknownFile(path); return; } - var configs = fileMetadata.Configs; - - if (!configs.Remove(certificateConfig)) + if (!fileMetadata.Configs.Remove(certificateConfig)) { _logger.UnknownObserver(path); return; @@ -349,38 +404,22 @@ private void RemoveWatchWorkerUnsynchronized(CertificateConfig certificateConfig _logger.RemovedObserver(path); - if (fileMetadata.LinkTargetPath is string linkTargetPath) - { - // We built this graph, so we already know there are no cycles - // Never log summary messages from recursive calls - RemoveWatchWorkerUnsynchronized(certificateConfig, linkTargetPath, logSummary: false); - } - - // If we found fileMetadata, there must be a containing/corresponding dirMetadata - var dirMetadata = _metadataForDirectory[dir]; - - if (configs.Count == 0) + var oldLinkTargetPaths = fileMetadata.OldLinkTargetPaths; + if (oldLinkTargetPaths.Count > 0) { - fileMetadata.Dispose(); - _metadataForFile.Remove(path); - dirMetadata.FileWatchCount--; - - _logger.RemovedFileWatcher(path); - - if (dirMetadata.FileWatchCount == 0) + foreach (var linkTargetPath in oldLinkTargetPaths) { - dirMetadata.Dispose(); - _metadataForDirectory.Remove(dir); - - _logger.RemovedDirectoryWatcher(dir); + RemoveWatchWorkerUnsynchronized(certificateConfig, linkTargetPath); } - } - if (logSummary) + // If there are OldLinkTargetpaths, we don't have to clean up LinkTargetPath because it hasn't been initialized + } + else if (fileMetadata.LinkTargetPath is string linkTargetPath) { - _logger.ObserverCount(path, configs.Count); - _logger.FileCount(dir, dirMetadata.FileWatchCount); + RemoveWatchWorkerUnsynchronized(certificateConfig, linkTargetPath); } + + fileMetadata.OldLinkTargetPaths.Clear(); } /// Test hook @@ -411,9 +450,10 @@ void IDisposable.Dispose() } } - private sealed class DirectoryWatchMetadata(IFileProviderWithLinkInfo fileProvider) : IDisposable + private sealed class DirectoryWatchMetadata(IFileProviderWithLinkInfo fileProvider, string? directoryName) : IDisposable { public readonly IFileProviderWithLinkInfo FileProvider = fileProvider; + public readonly string? DirectoryName = directoryName; // TODO (acasey): document public int FileWatchCount; public void Dispose() => (FileProvider as IDisposable)?.Dispose(); @@ -423,8 +463,9 @@ private sealed class FileWatchMetadata(IDisposable disposable) : IDisposable { public readonly IDisposable Disposable = disposable; public readonly HashSet Configs = new(ReferenceEqualityComparer.Instance); - public DateTimeOffset LastModifiedTime = DateTimeOffset.MinValue; + public readonly HashSet OldLinkTargetPaths = new(); public string? LinkTargetPath; + public DateTimeOffset LastModifiedTime = DateTimeOffset.MinValue; public void Dispose() => Disposable.Dispose(); } diff --git a/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcherLoggerExtensions.cs b/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcherLoggerExtensions.cs index afbb7b71a27e..59055f2dd2ea 100644 --- a/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcherLoggerExtensions.cs +++ b/src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcherLoggerExtensions.cs @@ -61,4 +61,7 @@ internal static partial class CertificatePathWatcherLoggerExtensions [LoggerMessage(18, LogLevel.Trace, "Flagged {Count} observers of '{Path}' as changed.", EventName = "FlaggedObservers")] public static partial void FlaggedObservers(this ILogger logger, string path, int count); + + [LoggerMessage(19, LogLevel.Trace, "Saw change event for '{OriginalPath}' (as '{Path}').", EventName = "FileEventReceived")] + public static partial void FileEventReceived(this ILogger logger, string path, string originalPath); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileProviderWithLinkInfo.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileProviderWithLinkInfo.cs index e3cf2d78fd56..b968009f0a4a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileProviderWithLinkInfo.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IFileProviderWithLinkInfo.cs @@ -10,13 +10,5 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; /// internal interface IFileProviderWithLinkInfo : IFileProvider { - /// - /// Returns the link target of the 's root directory. - /// - /// - /// Effectively . - /// - IFileInfoWithLinkInfo? ResolveLinkTarget(bool returnFinalTarget); - public new IFileInfoWithLinkInfo GetFileInfo(string subpath); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PhysicalFileProviderWithLinkInfo.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PhysicalFileProviderWithLinkInfo.cs index 7de987c7f65b..1a2ff8bcb646 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PhysicalFileProviderWithLinkInfo.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PhysicalFileProviderWithLinkInfo.cs @@ -29,18 +29,18 @@ IFileInfo IFileProvider.GetFileInfo(string subpath) IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string subpath) { - return new FileInfoWithLinkInfo(_inner.GetFileInfo(subpath)); - } - - IFileInfoWithLinkInfo? IFileProviderWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) - { - IFileInfoWithLinkInfo dirInfo = new DirectoryInfoWithLinkInfo(new DirectoryInfo(_inner.Root)); - return dirInfo.ResolveLinkTarget(returnFinalTarget); + var fileInfo = _inner.GetFileInfo(subpath); + if (!fileInfo.Exists && Directory.Exists(fileInfo.PhysicalPath)) + { + // https://github.com/dotnet/runtime/issues/36575 + fileInfo = new PhysicalDirectoryInfo(new DirectoryInfo(fileInfo.PhysicalPath)); + } + return new FileInfoWithLinkInfo(fileInfo); } private sealed class FileInfoWithLinkInfo : IFileInfoWithLinkInfo { - private readonly IFileInfo _inner; + private readonly IFileInfo _inner; // Could be either a file or a directory public FileInfoWithLinkInfo(IFileInfo inner) { @@ -69,52 +69,21 @@ public FileInfoWithLinkInfo(IFileInfo inner) return null; } - var fileInfo = new FileInfo(path); - - var linkFileSystemInfo = fileInfo.ResolveLinkTarget(returnFinalTarget); - - if (linkFileSystemInfo is not FileInfo linkInfo) - { - return null; - } - - return new FileInfoWithLinkInfo(new PhysicalFileInfo(linkInfo)); - } - } - private sealed class DirectoryInfoWithLinkInfo : IFileInfoWithLinkInfo - { - private readonly DirectoryInfo _directoryInfo; - - public DirectoryInfoWithLinkInfo(DirectoryInfo directoryInfo) - { - _directoryInfo = directoryInfo; - } - - bool IFileInfo.IsDirectory => true; + var fileSystemInfo = _inner.IsDirectory ? (FileSystemInfo)new DirectoryInfo(path) : new FileInfo(path); - bool IFileInfo.Exists => _directoryInfo.Exists; - DateTimeOffset IFileInfo.LastModified => _directoryInfo.LastWriteTimeUtc; - string IFileInfo.Name => _directoryInfo.Name; - string? IFileInfo.PhysicalPath => _directoryInfo.FullName; + var linkFileSystemInfo = fileSystemInfo.ResolveLinkTarget(returnFinalTarget); - long IFileInfo.Length => throw new NotSupportedException(); // We could probably just return 0, though that's not strictly accurate - Stream IFileInfo.CreateReadStream() => throw new NotSupportedException(); - - IFileInfoWithLinkInfo? IFileInfoWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) - { - if (!_directoryInfo.Exists) + if (linkFileSystemInfo is FileInfo linkFileInfo) { - return null; + return new FileInfoWithLinkInfo(new PhysicalFileInfo(linkFileInfo)); } - var linkFileSystemInfo = _directoryInfo.ResolveLinkTarget(returnFinalTarget); - - if (linkFileSystemInfo is not DirectoryInfo linkInfo) + if (linkFileSystemInfo is DirectoryInfo linkDirInfo) { - return null; + return new FileInfoWithLinkInfo(new PhysicalDirectoryInfo(linkDirInfo)); } - return new DirectoryInfoWithLinkInfo(linkInfo); + return null; } } } diff --git a/src/Servers/Kestrel/Core/test/CertificatePathWatcherTests.cs b/src/Servers/Kestrel/Core/test/CertificatePathWatcherTests.cs index c6bdd88e07a7..67ac1cd684ce 100644 --- a/src/Servers/Kestrel/Core/test/CertificatePathWatcherTests.cs +++ b/src/Servers/Kestrel/Core/test/CertificatePathWatcherTests.cs @@ -20,19 +20,21 @@ public class CertificatePathWatcherTests : LoggedTest [InlineData(false)] public void AddAndRemoveWatch(bool absoluteFilePath) { - var dir = Directory.GetCurrentDirectory(); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); var logger = LoggerFactory.CreateLogger(); - using var watcher = new CertificatePathWatcher(dir, logger, _ => NoChangeFileProvider.Instance); + using var watcher = new CertificatePathWatcher(rootDir, logger, _ => NoChangeFileProvider.Instance); var changeToken = watcher.GetChangeToken(); var certificateConfig = new CertificateConfig { - Path = absoluteFilePath ? filePath : fileName, + Path = absoluteFilePath ? filePath : fileSubpath, }; IDictionary messageProps; @@ -120,15 +122,18 @@ public void WatchMultipleDirectories(int dirCount, int fileCount) [InlineData(4)] public async Task FileChanged(int observerCount) { - var dir = Directory.GetCurrentDirectory(); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(dir); + var fileProvider = new MockFileProvider(rootDir); var fileLastModifiedTime = DateTimeOffset.UtcNow; - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(dirName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime); using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); @@ -153,8 +158,8 @@ public async Task FileChanged(int observerCount) Assert.Equal(observerCount, watcher.TestGetObserverCountUnsynchronized(filePath)); // Simulate file change on disk - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime.AddSeconds(1)); - fileProvider.FireChangeToken(fileName); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime.AddSeconds(1)); + fileProvider.FireChangeToken(fileSubpath); await signalTcs.Task.DefaultTimeout(); @@ -170,22 +175,25 @@ public async Task FileChanged(int observerCount) [Fact] public async Task FileReplacedWithLink() { - var dir = Directory.GetCurrentDirectory(); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); - var linkTargetName = Path.GetRandomFileName(); - var linkTargetPath = Path.Combine(dir, linkTargetName); + var linkTargetSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var linkTargetPath = Path.Combine(rootDir, linkTargetSubpath); var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(dir); + var fileProvider = new MockFileProvider(rootDir); var fileLastModifiedTime = DateTimeOffset.UtcNow; var linkTargetLastModifiedTime = fileLastModifiedTime.AddSeconds(-1); // target is *older* than original file - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); - fileProvider.SetLastModifiedTime(linkTargetName, linkTargetLastModifiedTime); + fileProvider.SetLastModifiedTime(dirName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(linkTargetSubpath, linkTargetLastModifiedTime); using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); @@ -207,9 +215,9 @@ public async Task FileReplacedWithLink() Assert.Equal(0, watcher.TestGetObserverCountUnsynchronized(linkTargetPath)); // Simulate file change on disk - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime.AddSeconds(1)); - fileProvider.SetLinkTarget(fileName, linkTargetName, fileProvider); - fileProvider.FireChangeToken(fileName); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime.AddSeconds(1)); + fileProvider.SetLinkTarget(fileSubpath, linkTargetSubpath, fileProvider); + fileProvider.FireChangeToken(fileSubpath); await signalTcs.Task.DefaultTimeout(); @@ -232,27 +240,30 @@ public async Task FileReplacedWithLink() [InlineData(false)] public async Task FileLinkChanged(bool updatedFileIsLink) { - var dir = Directory.GetCurrentDirectory(); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); - var oldLinkTargetName = Path.GetRandomFileName(); - var oldLinkTargetPath = Path.Combine(dir, oldLinkTargetName); + var oldLinkTargetSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var oldLinkTargetPath = Path.Combine(rootDir, oldLinkTargetSubpath); - var newLinkTargetName = Path.GetRandomFileName(); - var newLinkTargetPath = Path.Combine(dir, newLinkTargetName); + var newLinkTargetSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var newLinkTargetPath = Path.Combine(rootDir, newLinkTargetSubpath); var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(dir); + var fileProvider = new MockFileProvider(rootDir); var fileLastModifiedTime = DateTimeOffset.UtcNow; - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); - fileProvider.SetLastModifiedTime(oldLinkTargetName, fileLastModifiedTime); - fileProvider.SetLastModifiedTime(newLinkTargetName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(dirName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(oldLinkTargetSubpath, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(newLinkTargetSubpath, fileLastModifiedTime); - fileProvider.SetLinkTarget(fileName, oldLinkTargetName, fileProvider); + fileProvider.SetLinkTarget(fileSubpath, oldLinkTargetSubpath, fileProvider); using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); @@ -275,16 +286,16 @@ public async Task FileLinkChanged(bool updatedFileIsLink) Assert.Equal(0, watcher.TestGetObserverCountUnsynchronized(newLinkTargetPath)); // Simulate file change on disk - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime.AddSeconds(1)); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime.AddSeconds(1)); if (updatedFileIsLink) { - fileProvider.SetLinkTarget(fileName, newLinkTargetName, fileProvider); + fileProvider.SetLinkTarget(fileSubpath, newLinkTargetSubpath, fileProvider); } else { - fileProvider.RemoveLinkTarget(fileName); + fileProvider.RemoveLinkTarget(fileSubpath); } - fileProvider.FireChangeToken(fileName); + fileProvider.FireChangeToken(fileSubpath); await signalTcs.Task.DefaultTimeout(); @@ -306,23 +317,26 @@ public async Task FileLinkChanged(bool updatedFileIsLink) [Fact] public async Task FileLinkTargetChanged() { - var dir = Directory.GetCurrentDirectory(); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); - var linkTargetName = Path.GetRandomFileName(); - var linkTargetPath = Path.Combine(dir, linkTargetName); + var linkTargetSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var linkTargetPath = Path.Combine(rootDir, linkTargetSubpath); var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(dir); + var fileProvider = new MockFileProvider(rootDir); var fileLastModifiedTime = DateTimeOffset.UtcNow; - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); - fileProvider.SetLastModifiedTime(linkTargetName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(dirName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(linkTargetSubpath, fileLastModifiedTime); - fileProvider.SetLinkTarget(fileName, linkTargetName, fileProvider); + fileProvider.SetLinkTarget(fileSubpath, linkTargetSubpath, fileProvider); using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); @@ -344,9 +358,9 @@ public async Task FileLinkTargetChanged() Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(linkTargetPath)); // Simulate file change on disk - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime.AddSeconds(1)); - fileProvider.SetLinkTarget(fileName, linkTargetName, fileProvider); - fileProvider.FireChangeToken(fileName); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime.AddSeconds(1)); + fileProvider.SetLinkTarget(fileSubpath, linkTargetSubpath, fileProvider); + fileProvider.FireChangeToken(fileSubpath); await signalTcs.Task.DefaultTimeout(); @@ -371,26 +385,30 @@ public async Task FileLinkTargetChanged() [LogLevel(LogLevel.Trace)] public void FileLinkCycle(int cycleLength) { - var dir = Directory.GetCurrentDirectory(); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); - var fileNames = new string[cycleLength]; + var fileSubpaths = new string[cycleLength]; var filePaths = new string[cycleLength]; var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(dir); + var fileProvider = new MockFileProvider(rootDir); var fileLastModifiedTime = DateTimeOffset.UtcNow; + fileProvider.SetLastModifiedTime(dirName, fileLastModifiedTime); + for (int i = 0; i < cycleLength; i++) { - fileNames[i] = Path.GetRandomFileName(); - filePaths[i] = Path.Combine(dir, fileNames[i]); - fileProvider.SetLastModifiedTime(fileNames[i], fileLastModifiedTime); + fileSubpaths[i] = Path.Combine(dirName, Path.GetRandomFileName()); + filePaths[i] = Path.Combine(rootDir, fileSubpaths[i]); + fileProvider.SetLastModifiedTime(fileSubpaths[i], fileLastModifiedTime); } for (int i = 0; i < cycleLength; i++) { - fileProvider.SetLinkTarget(fileNames[i], fileNames[(i + 1) % cycleLength], fileProvider); + fileProvider.SetLinkTarget(fileSubpaths[i], fileSubpaths[(i + 1) % cycleLength], fileProvider); } using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); @@ -422,15 +440,19 @@ public void FileLinkCycle(int cycleLength) [Fact] public async Task OutOfOrderLastModifiedTime() { - var dir = Directory.GetCurrentDirectory(); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(dir); + var fileProvider = new MockFileProvider(rootDir); var fileLastModifiedTime = DateTimeOffset.UtcNow; - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); + + fileProvider.SetLastModifiedTime(dirName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime); using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); @@ -458,8 +480,8 @@ public async Task OutOfOrderLastModifiedTime() Assert.Equal(1, watcher.TestGetObserverCountUnsynchronized(filePath)); // Simulate file change on disk - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime.AddSeconds(-1)); - fileProvider.FireChangeToken(fileName); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime.AddSeconds(-1)); + fileProvider.FireChangeToken(fileSubpath); await logTcs.Task.DefaultTimeout(); @@ -496,9 +518,11 @@ public void DirectoryDoesNotExist() [InlineData(false)] public void RemoveUnknownFileWatch(bool previouslyAdded) { - var dir = Directory.GetCurrentDirectory(); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); var logger = LoggerFactory.CreateLogger(); @@ -530,9 +554,11 @@ public void RemoveUnknownFileWatch(bool previouslyAdded) [InlineData(false)] public void RemoveUnknownFileObserver(bool previouslyAdded) { - var dir = Directory.GetCurrentDirectory(); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); var logger = LoggerFactory.CreateLogger(); @@ -570,9 +596,11 @@ public void RemoveUnknownFileObserver(bool previouslyAdded) [LogLevel(LogLevel.Trace)] public void ReuseFileObserver() { - var dir = Directory.GetCurrentDirectory(); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); var logger = LoggerFactory.CreateLogger(); @@ -603,15 +631,19 @@ public void ReuseFileObserver() [LogLevel(LogLevel.Trace)] public async Task IgnoreDeletion(bool seeChangeForDeletion, bool restoredWithNewerLastModifiedTime) { - var dir = Directory.GetCurrentDirectory(); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); var logger = LoggerFactory.CreateLogger(); - var fileProvider = new MockFileProvider(dir); + var fileProvider = new MockFileProvider(rootDir); var fileLastModifiedTime = DateTimeOffset.UtcNow; - fileProvider.SetLastModifiedTime(fileName, fileLastModifiedTime); + + fileProvider.SetLastModifiedTime(dirName, fileLastModifiedTime); + fileProvider.SetLastModifiedTime(fileSubpath, fileLastModifiedTime); using var watcher = new CertificatePathWatcher(dir, logger, _ => fileProvider); @@ -646,12 +678,12 @@ public async Task IgnoreDeletion(bool seeChangeForDeletion, bool restoredWithNew }; // Simulate file deletion - fileProvider.SetLastModifiedTime(fileName, null); + fileProvider.SetLastModifiedTime(fileSubpath, null); // In some file systems and watch modes, there's no event when (e.g.) the directory containing the watched file is deleted if (seeChangeForDeletion) { - fileProvider.FireChangeToken(fileName); + fileProvider.FireChangeToken(fileSubpath); await logNoLastModifiedTcs.Task.DefaultTimeout(); } @@ -663,8 +695,8 @@ public async Task IgnoreDeletion(bool seeChangeForDeletion, bool restoredWithNew Assert.False(changeTcs.Task.IsCompleted); // Restore the file - fileProvider.SetLastModifiedTime(fileName, restoredWithNewerLastModifiedTime ? fileLastModifiedTime.AddSeconds(1) : fileLastModifiedTime); - fileProvider.FireChangeToken(fileName); + fileProvider.SetLastModifiedTime(fileSubpath, restoredWithNewerLastModifiedTime ? fileLastModifiedTime.AddSeconds(1) : fileLastModifiedTime); + fileProvider.FireChangeToken(fileSubpath); if (restoredWithNewerLastModifiedTime) { @@ -681,9 +713,11 @@ public async Task IgnoreDeletion(bool seeChangeForDeletion, bool restoredWithNew [Fact] public void UpdateWatches() { - var dir = Directory.GetCurrentDirectory(); - var fileName = Path.GetRandomFileName(); - var filePath = Path.Combine(dir, fileName); + var rootDir = Directory.GetCurrentDirectory(); + var dirName = Path.GetRandomFileName(); + var dir = Path.Combine(rootDir, dirName); + var fileSubpath = Path.Combine(dirName, Path.GetRandomFileName()); + var filePath = Path.Combine(rootDir, fileSubpath); var logger = LoggerFactory.CreateLogger(); @@ -771,7 +805,6 @@ private NoChangeFileProvider() IChangeToken IFileProvider.Watch(string filter) => NoChangeChangeToken.Instance; IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string subpath) => FixedTimeFileInfoWithLinkInfo.Instance; - IFileInfoWithLinkInfo IFileProviderWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) => null; private sealed class NoChangeChangeToken : IChangeToken { @@ -835,27 +868,27 @@ public MockFileProvider(string rootPath) _rootPath = rootPath; } - public void FireChangeToken(string fileName) + public void FireChangeToken(string subpath) { - var oldChangeToken = _changeTokens[fileName]; - _changeTokens[fileName] = new ConfigurationReloadToken(); + var oldChangeToken = _changeTokens[subpath]; + _changeTokens[subpath] = new ConfigurationReloadToken(); oldChangeToken.OnReload(); } - public void SetLastModifiedTime(string fileName, DateTimeOffset? lastModifiedTime) + public void SetLastModifiedTime(string subpath, DateTimeOffset? lastModifiedTime) { - _lastModifiedTimes[fileName] = lastModifiedTime; + _lastModifiedTimes[subpath] = lastModifiedTime; } - public void RemoveLinkTarget(string fileName) + public void RemoveLinkTarget(string subpath) { - _linkTargets.Remove(fileName); + _linkTargets.Remove(subpath); } - public void SetLinkTarget(string fileName, string linkTargetName, IFileProviderWithLinkInfo linkTargetFileProvider) + public void SetLinkTarget(string fileSubpath, string linkTargetSubpath, IFileProviderWithLinkInfo linkTargetFileProvider) { - var targetInfo = linkTargetFileProvider.GetFileInfo(linkTargetName); - _linkTargets[fileName] = targetInfo; + var targetInfo = linkTargetFileProvider.GetFileInfo(linkTargetSubpath); + _linkTargets[fileSubpath] = targetInfo; } IDirectoryContents IFileProvider.GetDirectoryContents(string subpath) @@ -863,29 +896,24 @@ IDirectoryContents IFileProvider.GetDirectoryContents(string subpath) throw new NotSupportedException(); } - IChangeToken IFileProvider.Watch(string fileName) + IChangeToken IFileProvider.Watch(string subpath) { - if (!_changeTokens.TryGetValue(fileName, out var changeToken)) + if (!_changeTokens.TryGetValue(subpath, out var changeToken)) { - _changeTokens[fileName] = changeToken = new ConfigurationReloadToken(); + _changeTokens[subpath] = changeToken = new ConfigurationReloadToken(); } return changeToken; } - IFileInfo IFileProvider.GetFileInfo(string fileName) - { - return ((IFileProviderWithLinkInfo)this).GetFileInfo(fileName); - } - - IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string fileName) + IFileInfo IFileProvider.GetFileInfo(string subpath) { - return new MockFileInfoWithLinkInfo(Path.Combine(_rootPath, fileName), _lastModifiedTimes[fileName], _linkTargets.TryGetValue(fileName, out var target) ? target : null); + return ((IFileProviderWithLinkInfo)this).GetFileInfo(subpath); } - IFileInfoWithLinkInfo IFileProviderWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) + IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string subpath) { - return null; + return new MockFileInfoWithLinkInfo(Path.Combine(_rootPath, subpath), _lastModifiedTimes[subpath], _linkTargets.TryGetValue(subpath, out var target) ? target : null); } } @@ -913,21 +941,4 @@ public MockFileInfoWithLinkInfo(string path, DateTimeOffset? lastModifiedTime, I IFileInfoWithLinkInfo IFileInfoWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) => _linkTarget; } - - private sealed class MockLinkFileProvider : IFileProviderWithLinkInfo - { - private readonly IFileInfoWithLinkInfo _linkTarget; - - public MockLinkFileProvider(IFileInfoWithLinkInfo linkTarget) - { - _linkTarget = linkTarget; - } - - IChangeToken IFileProvider.Watch(string filter) => throw new NotSupportedException(); - IDirectoryContents IFileProvider.GetDirectoryContents(string subpath) => throw new NotSupportedException(); - IFileInfo IFileProvider.GetFileInfo(string subpath) => throw new NotSupportedException(); - IFileInfoWithLinkInfo IFileProviderWithLinkInfo.GetFileInfo(string subpath) => throw new NotSupportedException(); - - IFileInfoWithLinkInfo IFileProviderWithLinkInfo.ResolveLinkTarget(bool returnFinalTarget) => _linkTarget; - } }