From 80e9fc0c43f72710737cf49b0692aee2849588ee Mon Sep 17 00:00:00 2001 From: Jaime Bernardo Date: Fri, 25 Mar 2022 10:08:10 +0000 Subject: [PATCH] [PTRun][Program]Fix bug when renaming url shortcut (#17184) * [PTRun][Program]Fix bug when renaming url shortcut * Try to use the old full path for old app removal * Guard against links to nowhere * Fix test to have link point to existing location * Update src/modules/launcher/Plugins/Microsoft.Plugin.Program/Programs/Win32Program.cs Co-authored-by: Heiko <61519853+htcfreek@users.noreply.github.com> * Fix nit. Co-authored-by: Heiko <61519853+htcfreek@users.noreply.github.com> --- .../Storage/Win32ProgramRepositoryTest.cs | 7 ++++--- .../Programs/Win32Program.cs | 10 ++++++++++ .../Storage/Win32ProgramRepository.cs | 17 +++++++++-------- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/modules/launcher/Plugins/Microsoft.Plugin.Program.UnitTests/Storage/Win32ProgramRepositoryTest.cs b/src/modules/launcher/Plugins/Microsoft.Plugin.Program.UnitTests/Storage/Win32ProgramRepositoryTest.cs index ed7135d098..b6c29050c6 100644 --- a/src/modules/launcher/Plugins/Microsoft.Plugin.Program.UnitTests/Storage/Win32ProgramRepositoryTest.cs +++ b/src/modules/launcher/Plugins/Microsoft.Plugin.Program.UnitTests/Storage/Win32ProgramRepositoryTest.cs @@ -352,10 +352,11 @@ namespace Microsoft.Plugin.Program.UnitTests.Storage string oldFullPath = directory + "\\" + oldpath; string fullPath = directory + "\\" + path; + string linkingTo = Directory.GetCurrentDirectory(); // ShellLinkHelper must be mocked for lnk applications var mockShellLink = new Mock(); - mockShellLink.Setup(m => m.RetrieveTargetPath(It.IsAny())).Returns(string.Empty); + mockShellLink.Setup(m => m.RetrieveTargetPath(It.IsAny())).Returns(linkingTo); Win32Program.ShellLinkHelper = mockShellLink.Object; // old item and new item are the actual items when they are in existence @@ -363,14 +364,14 @@ namespace Microsoft.Plugin.Program.UnitTests.Storage { Name = "oldpath", ExecutableName = oldpath, - FullPath = fullPath, + FullPath = linkingTo, }; Win32Program newitem = new Win32Program { Name = "path", ExecutableName = path, - FullPath = fullPath, + FullPath = linkingTo, }; win32ProgramRepository.Add(olditem); diff --git a/src/modules/launcher/Plugins/Microsoft.Plugin.Program/Programs/Win32Program.cs b/src/modules/launcher/Plugins/Microsoft.Plugin.Program/Programs/Win32Program.cs index fd3a009daa..1594bbf08b 100644 --- a/src/modules/launcher/Plugins/Microsoft.Plugin.Program/Programs/Win32Program.cs +++ b/src/modules/launcher/Plugins/Microsoft.Plugin.Program/Programs/Win32Program.cs @@ -491,9 +491,19 @@ namespace Microsoft.Plugin.Program.Programs } } } + else + { + // If the link points nowhere, consider it invalid. + return InvalidProgram; + } return program; } + catch (System.IO.FileLoadException e) + { + ProgramLogger.Warn($"Couldn't load the link file at {path}. This might be caused by a new link being created and locked by the OS.", e, MethodBase.GetCurrentMethod().DeclaringType, path); + return InvalidProgram; + } // Only do a catch all in production. This is so make developer aware of any unhandled exception and add the exception handling in. // Error caused likely due to trying to get the description of the program diff --git a/src/modules/launcher/Plugins/Microsoft.Plugin.Program/Storage/Win32ProgramRepository.cs b/src/modules/launcher/Plugins/Microsoft.Plugin.Program/Storage/Win32ProgramRepository.cs index 0fb3fad8df..656725b0f6 100644 --- a/src/modules/launcher/Plugins/Microsoft.Plugin.Program/Storage/Win32ProgramRepository.cs +++ b/src/modules/launcher/Plugins/Microsoft.Plugin.Program/Storage/Win32ProgramRepository.cs @@ -99,7 +99,7 @@ namespace Microsoft.Plugin.Program.Storage string newPath = e.FullPath; string extension = Path.GetExtension(newPath); - Win32Program.ApplicationType appType = Win32Program.GetAppTypeFromPath(newPath); + Win32Program.ApplicationType oldAppType = Win32Program.GetAppTypeFromPath(oldPath); Programs.Win32Program newApp = Win32Program.GetAppFromPath(newPath); Programs.Win32Program oldApp = null; @@ -109,13 +109,9 @@ namespace Microsoft.Plugin.Program.Storage // This situation is not encountered for other application types because the fullPath is the path itself, instead of being computed by using the path to the app. try { - if (appType == Win32Program.ApplicationType.ShortcutApplication) + if (oldAppType == Win32Program.ApplicationType.ShortcutApplication || oldAppType == Win32Program.ApplicationType.InternetShortcutApplication) { - oldApp = new Win32Program() { Name = Path.GetFileNameWithoutExtension(e.OldName), ExecutableName = Path.GetFileName(e.OldName), FullPath = newApp.FullPath }; - } - else if (appType == Win32Program.ApplicationType.InternetShortcutApplication) - { - oldApp = new Win32Program() { Name = Path.GetFileNameWithoutExtension(e.OldName), ExecutableName = Path.GetFileName(e.OldName), FullPath = newApp.FullPath }; + oldApp = new Win32Program() { Name = Path.GetFileNameWithoutExtension(e.OldName), ExecutableName = Path.GetFileName(e.OldName), FullPath = newApp?.FullPath ?? oldPath }; } else { @@ -132,7 +128,7 @@ namespace Microsoft.Plugin.Program.Storage { if (string.IsNullOrWhiteSpace(oldApp.Name) || string.IsNullOrWhiteSpace(oldApp.ExecutableName) || string.IsNullOrWhiteSpace(oldApp.FullPath)) { - Log.Error($"Old app was not initialized properly. OldFullPath: {e.OldFullPath}; OldName: {e.OldName}; FullPath: {e.FullPath}", GetType()); + Log.Warn($"Old app data was not initialized properly for removal after file renaming. This likely means it was not a valid app to begin with and removal is not needed. OldFullPath: {e.OldFullPath}; OldName: {e.OldName}; FullPath: {e.FullPath}", GetType()); } else { @@ -160,6 +156,11 @@ namespace Microsoft.Plugin.Program.Storage if (extension.Equals(LnkExtension, StringComparison.OrdinalIgnoreCase)) { app = GetAppWithSameLnkResolvedPath(path); + if (app == null) + { + // Cancelled links won't have a resolved path. + app = GetAppWithSameNameAndExecutable(Path.GetFileNameWithoutExtension(path), Path.GetFileName(path)); + } } else if (extension.Equals(UrlExtension, StringComparison.OrdinalIgnoreCase)) {