[mfkeeq199458]

Opening or dropping a shortcut (.lnk) now opens the log file it points at instead of the shortcut itself.

Resolution follows a chain of shortcuts to its final local target, bounded by MAX_SHORTCUT_HOPS = 3. A broken, cyclic, 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.

Safety: targets are normalized (environment variables expanded, relative paths resolved against the shortcut's directory) and only plain local drive paths are followed; UNC and device forms are rejected before any File.Exists probe. Mapped network drives are intentionally trusted, since the user created the mapping. A .lnk larger than 1 MiB is not resolved, but it is not skipped either: it opens as the file the user selected.

When a shortcut cannot be followed (broken, cyclic, over the cap, non-local target, or target missing), it still opens as a file, but the user is now told: FileOpenedEventArgs carries the unresolved paths and LogTabWindow shows one message per batch listing them, at most once per path per session.

Review follow-up: shortcuts are now resolved once per entry point; a single dropped shortcut previously went through three resolutions (up to 9 COM loads and 3 cap warnings). ResolveLogFile still resolves once because the .lxj decision must run on the resolved name — removing it would open a shortcut-to-session as a raw log. LoadFilesWithOption no longer writes resolved names into the caller's array. AddResolvedMultiFileTab sorts the already-resolved names before the .lxj split, so multi-file order follows target names again on every path — drop, command line and Ask → MultiFile included; the Array.Sort in LoadFilesWithOption is now only a pre-pass on unresolved names. AddMultiFileTab's .lxj branch still delegates to AddFileTabs, which performs one extra extension-only Resolve on .lxj names (no COM load, no cap warning).

[Hirogen]

Thanks @mfkeeq199458 — nice feature, and good call to avoid IShellLink::Resolve entirely (no link tracking / network search) and to keep the target, not the .lnk, in file history. All entry paths (drop, command line, second-instance pipe, history, session restore, multi-file) go through it.

A few things before merging:

Security — UNC guard
target.StartsWith(@"\\") (ShortcutResolver.cs:95) only catches the canonical form. With SLGP_RAWPATH the target string comes straight from the .lnk bytes, so a hand-crafted link with //evil/share/x.log or \??\UNC\evil\share\x.log would get past the check and reach File.Exists, which is exactly the NetNTLM leak the comment describes. Could you normalise first (e.g. Path.GetFullPath) and then reject anything that isn't a rooted local drive path (Path.GetPathRoot is X:\, not UNC/device)? A test with forward slashes and with \\?\ / \\.\ prefixes would lock this in.

Use Vanara instead of hand-rolled COM
LogExpert.UI already references Vanara.PInvoke.Shell32, which provides ShellLink / IShellLinkW. Using it would drop the ~40 lines of [ComImport] declarations and the hand-ordered IPersistFile vtable, and match how the rest of the codebase does interop.

Path handling

• SLGP_RAWPATH leaves %VAR% targets unexpanded. Please call Environment.ExpandEnvironmentVariables (or drop RAWPATH).
• Relative targets should be resolved against the .lnk's directory, not the working directory.
• The 260-char buffer truncates long-path targets; consider a larger buffer.

Behaviour

• Shortcuts are resolved twice on the LoadFilesWithOption/AddFileTabs path (and again in AddFileTab), so a link-to-link is followed 2 hops there but 1 hop from history. Resolving in one place would make it consistent.
• Array.Sort runs before resolution (:281), so multi-file order follows shortcut names rather than target names. Resolve, then sort.
• When a shortcut can't be followed (UNC, broken, oversized) it currently opens the .lnk as binary garbage with only a log line. A short message to the user would be friendlier. Not blocking, but worth considering.

Small cleanups

• The "resolve then check .lxj" pattern is repeated three times in FileOperationService; a small helper would tidy it.
• COMException already derives from ExternalException in the catch filter.
• var at :79, a using System.Text; instead of the fully qualified StringBuilder, and the comments in FileOperationService that restate the code can go.
• Tests: CreateShortcut is duplicated across both test files, and the oversized test hardcodes 1024 * 1024 + 1. Consider exposing the limit as internal and reusing it. Please also release the WScript.Shell COM objects.

Thanks — the UNC check and Vanara are the two I'd like to see addressed; the rest is polish.

[Hirogen]

Thanks @mfkeeq199458 — big improvement. The guard is now string-only (expand → GetFullPath against the shortcut's directory → require an X:\ root), so nothing touches the network before the check. The tests for //host/share, \\?\UNC, \??\UNC, \\.\ and %LOGONSERVER% cover exactly what I was worried about. Vanara, env-var expansion, the relative-target handling, resolve-then-sort and the ResolveLogFile helper all look good, and the new test classes pass locally (85/85).

One functional thing left, plus some polish:

Hop count (please fix)
The commit message says "resolution is one hop", but a shortcut is still resolved several times along one call chain:

• drop or command line, single file: LoadFilesWithOption loop (:280) → ResolveLogFile (:349) → AddFileTab (:71) = 3×
• SingleFiles / AskUser: pre-resolve at :280 → ResolveLogFile → AddFileTab = 3×
• history and session restore: AddFileTab only = 1×

So a link-to-link chain is followed 1 to 3 hops depending on how the file was opened, and each step loads a COM object. The simplest fix is to make Resolve idempotent: if the target itself ends in .lnk, treat it as unfollowable and return the input. Then repeated calls are cheap no-ops (they short-circuit on the extension) and the hop count is always exactly one. Then please drop the redundant call sites. Resolving in ResolveLogFile and AddFileTab (plus AddMultiFileTab) should cover every path. A test with a shortcut pointing to another shortcut would lock this in.

Small things

• ShortcutResolver.cs:~86: please release the CShellLinkW in a finally (Marshal.FinalReleaseComObject), as TestShortcuts already does.
• FileOperationService.cs:71-75: the string.Equals check isn't needed. request = request with { FileName = ShortcutResolver.Resolve(request.FileName) }; is enough.
• LoadFilesWithOption now overwrites the caller's array with resolved paths, and LoadDroppedFiles relies on that implicitly. Fine if it stays, but it becomes moot once the extra resolves are gone.
• Comments: we keep them minimal in this repo. The class remarks, the per-constant doc comments, and the NetNTLM explanation, which appears twice, could shrink to a line or two. The 7-line test class summary can go.
• Test names: Sanitize_* → SanitizeLocalTarget_*, to match Resolve_*.
• The PR description says "oversized files are not followed", but the check is on the .lnk size, not the target. Please adjust the wording.

Mapped network drives: a target on a mapped drive (Z:\…) passes the guard, so File.Exists will contact that server. That's fine: the user mapped the drive themselves, so it's a trusted location. No change needed. A one-line comment at the root check saying this is intentional would help the next reader.

Once the hop count is consistent, I'm happy to merge. The rest is polish. Thanks!

[Hirogen]

Thanks @mfkeeq199458 — all the polish from last round landed: finally release, the string.Equals removal, trimmed comments, the SanitizeLocalTarget_* rename and the mapped-drive note. The guard is still solid too. Every intermediate hop goes through SanitizeLocalTarget before any File.Exists, so a chain can't reach the network either.

Chain following
This isn't what I suggested (one hop, idempotent), but I'm fine with it. Returning the original input when the cap is hit makes the result the same on every entry path, so history, session restore and drop now open the same file. Let's keep it.

Redundant call sites (please fix)
The other half of my last request is still open: "drop the redundant call sites". A single dropped file is still resolved three times: in the LoadFilesWithOption loop (:276), then in ResolveLogFile (:345), then in AddFileTab (:71). AddMultiFileTab (:209) is a fourth site. For a shortcut that can't be followed, or one that hits the cap, that's up to 9 COM loads and the cap warning logged three times. The comment at ShortcutResolver.cs:72-73 only exists to work around this. Please resolve once per entry point: AddFileTab and AddMultiFileTab cover history, session restore and multi-file, so LoadFilesWithOption and ResolveLogFile shouldn't need their own calls. Once those are gone, LoadFilesWithOption also no longer needs to overwrite the caller's array.

One ordering detail to check after that change: the Open File dialog's Ask → MultiFile path re-sorts openFileDialog.FileNames (the unresolved names), so multi-file order there follows shortcut names, not target names.

PR description
The description still says "oversized files are not followed" and still describes a single hop. The correction is only in the a4b40e2 commit message. Please move it into the PR description.

Optional polish

• ResolveOneHop, SanitizeLocalTarget, ResolveLogFile and the private test helper don't need XML doc comments. The comment at ShortcutResolver.cs:155 restates the GetFullPath call.
• The three new tests in FileOperationServiceTests.cs could use // Arrange / // Act / // Assert like the rest of the file.
• Temp-directory setup is duplicated across both test files; CreateTempDirectory could be shared via TestShortcuts.
• TestShortcuts.cs:17-21 keeps both object? shortcut and dynamic link for the same COM object.

With the redundant call sites gone and the description updated, this is good to merge from my side. Thanks!

[Hirogen]

Thanks @mfkeeq199458 — round 4 does what I asked. I traced every entry path (drop, command line, second-instance pipe, history, session restore, multi-file, and Open File SingleFiles/MultiFile/Ask): each one resolves a shortcut exactly once, at most 3 COM loads, and the cap warning is logged at most once. Every hop still goes through SanitizeLocalTarget before File.Exists. The .lxj reasoning for keeping the resolve in ResolveLogFile is right, and the new test covers it. Build adds no warnings and the 89 shortcut/service tests pass. The polish from last round landed too.

One thing before merge: multi-file sort order
Moving the resolve to the entry points undid the round-1 fix. LoadFilesWithOption (:276) sorts before anything is resolved, so multi-file order now follows shortcut names again. Thanks for saying so in the description. Still, I'd rather keep target-name order: sorting the resolved names inside AddResolvedMultiFileTab (after Select(ShortcutResolver.Resolve)) would give the same order on every path, Ask → MultiFile included.

Small things

• ShortcutResolverTests: please add // Arrange / // Act / // Assert like the rest of the test project.
• ShortcutResolver.cs: the XML docs on the internal class can go. A one-line summary on Resolve is fine if you want to keep one.
• FileOperationServiceTests.cs: please leave the existing CreateServiceForWindows doc and the three existing LoadDroppedFiles_* tests as they were. Using CreateTempDirectory in new tests only keeps the diff focused.
• The a4b40e2 commit message still says multi-file order "follows the target names". After the sort fix above, that'll be true again.

Optional, non-blocking

• A shortcut that's broken, cyclic or over the cap still opens the .lnk as binary with only a log line. A short message to the user ("Couldn't follow shortcut X, opening it as a file") would be friendlier. Happy to track it as a follow-up if you'd rather not do it here.
• STGM_READ and IPersistFile could come from Vanara too, since you already use it for ShellLink.

With the sort order fixed, this is good to merge from my side. Thanks!

[mfkeeq199458]

Thanks @Hirogen — all four items are fixed, and both optional ones are handled. Pushed as 9daeddfc (sort order, user message, test polish) and f7bd2bd9 (Vanara STGM).

Multi-file sort order (the blocker)

Moved the sort into AddResolvedMultiFileTab, immediately after the Select(ShortcutResolver.Resolve) and before the .lxj split, so it sorts the resolved names. Every path funnels through there — drop, command line, SingleFiles, and Ask → MultiFile — so the order is target-name order again on all of them. AddMultiFileTab_Shortcuts_SortsByResolvedTargetName locks it in: the inputs are z.lnk → a.log and a.lnk → b.log in that order, so shortcut-name order would give b.log, a.log while the test expects a.log, b.log.

Small things

• ShortcutResolverTests: // Arrange / // Act / // Assert added to all 11 tests.
• ShortcutResolver.cs: class XML docs gone, Resolve keeps a one-line summary.
• FileOperationServiceTests.cs: the three LoadDroppedFiles_* tests are back on their original temp-directory setup and the CreateServiceForWindows doc is restored — CreateTempDirectory is only used by the new tests now. Both helpers are byte-identical to the base commit.
• a4b40e27: I did not rewrite it, since the hash is quoted in this thread. "Multi-file order follows the target names" is true again after the sort fix; the part of its body that says "resolve before sorting in LoadFilesWithOption" is the stale bit, so I moved the corrected wording into the PR description instead.

Optional: telling the user

Implemented. FileOpenedEventArgs gained UnresolvedShortcuts; AddResolvedFileTab fills it when the name is still a .lnk after resolution and AddResolvedMultiFileTab collects the ones left in the batch. LogTabWindow coalesces them through BeginInvoke, so a folder of broken links produces one message per batch rather than one per file, and remembers what it has already reported to keep it to once per path per session. New resources: LogTabWindow_UI_Message_UnresolvedShortcut_Single / _Multiple, translated for en, de and zh-CN.

This covers every way resolution can fail: broken link, cycle, over the cap, UNC/device target, missing target.

Optional: Vanara types — half of it

STGM is adopted: Vanara.PInvoke.STGM.STGM_READ, and the local STGM_READ constant is gone. It lives in Vanara.Core, which Vanara.PInvoke.Shell32 already brings in transitively, so no new package.

IPersistFile I could not move. I reflected over Vanara.PInvoke.Shell32, Vanara.PInvoke.Ole and Vanara.Core (5.0.7): none of them export the type, and IShellLinkW's Load/Save are inherited from System.Runtime.InteropServices.ComTypes.IPersistFile. So the cast stays on the BCL interface — it is the same interface, just declared by Vanara's dependency rather than by Vanara. Getting Vanara's own version would mean adding Vanara.Windows.Shell, which seemed like too much for a type we already have.

One change you didn't ask for

AddFileTab_RaisesFileOpenedEvent now uses Is.EqualTo instead of Is.SameAs. AddFileTab builds its request with request with { FileName = ... }, which produces a new record instance, so reference equality can no longer hold. Happy to restructure the code instead if you'd rather the test stayed as it was.

Verification

Build clean, no new analyzer warnings on the touched files. Full LogExpert.UI.Tests run: 392 passed, 0 failed (389 before, plus the three above).

Should be good to merge now — let me know if anything else needs tightening.

[Hirogen]

Thanks @mfkeeq199458 — the sort-order blocker is fixed. I traced drop, command line, Open File → MultiFile and Ask → MultiFile: they all sort the resolved names in AddResolvedMultiFileTab, on a fresh copy, so the caller's array isn't touched. Every path still resolves a shortcut exactly once. STGM from Vanara and the reasoning on IPersistFile both look good. The Is.SameAs → Is.EqualTo change is fine: FileTabRequest is a record that gets rebuilt with with, so reference equality can't hold.

Thanks for doing the user message as well. A few notes on it, none of them blocking:

• Which path is shown. For a broken chain a.lnk → b.lnk, FollowChain returns current, so the tab and the message show b.lnk, not the file the user picked. A chain that hits the cap returns the original instead. I'd return fileName in both cases, so the user always sees what they opened and the two failure modes behave the same.
• Oversized / fake .lnk. A .lnk over 1 MiB, or any file that is only named *.lnk, now also gets "Couldn't follow the shortcut". That's fine by me, but please mention it in the description, since it currently says these files just open as selected.
• Dedupe key. _reportedUnresolvedShortcuts compares the raw FileName, so logs\x.lnk from the command line and C:\…\logs\x.lnk from the dialog are reported twice. Path.GetFullPath before adding would fix that.
• Session restore. A broken shortcut stays in history and in the last-open list, so restore warns again on every launch. That's acceptable. Just flagging it in case you'd rather not keep unresolved .lnk entries in history.
• Tests. The batching and the dedupe aren't covered at all. If you pull the three fields and two methods out of LogTabWindow into a small UnresolvedShortcutNotifier (with a callback for showing the message), it becomes testable and the form gets a bit thinner. Optional, but it fits where we're heading with LogTabWindow.

Small things

• FileOperationServiceTests.cs: the new CreateShortcutToSession has an XML <summary> again. Please drop it, the same as the other private helpers.
• FileOpenedEventArgs.UnresolvedShortcuts: a one-line summary like its siblings is enough.
• ShortcutResolver.IsShortcut is internal now. Please move it into the Internal methods region.
• Ask → MultiFile sorts twice (LoadFilesWithOption and AddResolvedMultiFileTab). It's harmless, but the comment in AddResolvedMultiFileTab reads as if the sort moved, so the first one could go if nothing else depends on it.
• Minor: your reply says SingleFiles also goes through AddResolvedMultiFileTab. It doesn't; it goes through AddFileTabs. That's fine, because they're separate tabs.

None of this blocks the merge. Happy to merge as is, or after a quick pass on the path-shown and dedupe points, your call. Thanks — this turned out really solid!