From 6c09565244c39b9c784587970e322a01e253ffc9 Mon Sep 17 00:00:00 2001 From: Davide Giacometti Date: Fri, 8 Sep 2023 17:25:36 +0200 Subject: [PATCH] [Hosts]Improved save error handling (#28215) --- .github/actions/spell-check/expect.txt | 1 + .../Hosts/Hosts.Tests/HostsServiceTest.cs | 12 ++++ .../Hosts/Hosts/Helpers/HostsService.cs | 11 +--- .../Hosts/Hosts/Helpers/IHostsService.cs | 2 +- .../Helpers/NotRunningElevatedException.cs | 12 ++++ .../Hosts/Hosts/HostsXAML/Views/MainPage.xaml | 1 + .../Hosts/Hosts/Strings/en-us/Resources.resw | 12 +++- .../Hosts/Hosts/ViewModels/MainViewModel.cs | 58 ++++++++++++++----- 8 files changed, 81 insertions(+), 28 deletions(-) create mode 100644 src/modules/Hosts/Hosts/Helpers/NotRunningElevatedException.cs diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index a4621083ce..373fd2e106 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -1849,6 +1849,7 @@ subquery subresource Superbar sut +svchost SVE SVGIn SVGIO diff --git a/src/modules/Hosts/Hosts.Tests/HostsServiceTest.cs b/src/modules/Hosts/Hosts.Tests/HostsServiceTest.cs index 268de099f7..c75c0ce1b6 100644 --- a/src/modules/Hosts/Hosts.Tests/HostsServiceTest.cs +++ b/src/modules/Hosts/Hosts.Tests/HostsServiceTest.cs @@ -238,5 +238,17 @@ namespace Hosts.Tests var result = fileSystem.GetFile(service.HostsFilePath); Assert.AreEqual(result.TextContents, contentResult); } + + [TestMethod] + public async Task Save_NotRunningElevatedException() + { + var fileSystem = new CustomMockFileSystem(); + var userSettings = new Mock(); + var elevationHelper = new Mock(); + elevationHelper.Setup(m => m.IsElevated).Returns(false); + + var service = new HostsService(fileSystem, userSettings.Object, elevationHelper.Object); + await Assert.ThrowsExceptionAsync(async () => await service.WriteAsync("# Empty hosts file", Enumerable.Empty())); + } } } diff --git a/src/modules/Hosts/Hosts/Helpers/HostsService.cs b/src/modules/Hosts/Hosts/Helpers/HostsService.cs index ed698ea7af..c6eaada406 100644 --- a/src/modules/Hosts/Hosts/Helpers/HostsService.cs +++ b/src/modules/Hosts/Hosts/Helpers/HostsService.cs @@ -122,11 +122,11 @@ namespace Hosts.Helpers return new HostsData(entries, unparsedBuilder.ToString(), splittedEntries); } - public async Task WriteAsync(string additionalLines, IEnumerable entries) + public async Task WriteAsync(string additionalLines, IEnumerable entries) { if (!_elevationHelper.IsElevated) { - return false; + throw new NotRunningElevatedException(); } var lines = new List(); @@ -195,18 +195,11 @@ namespace Hosts.Helpers await _fileSystem.File.WriteAllLinesAsync(HostsFilePath, lines, Encoding); } - catch (Exception ex) - { - Logger.LogError("Failed to write hosts file", ex); - return false; - } finally { _fileSystemWatcher.EnableRaisingEvents = true; _asyncLock.Release(); } - - return true; } public async Task PingAsync(string address) diff --git a/src/modules/Hosts/Hosts/Helpers/IHostsService.cs b/src/modules/Hosts/Hosts/Helpers/IHostsService.cs index 71e3e4b727..b9a14d1de8 100644 --- a/src/modules/Hosts/Hosts/Helpers/IHostsService.cs +++ b/src/modules/Hosts/Hosts/Helpers/IHostsService.cs @@ -17,7 +17,7 @@ namespace Hosts.Helpers Task ReadAsync(); - Task WriteAsync(string additionalLines, IEnumerable entries); + Task WriteAsync(string additionalLines, IEnumerable entries); Task PingAsync(string address); diff --git a/src/modules/Hosts/Hosts/Helpers/NotRunningElevatedException.cs b/src/modules/Hosts/Hosts/Helpers/NotRunningElevatedException.cs new file mode 100644 index 0000000000..54d2aee710 --- /dev/null +++ b/src/modules/Hosts/Hosts/Helpers/NotRunningElevatedException.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +namespace Hosts.Helpers +{ + public class NotRunningElevatedException : Exception + { + } +} diff --git a/src/modules/Hosts/Hosts/HostsXAML/Views/MainPage.xaml b/src/modules/Hosts/Hosts/HostsXAML/Views/MainPage.xaml index 28f4e836bf..25a9f725ef 100644 --- a/src/modules/Hosts/Hosts/HostsXAML/Views/MainPage.xaml +++ b/src/modules/Hosts/Hosts/HostsXAML/Views/MainPage.xaml @@ -390,6 +390,7 @@ x:Uid="FileSaveError" Margin="0,8,0,0" Severity="Error" + Message="{x:Bind ViewModel.ErrorMessage, Mode=TwoWay}" IsOpen="{x:Bind ViewModel.Error, Mode=TwoWay}" Visibility="{x:Bind ViewModel.Error, Mode=TwoWay, Converter={StaticResource BoolToVisibilityConverter}}" /> diff --git a/src/modules/Hosts/Hosts/Strings/en-us/Resources.resw b/src/modules/Hosts/Hosts/Strings/en-us/Resources.resw index 1eb20bc5db..b17bbab0fc 100644 --- a/src/modules/Hosts/Hosts/Strings/en-us/Resources.resw +++ b/src/modules/Hosts/Hosts/Strings/en-us/Resources.resw @@ -216,8 +216,16 @@ Hosts file was modified externally. "Hosts" refers to the system hosts file, do not loc - - Failed to save hosts file. + + The hosts file cannot be saved because it is being used by another process. + "Hosts" refers to the system hosts file, do not loc + + + Unable to save the hosts file. + "Hosts" refers to the system hosts file, do not loc + + + The hosts file cannot be saved because the program isn't running as administrator. "Hosts" refers to the system hosts file, do not loc diff --git a/src/modules/Hosts/Hosts/ViewModels/MainViewModel.cs b/src/modules/Hosts/Hosts/ViewModels/MainViewModel.cs index 6eb536556f..816f64bb23 100644 --- a/src/modules/Hosts/Hosts/ViewModels/MainViewModel.cs +++ b/src/modules/Hosts/Hosts/ViewModels/MainViewModel.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.IO; using System.Linq; using System.Linq.Expressions; using System.Threading; @@ -44,6 +45,9 @@ namespace Hosts.ViewModels [ObservableProperty] private bool _error; + [ObservableProperty] + private string _errorMessage; + [ObservableProperty] private bool _fileChanged; @@ -126,12 +130,7 @@ namespace Hosts.ViewModels public void UpdateAdditionalLines(string lines) { AdditionalLines = lines; - - Task.Run(async () => - { - var error = !await _hostsService.WriteAsync(AdditionalLines, _entries); - await _dispatcherQueue.EnqueueAsync(() => Error = error); - }); + _ = Task.Run(SaveAsync); } public void Move(int oldIndex, int newIndex) @@ -287,20 +286,12 @@ namespace Hosts.ViewModels return; } - Task.Run(async () => - { - var error = !await _hostsService.WriteAsync(AdditionalLines, _entries); - await _dispatcherQueue.EnqueueAsync(() => Error = error); - }); + _ = Task.Run(SaveAsync); } private void Entries_CollectionChanged(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e) { - Task.Run(async () => - { - var error = !await _hostsService.WriteAsync(AdditionalLines, _entries); - await _dispatcherQueue.EnqueueAsync(() => Error = error); - }); + _ = Task.Run(SaveAsync); } private void FindDuplicates(CancellationToken cancellationToken) @@ -379,6 +370,41 @@ namespace Hosts.ViewModels }); } + private async Task SaveAsync() + { + bool error = true; + string errorMessage = string.Empty; + + try + { + await _hostsService.WriteAsync(AdditionalLines, _entries); + error = false; + } + catch (NotRunningElevatedException) + { + var resourceLoader = ResourceLoaderInstance.ResourceLoader; + errorMessage = resourceLoader.GetString("FileSaveError_NotElevated"); + } + catch (IOException ex) when ((ex.HResult & 0x0000FFFF) == 32) + { + // There are some edge cases where a big hosts file is being locked by svchost.exe https://github.com/microsoft/PowerToys/issues/28066 + var resourceLoader = ResourceLoaderInstance.ResourceLoader; + errorMessage = resourceLoader.GetString("FileSaveError_FileInUse"); + } + catch (Exception ex) + { + Logger.LogError("Failed to save hosts file", ex); + var resourceLoader = ResourceLoaderInstance.ResourceLoader; + errorMessage = resourceLoader.GetString("FileSaveError_Generic"); + } + + await _dispatcherQueue.EnqueueAsync(() => + { + Error = error; + ErrorMessage = errorMessage; + }); + } + protected virtual void Dispose(bool disposing) { if (!_disposed)