From 1808d02fdcb8df24ea4e403947838eea7d94e1f1 Mon Sep 17 00:00:00 2001 From: mfkeeq <546357529@qq.com> Date: Fri, 23 Oct 2026 15:17:36 +0800 Subject: [PATCH 1/4] Resolve Windows shortcuts (.lnk) to their target file when opening Opening or dropping a shortcut now opens the log file it points at instead of the shortcut itself. Resolution uses the shell's own IShellLink; UNC and device targets as well as oversized files are not followed and open as regular files. --- .../Services/FileOperationServiceTests.cs | 98 +++++++++++ .../Services/ShortcutResolverTests.cs | 96 +++++++++++ .../FileOperationService.cs | 30 +++- .../FileOperationService/ShortcutResolver.cs | 161 ++++++++++++++++++ 4 files changed, 379 insertions(+), 6 deletions(-) create mode 100644 src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs create mode 100644 src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs diff --git a/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs b/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs index 2625fb94..329bba63 100644 --- a/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs +++ b/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs @@ -657,6 +657,71 @@ public void LoadFilesWithOption_SingleFile_CallsAddFileTab () Assert.That(_factoryCalls[0].Request.FileName, Is.EqualTo("test.log")); } + [Test] + public void LoadFilesWithOption_ShortcutToLxjFile_CallsProjectCallback () + { + // The .lxj check has to run on the resolved name, or the shortcut gets opened as a raw log. + var (directory, shortcut, session) = CreateShortcutToSession(); + + try + { + var decision = _sut.LoadFilesWithOption(new[] { shortcut }, false); + + Assert.That(decision, Is.EqualTo(MultiFileDecision.Cancel)); + Assert.That(_projectCallbackCalls, Has.Count.EqualTo(1)); + Assert.That(_projectCallbackCalls[0].FileName, Is.EqualTo(session)); + Assert.That(_factoryCalls, Is.Empty, "Should not open the shortcut as a log"); + } + finally + { + Directory.Delete(directory, true); + } + } + + [Test] + public void AddFileTabs_ShortcutToLxjFile_CallsProjectCallback () + { + var (directory, shortcut, session) = CreateShortcutToSession(); + + try + { + _sut.AddFileTabs(new[] { shortcut }); + + Assert.That(_projectCallbackCalls, Has.Count.EqualTo(1)); + Assert.That(_projectCallbackCalls[0].FileName, Is.EqualTo(session)); + Assert.That(_factoryCalls, Is.Empty, "Should not open the shortcut as a log"); + } + finally + { + Directory.Delete(directory, true); + } + } + + [Test] + public void AddFileTab_ShortcutToLogFile_OpensTarget () + { + var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", "ShortcutLog", Guid.NewGuid().ToString()); + _ = Directory.CreateDirectory(directory); + + try + { + var log = Path.Join(directory, "actual.log"); + File.WriteAllText(log, "line1"); + + var shortcut = Path.Join(directory, "actual.lnk"); + CreateShortcut(shortcut, log); + + _sut.AddFileTab(new FileTabRequest { FileName = shortcut }); + + Assert.That(_factoryCalls, Has.Count.EqualTo(1)); + Assert.That(_factoryCalls[0].Request.FileName, Is.EqualTo(log), "the target file is opened, not the shortcut"); + } + finally + { + Directory.Delete(directory, true); + } + } + [Test] public void LoadFilesWithOption_SingleLxjFile_CallsProjectCallback () { @@ -1225,6 +1290,39 @@ public void LoadFiles_DelegatesToAddFileTabs () Assert.That(_factoryCalls[1].Request.FileName, Is.EqualTo("b.log")); } + /// + /// Creates a temporary directory holding a session file and a shortcut to it, and returns the + /// three paths. The caller owns the directory and deletes it. + /// + private static (string Directory, string Shortcut, string Session) CreateShortcutToSession () + { + var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", "ShortcutSession", Guid.NewGuid().ToString()); + _ = Directory.CreateDirectory(directory); + + var session = Path.Join(directory, "project.lxj"); + File.WriteAllText(session, ""); + + var shortcut = Path.Join(directory, "session.lnk"); + CreateShortcut(shortcut, session); + + return (directory, shortcut, session); + } + + /// + /// Creates a real shortcut through the shell's WScript.Shell automation object, so the tests + /// exercise the same files Explorer writes. + /// + private static void CreateShortcut (string shortcutPath, string targetPath) + { + var shellType = Type.GetTypeFromProgID("WScript.Shell"); + Assert.That(shellType, Is.Not.Null, "WScript.Shell should be available on Windows test machines"); + + dynamic shell = Activator.CreateInstance(shellType)!; + dynamic shortcut = shell.CreateShortcut(shortcutPath); + shortcut.TargetPath = targetPath; + shortcut.Save(); + } + /// /// Builds a second service on the shared config manager, standing in for another LogTabWindow /// of the same session with its own tab controller. diff --git a/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs b/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs new file mode 100644 index 00000000..7e2071e9 --- /dev/null +++ b/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs @@ -0,0 +1,96 @@ +using System.Runtime.Versioning; + +using LogExpert.UI.Services.FileOperationService; + +using NUnit.Framework; + +namespace LogExpert.UI.Tests.Services; + +/// +/// Tests the guard rails of : oversized files and targets off the +/// local machine are not followed, the shortcut keeps opening as a regular file instead. +/// +[TestFixture] +[SupportedOSPlatform("windows")] +internal class ShortcutResolverTests +{ + [Test] + public void Resolve_ShortcutToLocalTarget_ReturnsTarget () + { + var directory = CreateTempDirectory("ShortcutLocal"); + + try + { + var log = Path.Join(directory, "actual.log"); + File.WriteAllText(log, "line1"); + + var shortcut = Path.Join(directory, "actual.lnk"); + CreateShortcut(shortcut, log); + + Assert.That(ShortcutResolver.Resolve(shortcut), Is.EqualTo(log)); + } + finally + { + Directory.Delete(directory, true); + } + } + + [Test] + public void Resolve_ShortcutToUncTarget_ReturnsShortcutUnchanged () + { + var directory = CreateTempDirectory("ShortcutUnc"); + + try + { + var shortcut = Path.Join(directory, "remote.lnk"); + CreateShortcut(shortcut, @"\\nonexistent-host\share\actual.log"); + + Assert.That(ShortcutResolver.Resolve(shortcut), Is.EqualTo(shortcut), + "a UNC target must not be probed: File.Exists would authenticate against the remote host"); + } + finally + { + Directory.Delete(directory, true); + } + } + + [Test] + public void Resolve_OversizedShortcutFile_ReturnsPathUnchanged () + { + var directory = CreateTempDirectory("ShortcutOversized"); + + try + { + var shortcut = Path.Join(directory, "oversized.lnk"); + File.WriteAllBytes(shortcut, new byte[1024 * 1024 + 1]); + + Assert.That(ShortcutResolver.Resolve(shortcut), Is.EqualTo(shortcut)); + } + finally + { + Directory.Delete(directory, true); + } + } + + private static string CreateTempDirectory (string name) + { + var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", name, Guid.NewGuid().ToString()); + _ = Directory.CreateDirectory(directory); + return directory; + } + + /// + /// Creates a real shortcut through the shell's WScript.Shell automation object, so the tests + /// exercise the same files Explorer writes. + /// + private static void CreateShortcut (string shortcutPath, string targetPath) + { + var shellType = Type.GetTypeFromProgID("WScript.Shell"); + Assert.That(shellType, Is.Not.Null, "WScript.Shell should be available on Windows test machines"); + + dynamic shell = Activator.CreateInstance(shellType)!; + dynamic shortcut = shell.CreateShortcut(shortcutPath); + shortcut.TargetPath = targetPath; + shortcut.Save(); + } +} diff --git a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs index 0c72551c..93cc3558 100644 --- a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs +++ b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs @@ -68,6 +68,15 @@ internal sealed class FileOperationService ( public LogWindow AddFileTab (FileTabRequest request) { + // A shortcut resolves to its target here, so everything downstream — settings file + // indirection, .lxp handling, title, history — works on the target file. The request is + // only replaced when the name actually changed, keeping the caller's instance otherwise. + var resolvedFile = ShortcutResolver.Resolve(request.FileName); + if (!string.Equals(resolvedFile, request.FileName, StringComparison.Ordinal)) + { + request = request with { FileName = resolvedFile }; + } + var logFileName = PersisterHelpers.FindFilenameForSettings(request.FileName, _pluginRegistry); var normalizedPath = Path.GetFullPath(logFileName); @@ -204,6 +213,9 @@ public LogWindow AddTempFileTab (string fileName, string title) return null; } + // Resolve shortcuts first: a .lnk to a session file must not end up as a raw log line. + fileNames = fileNames.Select(ShortcutResolver.Resolve).ToArray(); + // Session files (.lxj) aren't logs — dispatch them to the project loader and // continue with only the log files. Without this, the .lxj would be treated as // a raw log line in the multi-file tab. @@ -270,13 +282,16 @@ public MultiFileDecision LoadFilesWithOption (string[] fileNames, bool invertLog if (fileNames.Length == 1) { - if (fileNames[0].EndsWith(".lxj", StringComparison.OrdinalIgnoreCase)) + // Resolve first: a shortcut to a session file must not be opened as a raw log. + var fileName = ShortcutResolver.Resolve(fileNames[0]); + + if (fileName.EndsWith(".lxj", StringComparison.OrdinalIgnoreCase)) { - _loadSessionCallback(fileNames[0], true); + _loadSessionCallback(fileName, true); return MultiFileDecision.Cancel; // Already handled } - _ = AddFileTab(new FileTabRequest { FileName = fileNames[0] }); + _ = AddFileTab(new FileTabRequest { FileName = fileName }); return MultiFileDecision.SingleFiles; } @@ -310,13 +325,16 @@ public void AddFileTabs (string[] fileNames, int? targetLine = null) { try { - if (fileName.EndsWith(".lxj", StringComparison.OrdinalIgnoreCase)) + // Resolve first: a shortcut to a session file must not be opened as a raw log. + var resolved = ShortcutResolver.Resolve(fileName); + + if (resolved.EndsWith(".lxj", StringComparison.OrdinalIgnoreCase)) { - _loadSessionCallback(fileName, false); + _loadSessionCallback(resolved, false); } else { - _ = AddFileTab(new FileTabRequest { FileName = fileName, TargetLine = targetLine }); + _ = AddFileTab(new FileTabRequest { FileName = resolved, TargetLine = targetLine }); } } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or LogFileException) diff --git a/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs b/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs new file mode 100644 index 00000000..a5946a6c --- /dev/null +++ b/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs @@ -0,0 +1,161 @@ +using System.Runtime.InteropServices; +using System.Runtime.Versioning; + +using NLog; + +namespace LogExpert.UI.Services.FileOperationService; + +/// +/// Resolves a Windows shortcut (.lnk) to the local file it points at. +/// +/// +/// +/// Resolution goes through the shell's own IShellLink implementation instead of parsing the +/// MS-SHLLINK format by hand. This class lives in the UI assembly, which is +/// net10.0-windows, so the COM dependency is safe here. +/// +/// +/// Anything that is not a .lnk, does not exist, is implausibly large, cannot be resolved, or +/// points off the local machine is returned unchanged: such a shortcut opens as a regular file, +/// exactly as before. +/// +/// +[SupportedOSPlatform("windows")] +internal static class ShortcutResolver +{ + #region Constants + + private const string SHORTCUT_EXTENSION = ".lnk"; + + /// Buffer size for IShellLinkW::GetPath, following the shell's own MAX_PATH. + private const int TARGET_PATH_CAPACITY = 260; + + /// + /// Real shortcuts are a few KB. Anything larger is not one, and handing it to the shell parser + /// whole would let a dropped file spike memory. + /// + private const int MAX_SHORTCUT_SIZE = 1024 * 1024; + + /// SLGP_RAWPATH: retrieve the stored target path without resolving it against the current shell state. + private const uint SLGP_RAWPATH = 0x00000004; + + /// STGM_READ: open the shortcut file read-only. + private const uint STGM_READ = 0x00000000; + + #endregion + + #region Fields + + private static readonly Logger _logger = LogManager.GetCurrentClassLogger(); + + #endregion + + #region Public methods + + /// + /// Resolves to the file a shortcut points at. + /// + /// A path that may or may not be a shortcut. + /// + /// The target path when it exists; otherwise unchanged. + /// + public static string Resolve (string fileName) + { + if (string.IsNullOrWhiteSpace(fileName) + || !Path.GetExtension(fileName).Equals(SHORTCUT_EXTENSION, StringComparison.OrdinalIgnoreCase) + || !File.Exists(fileName)) + { + return fileName; + } + + try + { + if (new FileInfo(fileName).Length > MAX_SHORTCUT_SIZE) + { + _logger.Warn("Ignoring {Shortcut}: it is above the {Limit} byte shortcut limit", fileName, MAX_SHORTCUT_SIZE); + return fileName; + } + + IShellLink shellLink = (IShellLink)new ShellLinkClass(); + ((IPersistFile)shellLink).Load(fileName, STGM_READ); + + var buffer = new System.Text.StringBuilder(TARGET_PATH_CAPACITY); + shellLink.GetPath(buffer, buffer.Capacity, IntPtr.Zero, SLGP_RAWPATH); + + var target = buffer.ToString(); + if (target.Length == 0) + { + _logger.Debug("Shortcut {Shortcut} has no target path, keeping it", fileName); + return fileName; + } + + // A UNC target would make File.Exists authenticate against the remote host, handing it + // the user's NetNTLM hash, and device paths (\\.\, \\?\) can block. Such a shortcut + // opens as a regular file instead. + if (target.StartsWith(@"\\", StringComparison.Ordinal)) + { + _logger.Debug("Shortcut target {Target} is not a local path, keeping {Shortcut}", target, fileName); + return fileName; + } + + if (!File.Exists(target)) + { + _logger.Debug("Shortcut target {Target} does not exist, keeping {Shortcut}", target, fileName); + return fileName; + } + + return target; + } + catch (Exception ex) when (ex is COMException + or ExternalException + or IOException + or UnauthorizedAccessException + or ArgumentException + or NotSupportedException) + { + _logger.Debug(ex, "Could not resolve shortcut {Shortcut}, keeping it", fileName); + return fileName; + } + } + + #endregion + + #region COM declarations + + /// The shell link CoClass (CLSID_ShellLink). + [ComImport] + [Guid("00021401-0000-0000-C000-000000000046")] + private class ShellLinkClass + { + } + + /// + /// IShellLinkW, declared with only its first method (GetPath). The interface's + /// remaining methods are never called, so they are left out. + /// + [ComImport] + [Guid("000214F9-0000-0000-C000-000000000046")] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + private interface IShellLink + { + void GetPath ([MarshalAs(UnmanagedType.LPWStr)] System.Text.StringBuilder pszFile, int cchMaxPath, IntPtr pFindData, uint fFlags); + } + + /// IPersistFile, used to load the shortcut file into the shell link object. + [ComImport] + [Guid("0000010B-0000-0000-C000-000000000046")] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + private interface IPersistFile + { + // From IPersist, which IPersistFile extends. + void GetClassID (out Guid pClassID); + + // IsDirty precedes Load in the vtable and has to be declared to keep the slot order. + [PreserveSig] + int IsDirty (); + + void Load ([MarshalAs(UnmanagedType.LPWStr)] string pszFileName, uint dwMode); + } + + #endregion +} From ea20c4f4a0d7ba7896af55f0a064811f93c21056 Mon Sep 17 00:00:00 2001 From: mfkeeq <546357529@qq.com> Date: Fri, 23 Oct 2026 18:36:09 +0800 Subject: [PATCH 2/4] Harden shortcut resolution and switch interop to Vanara Security: normalize the raw .lnk target (env-var expansion, resolve relative to the shortcut directory, Path.GetFullPath) before any existence probe, and only follow plain local drive targets (X:\). Disguised UNC/device forms (//host/share, \?\UNC\, \.\C:\) are now rejected, closing the NetNTLM leak via File.Exists. The guard is a pure internal SanitizeLocalTarget, unit-tested directly for the forward-slash and \?\ / \. \ forms the shell parser cannot be fed. Interop: replace hand-rolled [ComImport] declarations with Vanara's CShellLinkW/IShellLinkW, matching the rest of the codebase. Path handling: expand environment variables, resolve relative targets against the .lnk's directory, widen the GetPath buffer to 1024. Behaviour: sort after resolving so multi-file order follows target names; resolution is one hop; the repeated resolve-then-.lxj pattern in FileOperationService is consolidated into a ResolveLogFile helper. Tests: shared TestShortcuts helper with proper COM release; oversized test uses the internal MAX_SHORTCUT_SIZE; new guard tests lock in the UNC/device-path rejection. --- .../Services/FileOperationServiceTests.cs | 19 +-- .../Services/ShortcutResolverTests.cs | 70 +++++++---- .../Services/TestShortcuts.cs | 39 ++++++ .../FileOperationService.cs | 50 +++++--- .../FileOperationService/ShortcutResolver.cs | 111 +++++++++--------- 5 files changed, 179 insertions(+), 110 deletions(-) create mode 100644 src/LogExpert.UI.Tests/Services/TestShortcuts.cs diff --git a/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs b/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs index 329bba63..46b67a32 100644 --- a/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs +++ b/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs @@ -709,7 +709,7 @@ public void AddFileTab_ShortcutToLogFile_OpensTarget () File.WriteAllText(log, "line1"); var shortcut = Path.Join(directory, "actual.lnk"); - CreateShortcut(shortcut, log); + TestShortcuts.Create(shortcut, log); _sut.AddFileTab(new FileTabRequest { FileName = shortcut }); @@ -1303,26 +1303,11 @@ private static (string Directory, string Shortcut, string Session) CreateShortcu File.WriteAllText(session, ""); var shortcut = Path.Join(directory, "session.lnk"); - CreateShortcut(shortcut, session); + TestShortcuts.Create(shortcut, session); return (directory, shortcut, session); } - /// - /// Creates a real shortcut through the shell's WScript.Shell automation object, so the tests - /// exercise the same files Explorer writes. - /// - private static void CreateShortcut (string shortcutPath, string targetPath) - { - var shellType = Type.GetTypeFromProgID("WScript.Shell"); - Assert.That(shellType, Is.Not.Null, "WScript.Shell should be available on Windows test machines"); - - dynamic shell = Activator.CreateInstance(shellType)!; - dynamic shortcut = shell.CreateShortcut(shortcutPath); - shortcut.TargetPath = targetPath; - shortcut.Save(); - } - /// /// Builds a second service on the shared config manager, standing in for another LogTabWindow /// of the same session with its own tab controller. diff --git a/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs b/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs index 7e2071e9..9a2d5aa5 100644 --- a/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs +++ b/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs @@ -7,8 +7,12 @@ namespace LogExpert.UI.Tests.Services; /// -/// Tests the guard rails of : oversized files and targets off the -/// local machine are not followed, the shortcut keeps opening as a regular file instead. +/// Tests the guard rails of : end to end via +/// for whole-file behaviour (real shortcuts, oversized +/// files, plain UNC targets), and directly on +/// for disguised target forms — the shell parser cannot be fed strings like +/// //host/share through any API a test could use, but the guard must reject them, so it is +/// written as a pure function and tested as one. /// [TestFixture] [SupportedOSPlatform("windows")] @@ -25,7 +29,7 @@ public void Resolve_ShortcutToLocalTarget_ReturnsTarget () File.WriteAllText(log, "line1"); var shortcut = Path.Join(directory, "actual.lnk"); - CreateShortcut(shortcut, log); + TestShortcuts.Create(shortcut, log); Assert.That(ShortcutResolver.Resolve(shortcut), Is.EqualTo(log)); } @@ -43,7 +47,7 @@ public void Resolve_ShortcutToUncTarget_ReturnsShortcutUnchanged () try { var shortcut = Path.Join(directory, "remote.lnk"); - CreateShortcut(shortcut, @"\\nonexistent-host\share\actual.log"); + TestShortcuts.Create(shortcut, @"\\nonexistent-host\share\actual.log"); Assert.That(ShortcutResolver.Resolve(shortcut), Is.EqualTo(shortcut), "a UNC target must not be probed: File.Exists would authenticate against the remote host"); @@ -62,7 +66,7 @@ public void Resolve_OversizedShortcutFile_ReturnsPathUnchanged () try { var shortcut = Path.Join(directory, "oversized.lnk"); - File.WriteAllBytes(shortcut, new byte[1024 * 1024 + 1]); + File.WriteAllBytes(shortcut, new byte[ShortcutResolver.MAX_SHORTCUT_SIZE + 1]); Assert.That(ShortcutResolver.Resolve(shortcut), Is.EqualTo(shortcut)); } @@ -72,25 +76,51 @@ public void Resolve_OversizedShortcutFile_ReturnsPathUnchanged () } } - private static string CreateTempDirectory (string name) + [TestCase("//nonexistent-host/share/actual.log")] + [TestCase(@"\\?\UNC\nonexistent-host\share\actual.log")] + [TestCase(@"\\?\C:\Windows\system32\drivers\etc\hosts")] + [TestCase(@"\\.\C:\nonexistent\actual.log")] + [TestCase(@"\??\UNC\nonexistent-host\share\actual.log")] + public void Sanitize_DisguisedNonLocalTarget_IsRejected (string target) { - var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", name, Guid.NewGuid().ToString()); - _ = Directory.CreateDirectory(directory); - return directory; + Assert.That(ShortcutResolver.SanitizeLocalTarget(target, @"C:\logs"), Is.Null, + $"'{target}' must not be followed: probing it could reach the network"); } - /// - /// Creates a real shortcut through the shell's WScript.Shell automation object, so the tests - /// exercise the same files Explorer writes. - /// - private static void CreateShortcut (string shortcutPath, string targetPath) + [Test] + public void Sanitize_PlainLocalTarget_ReturnsFullPath () + { + Assert.That(ShortcutResolver.SanitizeLocalTarget(@"C:\logs\actual.log", @"D:\other"), + Is.EqualTo(@"C:\logs\actual.log")); + } + + [Test] + public void Sanitize_RelativeTarget_ResolvesAgainstShortcutDirectory () + { + Assert.That(ShortcutResolver.SanitizeLocalTarget(@"sub\actual.log", @"C:\logs"), + Is.EqualTo(@"C:\logs\sub\actual.log")); + } + + [Test] + public void Sanitize_EnvironmentVariableTarget_IsExpanded () { - var shellType = Type.GetTypeFromProgID("WScript.Shell"); - Assert.That(shellType, Is.Not.Null, "WScript.Shell should be available on Windows test machines"); + Environment.SetEnvironmentVariable("LOGEXPERT_TEST_VAR", @"C:\logs", EnvironmentVariableTarget.Process); - dynamic shell = Activator.CreateInstance(shellType)!; - dynamic shortcut = shell.CreateShortcut(shortcutPath); - shortcut.TargetPath = targetPath; - shortcut.Save(); + try + { + Assert.That(ShortcutResolver.SanitizeLocalTarget(@"%LOGEXPERT_TEST_VAR%\actual.log", @"D:\other"), + Is.EqualTo(@"C:\logs\actual.log")); + } + finally + { + Environment.SetEnvironmentVariable("LOGEXPERT_TEST_VAR", null, EnvironmentVariableTarget.Process); + } + } + + private static string CreateTempDirectory (string name) + { + var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", name, Guid.NewGuid().ToString()); + _ = Directory.CreateDirectory(directory); + return directory; } } diff --git a/src/LogExpert.UI.Tests/Services/TestShortcuts.cs b/src/LogExpert.UI.Tests/Services/TestShortcuts.cs new file mode 100644 index 00000000..c29edf27 --- /dev/null +++ b/src/LogExpert.UI.Tests/Services/TestShortcuts.cs @@ -0,0 +1,39 @@ +using System.Runtime.InteropServices; +using System.Runtime.Versioning; + +using NUnit.Framework; + +namespace LogExpert.UI.Tests.Services; + +/// +/// Creates real shortcuts through the shell's WScript.Shell automation object, so the tests +/// exercise the same files Explorer writes. The COM objects are released right after saving. +/// +[SupportedOSPlatform("windows")] +internal static class TestShortcuts +{ + public static void Create (string shortcutPath, string targetPath) + { + var shellType = Type.GetTypeFromProgID("WScript.Shell"); + Assert.That(shellType, Is.Not.Null, "WScript.Shell should be available on Windows test machines"); + + dynamic shell = Activator.CreateInstance(shellType)!; + object? shortcut = null; + try + { + dynamic link = shell.CreateShortcut(shortcutPath); + shortcut = link; + link.TargetPath = targetPath; + link.Save(); + } + finally + { + if (shortcut != null) + { + Marshal.FinalReleaseComObject(shortcut); + } + + Marshal.FinalReleaseComObject(shell); + } + } +} diff --git a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs index 93cc3558..77fa16d7 100644 --- a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs +++ b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs @@ -68,9 +68,6 @@ internal sealed class FileOperationService ( public LogWindow AddFileTab (FileTabRequest request) { - // A shortcut resolves to its target here, so everything downstream — settings file - // indirection, .lxp handling, title, history — works on the target file. The request is - // only replaced when the name actually changed, keeping the caller's instance otherwise. var resolvedFile = ShortcutResolver.Resolve(request.FileName); if (!string.Equals(resolvedFile, request.FileName, StringComparison.Ordinal)) { @@ -213,7 +210,6 @@ public LogWindow AddTempFileTab (string fileName, string title) return null; } - // Resolve shortcuts first: a .lnk to a session file must not end up as a raw log line. fileNames = fileNames.Select(ShortcutResolver.Resolve).ToArray(); // Session files (.lxj) aren't logs — dispatch them to the project loader and @@ -278,20 +274,23 @@ public void LoadDroppedFiles (string[] selectedFiles, bool invertLogic, Func + /// Resolves a possible shortcut and dispatches a resolved session file (.lxj) to the + /// session loader. Returns the name to open as a log, or when the file + /// was a session and has been handled. + /// + private string? ResolveLogFile (string fileName, bool asFirstFile) + { + var resolved = ShortcutResolver.Resolve(fileName); + if (resolved.EndsWith(".lxj", StringComparison.OrdinalIgnoreCase)) + { + _loadSessionCallback(resolved, asFirstFile); + return null; + } + + return resolved; + } + public LogWindow? PasteFromClipboard () { var text = _clipboardTextProvider(); diff --git a/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs b/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs index a5946a6c..872551d8 100644 --- a/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs +++ b/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs @@ -1,8 +1,14 @@ using System.Runtime.InteropServices; +using System.Runtime.InteropServices.ComTypes; using System.Runtime.Versioning; +using System.Text; using NLog; +using IShellLinkW = Vanara.PInvoke.Shell32.IShellLinkW; +using CShellLinkW = Vanara.PInvoke.Shell32.CShellLinkW; +using SLGP = Vanara.PInvoke.Shell32.SLGP; + namespace LogExpert.UI.Services.FileOperationService; /// @@ -10,14 +16,15 @@ namespace LogExpert.UI.Services.FileOperationService; /// /// /// -/// Resolution goes through the shell's own IShellLink implementation instead of parsing the -/// MS-SHLLINK format by hand. This class lives in the UI assembly, which is -/// net10.0-windows, so the COM dependency is safe here. +/// Resolution goes through the shell's own IShellLink implementation (Vanara's +/// ) instead of parsing the MS-SHLLINK format by hand. This class lives in +/// the UI assembly, which is net10.0-windows, so the COM dependency is safe here. /// /// /// Anything that is not a .lnk, does not exist, is implausibly large, cannot be resolved, or /// points off the local machine is returned unchanged: such a shortcut opens as a regular file, -/// exactly as before. +/// exactly as before. Resolution is one hop: a target that is itself a shortcut is returned as-is +/// and resolved by the next caller. /// /// [SupportedOSPlatform("windows")] @@ -27,20 +34,19 @@ internal static class ShortcutResolver private const string SHORTCUT_EXTENSION = ".lnk"; - /// Buffer size for IShellLinkW::GetPath, following the shell's own MAX_PATH. - private const int TARGET_PATH_CAPACITY = 260; + /// + /// Buffer size for IShellLinkW::GetPath, above MAX_PATH so long-path targets are not truncated. + /// + private const int TARGET_PATH_CAPACITY = 1024; /// /// Real shortcuts are a few KB. Anything larger is not one, and handing it to the shell parser /// whole would let a dropped file spike memory. /// - private const int MAX_SHORTCUT_SIZE = 1024 * 1024; - - /// SLGP_RAWPATH: retrieve the stored target path without resolving it against the current shell state. - private const uint SLGP_RAWPATH = 0x00000004; + internal const int MAX_SHORTCUT_SIZE = 1024 * 1024; /// STGM_READ: open the shortcut file read-only. - private const uint STGM_READ = 0x00000000; + private const int STGM_READ = 0; #endregion @@ -53,7 +59,7 @@ internal static class ShortcutResolver #region Public methods /// - /// Resolves to the file a shortcut points at. + /// Resolves to the file a shortcut points at (one hop). /// /// A path that may or may not be a shortcut. /// @@ -76,11 +82,11 @@ public static string Resolve (string fileName) return fileName; } - IShellLink shellLink = (IShellLink)new ShellLinkClass(); + IShellLinkW shellLink = (IShellLinkW)new CShellLinkW(); ((IPersistFile)shellLink).Load(fileName, STGM_READ); - var buffer = new System.Text.StringBuilder(TARGET_PATH_CAPACITY); - shellLink.GetPath(buffer, buffer.Capacity, IntPtr.Zero, SLGP_RAWPATH); + var buffer = new StringBuilder(TARGET_PATH_CAPACITY); + shellLink.GetPath(buffer, buffer.Capacity, out _, SLGP.SLGP_RAWPATH); var target = buffer.ToString(); if (target.Length == 0) @@ -89,25 +95,29 @@ public static string Resolve (string fileName) return fileName; } - // A UNC target would make File.Exists authenticate against the remote host, handing it - // the user's NetNTLM hash, and device paths (\\.\, \\?\) can block. Such a shortcut - // opens as a regular file instead. - if (target.StartsWith(@"\\", StringComparison.Ordinal)) + // A relative target sits next to the shortcut, not in the current working directory. + var shortcutDirectory = Path.GetDirectoryName(Path.GetFullPath(fileName))!; + + // The target string comes straight from the .lnk bytes, so it can be a UNC or device + // path in disguise (//host/share, \\?\UNC\...). Only a plain local drive path is + // followed, because probing anything else would authenticate against a remote host, + // handing it the user's NetNTLM hash. + var fullTarget = SanitizeLocalTarget(target, shortcutDirectory); + if (fullTarget is null) { - _logger.Debug("Shortcut target {Target} is not a local path, keeping {Shortcut}", target, fileName); + _logger.Debug("Shortcut target {Target} is not a local drive path, keeping {Shortcut}", target, fileName); return fileName; } - if (!File.Exists(target)) + if (!File.Exists(fullTarget)) { _logger.Debug("Shortcut target {Target} does not exist, keeping {Shortcut}", target, fileName); return fileName; } - return target; + return fullTarget; } - catch (Exception ex) when (ex is COMException - or ExternalException + catch (Exception ex) when (ex is ExternalException or IOException or UnauthorizedAccessException or ArgumentException @@ -120,41 +130,34 @@ or ArgumentException #endregion - #region COM declarations - - /// The shell link CoClass (CLSID_ShellLink). - [ComImport] - [Guid("00021401-0000-0000-C000-000000000046")] - private class ShellLinkClass - { - } + #region Internal methods /// - /// IShellLinkW, declared with only its first method (GetPath). The interface's - /// remaining methods are never called, so they are left out. + /// Turns a raw shortcut target into the full path to follow — but only when it lands on a + /// plain local drive (X:\). Environment variables are expanded and relative targets are + /// resolved against the shortcut's directory; anything else (UNC, device paths like + /// \\?\ or \\.\, malformed strings) yields , because + /// probing such a path would authenticate against a remote host and leak the user's NetNTLM + /// hash. Pure string handling: no file system access. /// - [ComImport] - [Guid("000214F9-0000-0000-C000-000000000046")] - [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] - private interface IShellLink + /// The raw target string from the .lnk. + /// Directory the shortcut lives in; the base for relative targets. + internal static string? SanitizeLocalTarget (string target, string shortcutDirectory) { - void GetPath ([MarshalAs(UnmanagedType.LPWStr)] System.Text.StringBuilder pszFile, int cchMaxPath, IntPtr pFindData, uint fFlags); - } - - /// IPersistFile, used to load the shortcut file into the shell link object. - [ComImport] - [Guid("0000010B-0000-0000-C000-000000000046")] - [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] - private interface IPersistFile - { - // From IPersist, which IPersistFile extends. - void GetClassID (out Guid pClassID); - - // IsDirty precedes Load in the vtable and has to be declared to keep the slot order. - [PreserveSig] - int IsDirty (); + try + { + var expanded = Environment.ExpandEnvironmentVariables(target); + var fullTarget = Path.GetFullPath(expanded, shortcutDirectory); - void Load ([MarshalAs(UnmanagedType.LPWStr)] string pszFileName, uint dwMode); + var root = Path.GetPathRoot(fullTarget); + return root is not null && root.Length == 3 && root[1] == ':' ? fullTarget : null; + } + catch (Exception ex) when (ex is ArgumentException + or IOException + or NotSupportedException) + { + return null; + } } #endregion From a4b40e276ba4cd51ec134cf7f17336d11957459b Mon Sep 17 00:00:00 2001 From: mfkeeq <546357529@qq.com> Date: Sat, 24 Oct 2026 12:38:05 +0800 Subject: [PATCH 3/4] Follow shortcut chains to the final target with a 3-hop cap Shortcut (.lnk) handling now follows a chain of shortcuts to its final existing local file, bounded by MAX_SHORTCUT_HOPS = 3, and the same resolution is applied at every entry point (AddFileTab, AddMultiFileTab, LoadFilesWithOption, ResolveLogFile). This corrects the earlier PR description: - resolution is no longer described as a single hop: a valid chain is followed to its end; a broken, non-local or over-limit chain keeps the last resolvable path (the original shortcut when the hop cap is hit) so it still opens as before - an oversized .lnk (above 1 MiB) is not resolved, but it is not skipped either; it is opened as the file the user selected Review follow-ups: - release the shell link COM object in a finally block - remove a redundant string.Equals - document that mapped network drives are intentionally trusted - resolve before sorting in LoadFilesWithOption so multi-file order follows the target names - trim duplicated comments - rename Sanitize_* tests to SanitizeLocalTarget_* Chain, limit and cycle behaviour is covered through an injected one-hop resolver, because the Windows shell dereferences .lnk targets when saving. All 389 LogExpert.UI tests pass. --- .../Services/FileOperationServiceTests.cs | 2 +- .../Services/ShortcutResolverTests.cs | 76 ++++++-- .../Services/TestShortcuts.cs | 4 - .../FileOperationService.cs | 6 +- .../FileOperationService/ShortcutResolver.cs | 168 +++++++++++------- 5 files changed, 165 insertions(+), 91 deletions(-) diff --git a/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs b/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs index 46b67a32..899386ae 100644 --- a/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs +++ b/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs @@ -205,7 +205,7 @@ public void AddFileTab_RaisesFileOpenedEvent () // Assert Assert.That(receivedArgs, Is.Not.Null); Assert.That(receivedArgs!.LogWindow, Is.SameAs(_stubLogWindow)); - Assert.That(receivedArgs.Request, Is.SameAs(request)); + Assert.That(receivedArgs.Request, Is.EqualTo(request)); Assert.That(receivedArgs.ResolvedFileName, Is.Not.Null.And.Not.Empty); Assert.That(receivedArgs.EncodingOptions, Is.Not.Null); } diff --git a/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs b/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs index 9a2d5aa5..c46f87cf 100644 --- a/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs +++ b/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs @@ -6,14 +6,6 @@ namespace LogExpert.UI.Tests.Services; -/// -/// Tests the guard rails of : end to end via -/// for whole-file behaviour (real shortcuts, oversized -/// files, plain UNC targets), and directly on -/// for disguised target forms — the shell parser cannot be fed strings like -/// //host/share through any API a test could use, but the guard must reject them, so it is -/// written as a pure function and tested as one. -/// [TestFixture] [SupportedOSPlatform("windows")] internal class ShortcutResolverTests @@ -76,33 +68,79 @@ public void Resolve_OversizedShortcutFile_ReturnsPathUnchanged () } } + // The shell resolves .lnk targets when saving, so a real chain cannot be produced with + // WScript.Shell; the follower is exercised with injected hops instead. + [Test] + public void FollowChain_ShortcutToShortcut_ReturnsFinalTarget () + { + var targets = new Dictionary + { + [@"C:\logs\first.lnk"] = @"C:\logs\second.lnk", + [@"C:\logs\second.lnk"] = @"C:\logs\actual.log" + }; + + Assert.That(ShortcutResolver.FollowChain(@"C:\logs\first.lnk", path => targets.GetValueOrDefault(path)), + Is.EqualTo(@"C:\logs\actual.log")); + } + + [Test] + public void FollowChain_ChainAtLimit_ReturnsFinalTarget () + { + var targets = CreateChain(ShortcutResolver.MAX_SHORTCUT_HOPS); + + Assert.That(ShortcutResolver.FollowChain(@"C:\logs\link1.lnk", path => targets.GetValueOrDefault(path)), + Is.EqualTo(@"C:\logs\actual.log")); + } + + [Test] + public void FollowChain_ChainBeyondLimit_StopsAfterMaxHops () + { + var targets = CreateChain(ShortcutResolver.MAX_SHORTCUT_HOPS + 1); + + Assert.That(ShortcutResolver.FollowChain(@"C:\logs\link1.lnk", path => targets.GetValueOrDefault(path)), + Is.EqualTo(@"C:\logs\link1.lnk")); + } + + [Test] + public void FollowChain_CyclicChain_StopsAfterMaxHops () + { + var targets = new Dictionary + { + [@"C:\logs\first.lnk"] = @"C:\logs\second.lnk", + [@"C:\logs\second.lnk"] = @"C:\logs\first.lnk" + }; + + Assert.That(ShortcutResolver.FollowChain(@"C:\logs\first.lnk", path => targets.GetValueOrDefault(path)), + Is.EqualTo(@"C:\logs\first.lnk")); + } + [TestCase("//nonexistent-host/share/actual.log")] [TestCase(@"\\?\UNC\nonexistent-host\share\actual.log")] [TestCase(@"\\?\C:\Windows\system32\drivers\etc\hosts")] [TestCase(@"\\.\C:\nonexistent\actual.log")] [TestCase(@"\??\UNC\nonexistent-host\share\actual.log")] - public void Sanitize_DisguisedNonLocalTarget_IsRejected (string target) + public void SanitizeLocalTarget_DisguisedNonLocalTarget_IsRejected (string target) { Assert.That(ShortcutResolver.SanitizeLocalTarget(target, @"C:\logs"), Is.Null, $"'{target}' must not be followed: probing it could reach the network"); } [Test] - public void Sanitize_PlainLocalTarget_ReturnsFullPath () + public void SanitizeLocalTarget_PlainLocalTarget_ReturnsFullPath () { Assert.That(ShortcutResolver.SanitizeLocalTarget(@"C:\logs\actual.log", @"D:\other"), Is.EqualTo(@"C:\logs\actual.log")); } [Test] - public void Sanitize_RelativeTarget_ResolvesAgainstShortcutDirectory () + public void SanitizeLocalTarget_RelativeTarget_ResolvesAgainstShortcutDirectory () { Assert.That(ShortcutResolver.SanitizeLocalTarget(@"sub\actual.log", @"C:\logs"), Is.EqualTo(@"C:\logs\sub\actual.log")); } [Test] - public void Sanitize_EnvironmentVariableTarget_IsExpanded () + public void SanitizeLocalTarget_EnvironmentVariableTarget_IsExpanded () { Environment.SetEnvironmentVariable("LOGEXPERT_TEST_VAR", @"C:\logs", EnvironmentVariableTarget.Process); @@ -117,6 +155,20 @@ public void Sanitize_EnvironmentVariableTarget_IsExpanded () } } + private static Dictionary CreateChain (int linkCount) + { + var targets = new Dictionary(); + + for (var i = 1; i <= linkCount; i++) + { + targets[$@"C:\logs\link{i}.lnk"] = i == linkCount + ? @"C:\logs\actual.log" + : $@"C:\logs\link{i + 1}.lnk"; + } + + return targets; + } + private static string CreateTempDirectory (string name) { var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", name, Guid.NewGuid().ToString()); diff --git a/src/LogExpert.UI.Tests/Services/TestShortcuts.cs b/src/LogExpert.UI.Tests/Services/TestShortcuts.cs index c29edf27..c919d9b1 100644 --- a/src/LogExpert.UI.Tests/Services/TestShortcuts.cs +++ b/src/LogExpert.UI.Tests/Services/TestShortcuts.cs @@ -5,10 +5,6 @@ namespace LogExpert.UI.Tests.Services; -/// -/// Creates real shortcuts through the shell's WScript.Shell automation object, so the tests -/// exercise the same files Explorer writes. The COM objects are released right after saving. -/// [SupportedOSPlatform("windows")] internal static class TestShortcuts { diff --git a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs index 77fa16d7..4f4dbfc2 100644 --- a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs +++ b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs @@ -68,11 +68,7 @@ internal sealed class FileOperationService ( public LogWindow AddFileTab (FileTabRequest request) { - var resolvedFile = ShortcutResolver.Resolve(request.FileName); - if (!string.Equals(resolvedFile, request.FileName, StringComparison.Ordinal)) - { - request = request with { FileName = resolvedFile }; - } + request = request with { FileName = ShortcutResolver.Resolve(request.FileName) }; var logFileName = PersisterHelpers.FindFilenameForSettings(request.FileName, _pluginRegistry); var normalizedPath = Path.GetFullPath(logFileName); diff --git a/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs b/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs index 872551d8..e4e020fc 100644 --- a/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs +++ b/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs @@ -12,21 +12,8 @@ namespace LogExpert.UI.Services.FileOperationService; /// -/// Resolves a Windows shortcut (.lnk) to the local file it points at. +/// Resolves a Windows shortcut (.lnk) to the local file at the end of its chain. /// -/// -/// -/// Resolution goes through the shell's own IShellLink implementation (Vanara's -/// ) instead of parsing the MS-SHLLINK format by hand. This class lives in -/// the UI assembly, which is net10.0-windows, so the COM dependency is safe here. -/// -/// -/// Anything that is not a .lnk, does not exist, is implausibly large, cannot be resolved, or -/// points off the local machine is returned unchanged: such a shortcut opens as a regular file, -/// exactly as before. Resolution is one hop: a target that is itself a shortcut is returned as-is -/// and resolved by the next caller. -/// -/// [SupportedOSPlatform("windows")] internal static class ShortcutResolver { @@ -34,18 +21,13 @@ internal static class ShortcutResolver private const string SHORTCUT_EXTENSION = ".lnk"; - /// - /// Buffer size for IShellLinkW::GetPath, above MAX_PATH so long-path targets are not truncated. - /// private const int TARGET_PATH_CAPACITY = 1024; - /// - /// Real shortcuts are a few KB. Anything larger is not one, and handing it to the shell parser - /// whole would let a dropped file spike memory. - /// internal const int MAX_SHORTCUT_SIZE = 1024 * 1024; - /// STGM_READ: open the shortcut file read-only. + // A chain is followed to its end, but a broken or cyclic chain must not load COM objects forever. + internal const int MAX_SHORTCUT_HOPS = 3; + private const int STGM_READ = 0; #endregion @@ -59,30 +41,105 @@ internal static class ShortcutResolver #region Public methods /// - /// Resolves to the file a shortcut points at (one hop). + /// Follows through at most shortcuts. /// - /// A path that may or may not be a shortcut. /// - /// The target path when it exists; otherwise unchanged. + /// The final existing target; otherwise the last path that could not be followed (the input + /// itself when the hop limit is reached). /// public static string Resolve (string fileName) { - if (string.IsNullOrWhiteSpace(fileName) - || !Path.GetExtension(fileName).Equals(SHORTCUT_EXTENSION, StringComparison.OrdinalIgnoreCase) - || !File.Exists(fileName)) + return FollowChain(fileName, ResolveOneHop); + } + + #endregion + + #region Internal methods + + internal static string FollowChain (string fileName, Func resolveOneHop) + { + var current = fileName; + + for (var hop = 0; ; hop++) { - return fileName; + if (!IsShortcut(current)) + { + return current; + } + + if (hop >= MAX_SHORTCUT_HOPS) + { + // Keep the input, not the partially followed path: another call site resolving + // the result again would otherwise continue past the limit. + _logger.Warn("Shortcut chain starting at {Shortcut} exceeds {Limit} hops; keeping it", fileName, MAX_SHORTCUT_HOPS); + return fileName; + } + + var target = resolveOneHop(current); + if (target is null) + { + return current; + } + + current = target; + } + } + + /// + /// Normalizes a raw shortcut target and returns it only when it is a plain local drive path + /// (X:\); anything that could reach the network yields . + /// + internal static string? SanitizeLocalTarget (string target, string shortcutDirectory) + { + try + { + var expanded = Environment.ExpandEnvironmentVariables(target); + var fullTarget = Path.GetFullPath(expanded, shortcutDirectory); + + // Mapped drives are allowed on purpose: the user created the mapping, so it is trusted. + var root = Path.GetPathRoot(fullTarget); + return root is not null && root.Length == 3 && root[1] == ':' ? fullTarget : null; + } + catch (Exception ex) when (ex is ArgumentException + or IOException + or NotSupportedException) + { + return null; } + } + + #endregion + + #region Private methods + + private static bool IsShortcut (string fileName) + { + return !string.IsNullOrWhiteSpace(fileName) + && Path.GetExtension(fileName).Equals(SHORTCUT_EXTENSION, StringComparison.OrdinalIgnoreCase); + } + + /// + /// Resolves a single shortcut hop, or returns when it cannot be followed. + /// + private static string? ResolveOneHop (string fileName) + { + IShellLinkW? shellLink = null; try { + if (!File.Exists(fileName)) + { + _logger.Debug("Shortcut {Shortcut} does not exist, keeping it", fileName); + return null; + } + if (new FileInfo(fileName).Length > MAX_SHORTCUT_SIZE) { _logger.Warn("Ignoring {Shortcut}: it is above the {Limit} byte shortcut limit", fileName, MAX_SHORTCUT_SIZE); - return fileName; + return null; } - IShellLinkW shellLink = (IShellLinkW)new CShellLinkW(); + shellLink = (IShellLinkW)new CShellLinkW(); ((IPersistFile)shellLink).Load(fileName, STGM_READ); var buffer = new StringBuilder(TARGET_PATH_CAPACITY); @@ -92,27 +149,25 @@ public static string Resolve (string fileName) if (target.Length == 0) { _logger.Debug("Shortcut {Shortcut} has no target path, keeping it", fileName); - return fileName; + return null; } // A relative target sits next to the shortcut, not in the current working directory. var shortcutDirectory = Path.GetDirectoryName(Path.GetFullPath(fileName))!; - // The target string comes straight from the .lnk bytes, so it can be a UNC or device - // path in disguise (//host/share, \\?\UNC\...). Only a plain local drive path is - // followed, because probing anything else would authenticate against a remote host, - // handing it the user's NetNTLM hash. + // The target comes straight from the .lnk bytes, so only follow a plain local drive path: + // probing a UNC/device path would authenticate against a remote host (NetNTLM leak). var fullTarget = SanitizeLocalTarget(target, shortcutDirectory); if (fullTarget is null) { _logger.Debug("Shortcut target {Target} is not a local drive path, keeping {Shortcut}", target, fileName); - return fileName; + return null; } if (!File.Exists(fullTarget)) { _logger.Debug("Shortcut target {Target} does not exist, keeping {Shortcut}", target, fileName); - return fileName; + return null; } return fullTarget; @@ -124,39 +179,14 @@ or ArgumentException or NotSupportedException) { _logger.Debug(ex, "Could not resolve shortcut {Shortcut}, keeping it", fileName); - return fileName; - } - } - - #endregion - - #region Internal methods - - /// - /// Turns a raw shortcut target into the full path to follow — but only when it lands on a - /// plain local drive (X:\). Environment variables are expanded and relative targets are - /// resolved against the shortcut's directory; anything else (UNC, device paths like - /// \\?\ or \\.\, malformed strings) yields , because - /// probing such a path would authenticate against a remote host and leak the user's NetNTLM - /// hash. Pure string handling: no file system access. - /// - /// The raw target string from the .lnk. - /// Directory the shortcut lives in; the base for relative targets. - internal static string? SanitizeLocalTarget (string target, string shortcutDirectory) - { - try - { - var expanded = Environment.ExpandEnvironmentVariables(target); - var fullTarget = Path.GetFullPath(expanded, shortcutDirectory); - - var root = Path.GetPathRoot(fullTarget); - return root is not null && root.Length == 3 && root[1] == ':' ? fullTarget : null; + return null; } - catch (Exception ex) when (ex is ArgumentException - or IOException - or NotSupportedException) + finally { - return null; + if (shellLink is not null) + { + Marshal.FinalReleaseComObject(shellLink); + } } } From bd755d0623132e24041a9d75526a3a31057e7e8c Mon Sep 17 00:00:00 2001 From: mfkeeq <546357529@qq.com> Date: Wed, 28 Oct 2026 11:18:22 +0800 Subject: [PATCH 4/4] Resolve shortcuts once per entry point Add resolved-name tab creation paths so every entry point resolves a shortcut exactly once. A single dropped shortcut used to be resolved three times (up to 9 COM loads and 3 cap warnings); it is now resolved once. AddMultiFileTab reuses its already-resolved names when splitting out .lxj files. Share temp-directory setup through TestShortcuts, remove the helper doc comments called out in review, and mark the new shortcut tests with Arrange/Act/Assert. --- .../Services/FileOperationServiceTests.cs | 33 ++++++++---------- .../Services/ShortcutResolverTests.cs | 13 ++----- .../Services/TestShortcuts.cs | 16 ++++++--- .../FileOperationService.cs | 34 +++++++------------ .../FileOperationService/ShortcutResolver.cs | 10 ------ 5 files changed, 41 insertions(+), 65 deletions(-) diff --git a/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs b/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs index 899386ae..6d1d1f8b 100644 --- a/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs +++ b/src/LogExpert.UI.Tests/Services/FileOperationServiceTests.cs @@ -509,8 +509,7 @@ public void LoadDroppedFiles_Ask_UsesOnlyTheConfirmedSelection (MultiFileDecisio [TestCase(1, true, 1)] public void LoadDroppedFiles_CombinedChoice_ReadsAccessibleSelectedFiles (int unavailableIndex, bool lockFile, int expectedLines) { - var directory = Path.Join(Path.GetTempPath(), "LogExpertDropRouting", Guid.NewGuid().ToString()); - _ = Directory.CreateDirectory(directory); + var directory = TestShortcuts.CreateTempDirectory("DropRouting"); var first = Path.Join(directory, "a.log"); var second = Path.Join(directory, "b.txt"); File.WriteAllText(first, "first\n"); @@ -575,8 +574,7 @@ public void LoadDroppedFiles_UnavailableFile_DoesNotBlockRemainingSelection (boo [Test] public void LoadDroppedFiles_CombinedSessionAndLog_DoesNotOpenUnselectedRotations () { - var directory = Path.Join(Path.GetTempPath(), "LogExpertDropRouting", Guid.NewGuid().ToString()); - _ = Directory.CreateDirectory(directory); + var directory = TestShortcuts.CreateTempDirectory("DropRouting"); var log = Path.Join(directory, "b.log"); var session = Path.Join(directory, "session.lxj"); File.WriteAllText(log, "selected\n"); @@ -606,8 +604,7 @@ public void LoadDroppedFiles_CombinedSessionAndLog_DoesNotOpenUnselectedRotation [Test] public void LoadDroppedFiles_CombinedSelectionAfterTruncation_KeepsOnlySelectedPaths () { - var directory = Path.Join(Path.GetTempPath(), "LogExpertDropRouting", Guid.NewGuid().ToString()); - _ = Directory.CreateDirectory(directory); + var directory = TestShortcuts.CreateTempDirectory("DropRouting"); var first = Path.Join(directory, "a.log"); var second = Path.Join(directory, "b.log"); File.WriteAllText(first, "first\n"); @@ -660,13 +657,15 @@ public void LoadFilesWithOption_SingleFile_CallsAddFileTab () [Test] public void LoadFilesWithOption_ShortcutToLxjFile_CallsProjectCallback () { - // The .lxj check has to run on the resolved name, or the shortcut gets opened as a raw log. + // Arrange — the .lxj check has to run on the resolved name, or the shortcut gets opened as a raw log. var (directory, shortcut, session) = CreateShortcutToSession(); try { + // Act var decision = _sut.LoadFilesWithOption(new[] { shortcut }, false); + // Assert Assert.That(decision, Is.EqualTo(MultiFileDecision.Cancel)); Assert.That(_projectCallbackCalls, Has.Count.EqualTo(1)); Assert.That(_projectCallbackCalls[0].FileName, Is.EqualTo(session)); @@ -681,12 +680,15 @@ public void LoadFilesWithOption_ShortcutToLxjFile_CallsProjectCallback () [Test] public void AddFileTabs_ShortcutToLxjFile_CallsProjectCallback () { + // Arrange var (directory, shortcut, session) = CreateShortcutToSession(); try { + // Act _sut.AddFileTabs(new[] { shortcut }); + // Assert Assert.That(_projectCallbackCalls, Has.Count.EqualTo(1)); Assert.That(_projectCallbackCalls[0].FileName, Is.EqualTo(session)); Assert.That(_factoryCalls, Is.Empty, "Should not open the shortcut as a log"); @@ -700,8 +702,8 @@ public void AddFileTabs_ShortcutToLxjFile_CallsProjectCallback () [Test] public void AddFileTab_ShortcutToLogFile_OpensTarget () { - var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", "ShortcutLog", Guid.NewGuid().ToString()); - _ = Directory.CreateDirectory(directory); + // Arrange + var directory = TestShortcuts.CreateTempDirectory("ShortcutLog"); try { @@ -711,8 +713,10 @@ public void AddFileTab_ShortcutToLogFile_OpensTarget () var shortcut = Path.Join(directory, "actual.lnk"); TestShortcuts.Create(shortcut, log); + // Act _sut.AddFileTab(new FileTabRequest { FileName = shortcut }); + // Assert Assert.That(_factoryCalls, Has.Count.EqualTo(1)); Assert.That(_factoryCalls[0].Request.FileName, Is.EqualTo(log), "the target file is opened, not the shortcut"); } @@ -1290,14 +1294,9 @@ public void LoadFiles_DelegatesToAddFileTabs () Assert.That(_factoryCalls[1].Request.FileName, Is.EqualTo("b.log")); } - /// - /// Creates a temporary directory holding a session file and a shortcut to it, and returns the - /// three paths. The caller owns the directory and deletes it. - /// private static (string Directory, string Shortcut, string Session) CreateShortcutToSession () { - var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", "ShortcutSession", Guid.NewGuid().ToString()); - _ = Directory.CreateDirectory(directory); + var directory = TestShortcuts.CreateTempDirectory("ShortcutSession"); var session = Path.Join(directory, "project.lxj"); File.WriteAllText(session, ""); @@ -1308,10 +1307,6 @@ private static (string Directory, string Shortcut, string Session) CreateShortcu return (directory, shortcut, session); } - /// - /// Builds a second service on the shared config manager, standing in for another LogTabWindow - /// of the same session with its own tab controller. - /// private FileOperationService CreateServiceForWindows (params LogWindow[] windows) { var tabControllerMock = new Mock(); diff --git a/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs b/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs index c46f87cf..783f56ac 100644 --- a/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs +++ b/src/LogExpert.UI.Tests/Services/ShortcutResolverTests.cs @@ -13,7 +13,7 @@ internal class ShortcutResolverTests [Test] public void Resolve_ShortcutToLocalTarget_ReturnsTarget () { - var directory = CreateTempDirectory("ShortcutLocal"); + var directory = TestShortcuts.CreateTempDirectory("ShortcutLocal"); try { @@ -34,7 +34,7 @@ public void Resolve_ShortcutToLocalTarget_ReturnsTarget () [Test] public void Resolve_ShortcutToUncTarget_ReturnsShortcutUnchanged () { - var directory = CreateTempDirectory("ShortcutUnc"); + var directory = TestShortcuts.CreateTempDirectory("ShortcutUnc"); try { @@ -53,7 +53,7 @@ public void Resolve_ShortcutToUncTarget_ReturnsShortcutUnchanged () [Test] public void Resolve_OversizedShortcutFile_ReturnsPathUnchanged () { - var directory = CreateTempDirectory("ShortcutOversized"); + var directory = TestShortcuts.CreateTempDirectory("ShortcutOversized"); try { @@ -168,11 +168,4 @@ private static Dictionary CreateChain (int linkCount) return targets; } - - private static string CreateTempDirectory (string name) - { - var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", name, Guid.NewGuid().ToString()); - _ = Directory.CreateDirectory(directory); - return directory; - } } diff --git a/src/LogExpert.UI.Tests/Services/TestShortcuts.cs b/src/LogExpert.UI.Tests/Services/TestShortcuts.cs index c919d9b1..8b2cd218 100644 --- a/src/LogExpert.UI.Tests/Services/TestShortcuts.cs +++ b/src/LogExpert.UI.Tests/Services/TestShortcuts.cs @@ -8,25 +8,31 @@ namespace LogExpert.UI.Tests.Services; [SupportedOSPlatform("windows")] internal static class TestShortcuts { + public static string CreateTempDirectory (string name) + { + var directory = Path.Join(Path.GetTempPath(), "LogExpertTests", name, Guid.NewGuid().ToString()); + _ = Directory.CreateDirectory(directory); + return directory; + } + public static void Create (string shortcutPath, string targetPath) { var shellType = Type.GetTypeFromProgID("WScript.Shell"); Assert.That(shellType, Is.Not.Null, "WScript.Shell should be available on Windows test machines"); dynamic shell = Activator.CreateInstance(shellType)!; - object? shortcut = null; + dynamic? link = null; try { - dynamic link = shell.CreateShortcut(shortcutPath); - shortcut = link; + link = shell.CreateShortcut(shortcutPath); link.TargetPath = targetPath; link.Save(); } finally { - if (shortcut != null) + if (link != null) { - Marshal.FinalReleaseComObject(shortcut); + Marshal.FinalReleaseComObject(link); } Marshal.FinalReleaseComObject(shell); diff --git a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs index 4f4dbfc2..096391a4 100644 --- a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs +++ b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs @@ -68,8 +68,11 @@ internal sealed class FileOperationService ( public LogWindow AddFileTab (FileTabRequest request) { - request = request with { FileName = ShortcutResolver.Resolve(request.FileName) }; + return AddResolvedFileTab(request with { FileName = ShortcutResolver.Resolve(request.FileName) }); + } + private LogWindow AddResolvedFileTab (FileTabRequest request) + { var logFileName = PersisterHelpers.FindFilenameForSettings(request.FileName, _pluginRegistry); var normalizedPath = Path.GetFullPath(logFileName); @@ -201,13 +204,13 @@ public LogWindow AddTempFileTab (string fileName, string title) public LogWindow? AddMultiFileTab (string[] fileNames) { - if (fileNames.Length < 1) - { - return null; - } - - fileNames = fileNames.Select(ShortcutResolver.Resolve).ToArray(); + return fileNames.Length < 1 + ? null + : AddResolvedMultiFileTab(fileNames.Select(ShortcutResolver.Resolve).ToArray()); + } + private LogWindow? AddResolvedMultiFileTab (string[] fileNames) + { // Session files (.lxj) aren't logs — dispatch them to the project loader and // continue with only the log files. Without this, the .lxj would be treated as // a raw log line in the multi-file tab. @@ -216,7 +219,7 @@ public LogWindow AddTempFileTab (string fileName, string title) AddFileTabs(fileNames.Where(f => f.EndsWith(".lxj", StringComparison.OrdinalIgnoreCase)).ToArray()); var logs = fileNames.Where(f => !f.EndsWith(".lxj", StringComparison.OrdinalIgnoreCase)).ToArray(); - return logs.Length > 0 ? AddMultiFileTab(logs) : null; + return logs.Length > 0 ? AddResolvedMultiFileTab(logs) : null; } EncodingOptions encodingOptions = new(); @@ -270,12 +273,6 @@ public void LoadDroppedFiles (string[] selectedFiles, bool invertLogic, Func - /// Resolves a possible shortcut and dispatches a resolved session file (.lxj) to the - /// session loader. Returns the name to open as a log, or when the file - /// was a session and has been handled. - /// private string? ResolveLogFile (string fileName, bool asFirstFile) { var resolved = ShortcutResolver.Resolve(fileName); diff --git a/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs b/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs index e4e020fc..1e77c167 100644 --- a/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs +++ b/src/LogExpert.UI/Services/FileOperationService/ShortcutResolver.cs @@ -69,8 +69,6 @@ internal static string FollowChain (string fileName, Func resol if (hop >= MAX_SHORTCUT_HOPS) { - // Keep the input, not the partially followed path: another call site resolving - // the result again would otherwise continue past the limit. _logger.Warn("Shortcut chain starting at {Shortcut} exceeds {Limit} hops; keeping it", fileName, MAX_SHORTCUT_HOPS); return fileName; } @@ -85,10 +83,6 @@ internal static string FollowChain (string fileName, Func resol } } - /// - /// Normalizes a raw shortcut target and returns it only when it is a plain local drive path - /// (X:\); anything that could reach the network yields . - /// internal static string? SanitizeLocalTarget (string target, string shortcutDirectory) { try @@ -118,9 +112,6 @@ private static bool IsShortcut (string fileName) && Path.GetExtension(fileName).Equals(SHORTCUT_EXTENSION, StringComparison.OrdinalIgnoreCase); } - /// - /// Resolves a single shortcut hop, or returns when it cannot be followed. - /// private static string? ResolveOneHop (string fileName) { IShellLinkW? shellLink = null; @@ -152,7 +143,6 @@ private static bool IsShortcut (string fileName) return null; } - // A relative target sits next to the shortcut, not in the current working directory. var shortcutDirectory = Path.GetDirectoryName(Path.GetFullPath(fileName))!; // The target comes straight from the .lnk bytes, so only follow a plain local drive path: