Fix crash when setBreakpoint from VSCode sends a git:/ URI...

[rjmholt]

Fixes PowerShell/vscode-powershell#1994

[rjmholt]

Wait, it looks like we already have a test for a path like this and it's expected as a possible in-memory input path:

PowerShellEditorServices/test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs

Line 163 in fd77c0d

string shortUriForm = "git:/c%3A/Users/Keith/GitHub/dahlbyk/posh-git/src/PoshGitTypes.ps1?%7B%22path%22%3A%22c%3A%5C%5CUsers%5C%5CKeith%5C%5CGitHub%5C%5Cdahlbyk%5C%5Cposh-git%5C%5Csrc%5C%5CPoshGitTypes.ps1%22%2C%22ref%22%3A%22~%22%7D";

@rkeithhill is it possible this is already fixed?

[TylerLeonhardt]

@rjmholt multiple folks mentioned the debug adapter in the issues... it could be a different code path?

[rkeithhill]

I thought I had suggested a code change like this before but we eventually decided that the GetFile() call on line 199 would throw in this case and we'd return false. That said, I'd rather test this in a way that doesn't rely on catching an exception.

[TylerLeonhardt]

Sorry @rkeithhill, are you for this change?

[rkeithhill]

Yes.

[rjmholt]

My worry is that there seems to be a test that exists already for a git:/ schemed file. My change would just reject those (and anything else that isn't file, untitled or inmemory). The fact that we already have a test for this makes me feel like there is an expectation that we can understand and handle a URI like this -- does that seem right?

[rkeithhill]

Those tests for git:/ and gitlens-git:/ look like my tests. I think the intent here was to have those uri schemes return true for IsInMemory which we used to signal that you can't do things like set breakpoints with an "in memory" document. In retrospect, perhaps that is not the best approach RE the way we used IsInMemory but the basic idea was to determine whether or not we had a file path we could provide to Set-PSBreakpoint.

[rkeithhill]

In an ideal design, I could walk up to the ScriptFile/EditorDocument and query a property like CanHaveBreakpoints or something like that to determine whether or not an LSP text document could have a breakpoint set on it. But I think this is good as a stop-gap. Ship it!

[rjmholt]

multiple folks mentioned the debug adapter in the issues... it could be a different code path?

The stack trace I'm working against looks like this:

    System.AggregateException: One or more errors occurred. ---> System.ArgumentException: Illegal characters in path.
       at System.Security.Permissions.FileIOPermission.EmulateFileIOPermissionChecks(String fullPath)
       at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
       at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access)
       at Microsoft.PowerShell.EditorServices.Workspace.GetFile(String filePath)
       at Microsoft.PowerShell.EditorServices.Workspace.TryGetFile(String filePath, ScriptFile& scriptFile)
       at Microsoft.PowerShell.EditorServices.Protocol.Server.DebugAdapter.<HandleSetBreakpointsRequest>d__30.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol.MessageDispatcher.<DispatchMessage>d__7.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol.ProtocolEndpoint.<ListenForMessages>d__36.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at Microsoft.PowerShell.EditorServices.Utility.AsyncContext.Start(Func`1 asyncMainFunc, ILogger logger)
       at System.Threading.Tasks.Task.Execute()
       --- End of inner exception stack trace ---
    ---> (Inner Exception #0) System.ArgumentException: Illegal characters in path.
       at System.Security.Permissions.FileIOPermission.EmulateFileIOPermissionChecks(String fullPath)
       at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
       at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access)
       at Microsoft.PowerShell.EditorServices.Workspace.GetFile(String filePath)
       at Microsoft.PowerShell.EditorServices.Workspace.TryGetFile(String filePath, ScriptFile& scriptFile)
       at Microsoft.PowerShell.EditorServices.Protocol.Server.DebugAdapter.<HandleSetBreakpointsRequest>d__30.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol.MessageDispatcher.<DispatchMessage>d__7.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol.ProtocolEndpoint.<ListenForMessages>d__36.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
       at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
       at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
       at Microsoft.PowerShell.EditorServices.Utility.AsyncContext.Start(Func`1 asyncMainFunc, ILogger logger)
       at System.Threading.Tasks.Task.Execute()<---

So Workspace.TryGetFile seems like the right place. What I don't understand is that we seem to have already dealt with this kind of URI scheme elsewhere, but we're still throwing here...

[TylerLeonhardt]

LGTM