microsoft / microsoft/vs-solutionpersistence

SolutionModel incorrectly allows duplicated projects that differ on directory separator characters

Open
#134 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
213
Forks
14
PR merge metrics
No merged PRs in 30d

Description

Run the following program:

using System.Text;
using Microsoft.VisualStudio.SolutionPersistence.Model;
using Microsoft.VisualStudio.SolutionPersistence.Serializer;

var solutionModel = new SolutionModel();
solutionModel.AddProject(@"Core\Lib1.csproj");
solutionModel.AddProject(@"Core/Lib1.csproj");

Console.WriteLine("After adding a project:");
Console.WriteLine("Projects: " + string.Join(", ", solutionModel.SolutionProjects.Select(x => x.FilePath)));

using var memoryStream = new MemoryStream();
await SolutionSerializers.SlnXml.SaveAsync(memoryStream, solutionModel, CancellationToken.None);
memoryStream.Position = 0;

solutionModel = await SolutionSerializers.SlnXml.OpenAsync(memoryStream, CancellationToken.None);

Console.WriteLine("After saving and re-opening the solution:");
Console.WriteLine("Projects: " + string.Join(", ", solutionModel.SolutionProjects.Select(x => x.FilePath)));

Actual result:

After adding a project:
Projects: Core\Lib1.csproj, Core/Lib1.csproj
Unhandled exception. Microsoft.VisualStudio.SolutionPersistence.Model.SolutionException: Duplicate item 'Core/Lib1.csproj' of type 'Project'. (Parameter 'value')

You can add projects that share the same path that only differ on directory separator characters and solution model will see them as different projects.
However, the model just adds two projects that are effectively the same until you attempt to serialize and deserialize it back - then it normalizes the paths and finally notices the duplication

Expected result:
Either, the model rejects project paths with \ like it does for solution folders,
or the model automatically normalizes the separators

Notes:
The current behavior basically makes it users' responsibility to conform to the undocumented standard path format - I've only found this documentation on an internal static method that acknowledges that the model expects / directory separators.

https://github.com/microsoft/vs-solutionpersistence/blob/a5c8b172f89d74a447648518b16e02978c79c8a3/src/Microsoft.VisualStudio.SolutionPersistence/Utilities/PathExtensions.cs#L10-L20

This normalization also depends on PathExtensions.IsWindows which leads to solution file being inconsistently serialized on different OSs - the following code has different output based on which OS is running it:

using System.Text;
using Microsoft.VisualStudio.SolutionPersistence.Model;
using Microsoft.VisualStudio.SolutionPersistence.Serializer;

var solutionModel = new SolutionModel();
solutionModel.AddProject(@"Core\ConsoleApp.csproj");

using var memoryStream = new MemoryStream();
await SolutionSerializers.SlnXml.SaveAsync(memoryStream, solutionModel, CancellationToken.None);
memoryStream.Position = 0;
using var reader = new StreamReader(memoryStream, new UTF8Encoding(false), leaveOpen: true);

Console.WriteLine(reader.ReadToEnd());

On macOS:

<Solution>
  <Project Path="Core\ConsoleApp.csproj" />
</Solution>

On Windows:

<Solution>
  <Project Path="Core/ConsoleApp.csproj" />
</Solution>

Note that the directory separator got normalized to / on Windows but not on macOS - unless you normalize the paths yourself before adding a project to the solution model, the solution file will depend on the OS of the user

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/Microsoft.VisualStudio.SolutionPersistence/Utilities/PathExtensions.cs, especially the normalization behavior referenced in the issue, then trace SolutionModel.AddProject and the SlnXml serializer. Compare project-path handling on Windows and macOS and verify that adding separator variants cannot produce a model that fails during save or reload.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.