Escape file URIs (#1189)

* Escape file URIs

* Fix test

* Add tests

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;

src/Microsoft.SqlTools.ServiceLayer/NotebookConvert/NotebookConvertService.cs

@@ -101,7 +101,11 @@ namespace Microsoft.SqlTools.ServiceLayer.NotebookConvert
 
                 try
                 {
-                    var file = WorkspaceService<SqlToolsSettings>.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<SqlToolsSettings>.Instance.Workspace.GetFile(escapedClientUri);
                     // Temporary notebook that we just fill in with the sql until the parsing logic is added
                     var result = new ConvertSqlToNotebookResult
                     {

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

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<ExecuteRequestParamsBase>().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<InvalidCastException>(() => queryService.GetSqlText(mockParams));
+            Assert.Throws<InvalidCastException>(() => queryService.GetSqlText(invalidParams));
         }
 
         #endregion

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<SqlToolsSettings> {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]