diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs index 97ed65a2..48d15fcd 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs @@ -743,8 +743,12 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { Logger.Write(TraceEventType.Verbose, "HandleRebuildIntelliSenseNotification"); + // This URI doesn't come in escaped - so if it's a file path with reserved characters (such as %) + // then we'll fail to find it since GetFile expects the URI to be a fully-escaped URI as that's + // what the document events are sent in as. + var escapedOwnerUri = Uri.EscapeUriString(rebuildParams.OwnerUri); // Skip closing this file if the file doesn't exist - var scriptFile = this.CurrentWorkspace.GetFile(rebuildParams.OwnerUri); + var scriptFile = this.CurrentWorkspace.GetFile(escapedOwnerUri); if (scriptFile == null) { return; @@ -1078,7 +1082,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices { if (scriptInfo.IsConnected) { - var scriptFile = CurrentWorkspace.GetFile(info.OwnerUri); + // This URI doesn't come in escaped - so if it's a file path with reserved characters (such as %) + // then we'll fail to find it since GetFile expects the URI to be a fully-escaped URI as that's + // what the document events are sent in as. + var fileUri = Uri.EscapeUriString(info.OwnerUri); + var scriptFile = CurrentWorkspace.GetFile(fileUri); if (scriptFile == null) { return; diff --git a/src/Microsoft.SqlTools.ServiceLayer/NotebookConvert/NotebookConvertService.cs b/src/Microsoft.SqlTools.ServiceLayer/NotebookConvert/NotebookConvertService.cs index 460cefc7..bd795151 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/NotebookConvert/NotebookConvertService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/NotebookConvert/NotebookConvertService.cs @@ -101,7 +101,11 @@ namespace Microsoft.SqlTools.ServiceLayer.NotebookConvert try { - var file = WorkspaceService.Instance.Workspace.GetFile(parameters.ClientUri); + // This URI doesn't come in escaped - so if it's a file path with reserved characters (such as %) + // then we'll fail to find it since GetFile expects the URI to be a fully-escaped URI as that's + // what the document events are sent in as. + var escapedClientUri = Uri.EscapeUriString(parameters.ClientUri); + var file = WorkspaceService.Instance.Workspace.GetFile(escapedClientUri); // Temporary notebook that we just fill in with the sql until the parsing logic is added var result = new ConvertSqlToNotebookResult { diff --git a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/QueryExecutionService.cs b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/QueryExecutionService.cs index 77d7c9f3..6a411987 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/QueryExecutionService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/QueryExecutionService.cs @@ -930,18 +930,22 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution // Internal for testing purposes internal string GetSqlText(ExecuteRequestParamsBase request) { + // This URI doesn't come in escaped - so if it's a file path with reserved characters (such as %) + // then we'll fail to find it since GetFile expects the URI to be a fully-escaped URI as that's + // what the document events are sent in as. + var escapedOwnerUri = Uri.EscapeUriString(request.OwnerUri); // If it is a document selection, we'll retrieve the text from the document ExecuteDocumentSelectionParams docRequest = request as ExecuteDocumentSelectionParams; if (docRequest != null) { - return GetSqlTextFromSelectionData(docRequest.OwnerUri, docRequest.QuerySelection); + return GetSqlTextFromSelectionData(escapedOwnerUri, docRequest.QuerySelection); } // If it is a document statement, we'll retrieve the text from the document ExecuteDocumentStatementParams stmtRequest = request as ExecuteDocumentStatementParams; if (stmtRequest != null) { - return GetSqlStatementAtPosition(stmtRequest.OwnerUri, stmtRequest.Line, stmtRequest.Column); + return GetSqlStatementAtPosition(escapedOwnerUri, stmtRequest.Line, stmtRequest.Column); } // If it is an ExecuteStringParams, return the text as is @@ -964,6 +968,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution ScriptFile queryFile = WorkspaceService.Workspace.GetFile(ownerUri); if (queryFile == null) { + Logger.Write(TraceEventType.Warning, $"[GetSqlTextFromSelectionData] Unable to find document with OwnerUri {ownerUri}"); return string.Empty; } // If a selection was not provided, use the entire document diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/ServiceIntegrationTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/ServiceIntegrationTests.cs index d2cc03e1..3a9e8af7 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/ServiceIntegrationTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/ServiceIntegrationTests.cs @@ -18,11 +18,13 @@ using Microsoft.SqlTools.ServiceLayer.Test.Common; using Microsoft.SqlTools.ServiceLayer.Test.Common.RequestContextMocking; using Microsoft.SqlTools.ServiceLayer.UnitTests.Utility; using Microsoft.SqlTools.ServiceLayer.Workspace; -using Moq; using NUnit.Framework; namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.Execution { + + public class InvalidParams : ExecuteRequestParamsBase { } + public class ServiceIntegrationTests { @@ -131,12 +133,12 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.Execution // ... Mock up an implementation of ExecuteRequestParamsBase // ... Create a query execution service without a connection service or workspace // service (we won't execute code that uses either - var mockParams = new Mock().Object; + var invalidParams = new InvalidParams() { OwnerUri = "" }; var queryService = new QueryExecutionService(null, null); // If: I attempt to get query text from the mock params // Then: It should throw an exception - Assert.Throws(() => queryService.GetSqlText(mockParams)); + Assert.Throws(() => queryService.GetSqlText(invalidParams)); } #endregion diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs index 861f56c2..2e2169f6 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs @@ -198,22 +198,28 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Workspace } [Test] - public async Task WorkspaceContainsFile() + [TestCase(TestObjects.ScriptUri)] + [TestCase("file://some/path%20with%20encoded%20spaces/file.sql")] + [TestCase("file://some/path with spaces/file.sql")] + [TestCase("file://some/fileWith#.sql")] + [TestCase("file://some/fileUriWithQuery.sql?var=foo")] + [TestCase("file://some/fileUriWithFragment.sql#foo")] + public async Task WorkspaceContainsFile(string uri) { var workspace = new ServiceLayer.Workspace.Workspace(); var workspaceService = new WorkspaceService {Workspace = workspace}; - var openedFile = workspace.GetFileBuffer(TestObjects.ScriptUri, string.Empty); + workspace.GetFileBuffer(uri, string.Empty); // send a document open event var openParams = new DidOpenTextDocumentNotification { - TextDocument = new TextDocumentItem { Uri = TestObjects.ScriptUri } + TextDocument = new TextDocumentItem { Uri = uri } }; await workspaceService.HandleDidOpenTextDocumentNotification(openParams, eventContext: null); // verify the file is being tracked by workspace - Assert.True(workspaceService.Workspace.ContainsFile(TestObjects.ScriptUri)); + Assert.True(workspaceService.Workspace.ContainsFile(uri)); } [Test]