Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not throw for user paths that contain a quote character in them. #58357

Merged
merged 1 commit into from
Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,30 @@ public async Task TestNullFilePaths()
Assert.Null(await storage.ReadStreamAsync(document, streamName));
}

[Theory, CombinatorialData, WorkItem(1436188, "https://devdiv.visualstudio.com/DevDiv/_queries/edit/1436188")]
public async Task CacheDirectoryInPathWithSingleQuote(Size size, bool withChecksum, [CombinatorialRange(0, Iterations)] int iteration)
{
_ = iteration;
using var folderRoot = new DisposableDirectory(new TempRoot());
var folder = folderRoot.CreateDirectory(PersistentFolderPrefix + "'" + Guid.NewGuid());
var solution = CreateOrOpenSolution(folder);

var streamName1 = "PersistentService_Solution_WriteReadDifferentInstances1";
var streamName2 = "PersistentService_Solution_WriteReadDifferentInstances2";

await using (var storage = await GetStorageAsync(solution, folder))
{
Assert.True(await storage.WriteStreamAsync(streamName1, EncodeString(GetData1(size)), GetChecksum1(withChecksum)));
Assert.True(await storage.WriteStreamAsync(streamName2, EncodeString(GetData2(size)), GetChecksum2(withChecksum)));
}

await using (var storage = await GetStorageAsync(solution, folder))
{
Assert.Equal(GetData1(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName1, GetChecksum1(withChecksum))));
Assert.Equal(GetData2(size), ReadStringToEnd(await storage.ReadStreamAsync(streamName2, GetChecksum2(withChecksum))));
}
}

[Theory, CombinatorialData]
public async Task PersistentService_Solution_WriteReadDifferentInstances(Size size, bool withChecksum, [CombinatorialRange(0, Iterations)] int iteration)
{
Expand Down Expand Up @@ -911,9 +935,10 @@ private void DoSimultaneousWrites(Func<string, Task> write)
Assert.Empty(exceptions);
}

protected Solution CreateOrOpenSolution(bool nullPaths = false)
protected Solution CreateOrOpenSolution(TempDirectory? persistentFolder = null, bool nullPaths = false)
{
var solutionFile = _persistentFolder.CreateOrOpenFile("Solution1.sln").WriteAllText("");
persistentFolder ??= _persistentFolder;
var solutionFile = persistentFolder.CreateOrOpenFile("Solution1.sln").WriteAllText("");

var info = SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create(), solutionFile.Path);

Expand All @@ -922,12 +947,12 @@ protected Solution CreateOrOpenSolution(bool nullPaths = false)

var solution = workspace.CurrentSolution;

var projectFile = _persistentFolder.CreateOrOpenFile("Project1.csproj").WriteAllText("");
var projectFile = persistentFolder.CreateOrOpenFile("Project1.csproj").WriteAllText("");
solution = solution.AddProject(ProjectInfo.Create(ProjectId.CreateNewId(), VersionStamp.Create(), "Project1", "Project1", LanguageNames.CSharp,
filePath: nullPaths ? null : projectFile.Path));
var project = solution.Projects.Single();

var documentFile = _persistentFolder.CreateOrOpenFile("Document1.cs").WriteAllText("");
var documentFile = persistentFolder.CreateOrOpenFile("Document1.cs").WriteAllText("");
solution = solution.AddDocument(DocumentInfo.Create(DocumentId.CreateNewId(project.Id), "Document1",
filePath: nullPaths ? null : documentFile.Path));

Expand All @@ -939,12 +964,14 @@ protected Solution CreateOrOpenSolution(bool nullPaths = false)

internal async Task<IChecksummedPersistentStorage> GetStorageAsync(
Solution solution,
TempDirectory? persistentFolder = null,
IPersistentStorageFaultInjector? faultInjector = null,
bool throwOnFailure = true)
{
// If we handed out one for a previous test, we need to shut that down first
persistentFolder ??= _persistentFolder;
_storageService?.GetTestAccessor().Shutdown();
var configuration = new MockPersistentStorageConfiguration(solution.Id, _persistentFolder.Path, throwOnFailure);
var configuration = new MockPersistentStorageConfiguration(solution.Id, persistentFolder.Path, throwOnFailure);

_storageService = GetStorageService((IMefHostExportProvider)solution.Workspace.Services.HostServices, configuration, faultInjector, _persistentFolder.Path);
var storage = await _storageService.GetStorageAsync(SolutionKey.ToSolutionKey(solution), CancellationToken.None);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public async Task TestCrashInNewConnection()

// Because instantiating the connection will fail, we will not get back
// a working persistent storage. We are testing a fault recovery code path.
await using (var storage = await GetStorageAsync(solution, faultInjector, throwOnFailure: false))
await using (var storage = await GetStorageAsync(solution, faultInjector: faultInjector, throwOnFailure: false))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added another optional parameter, and this positional one needed to become named.

using (var memStream = new MemoryStream())
using (var streamWriter = new StreamWriter(memStream))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,14 @@ public static SqlConnection Create(IPersistentStorageFaultInjector? faultInjecto
// uri of the DB on disk we're associating this in-memory cache with. This throws on at least OSX for
// reasons that aren't fully understood yet. If more details/fixes emerge in that linked issue, we can
// ideally remove this and perform the attachment uniformly on all platforms.

// From: https://www.sqlite.org/lang_expr.html
//
// A string constant is formed by enclosing the string in single quotes ('). A single quote within the
// string can be encoded by putting two single quotes in a row - as in Pascal. C-style escapes using the
// backslash character are not supported because they are not standard SQL.
var attachString = RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
? $"attach database '{new Uri(databasePath).AbsoluteUri}?mode=memory&cache=shared' as {Database.WriteCache.GetName()};"
? $"attach database '{new Uri(databasePath.Replace("'", "''")).AbsoluteUri}?mode=memory&cache=shared' as {Database.WriteCache.GetName()};"
: $"attach database 'file::memory:?cache=shared' as {Database.WriteCache.GetName()};";

connection.ExecuteCommand(attachString);
Expand Down