MSBuild log generated by StyleCop throws ArgumentOutOfRangeException

[brianfeucht]

The following Xml MSBuild log causes an System.ArgumentOutOfRangeException : startIndex cannot be larger than length of string.

<?xml version="1.0" encoding="utf-8"?>
<build started="3/11/2019 22:29:45">
  <warning code="SA1633" file="File.cs" line="1" column="1" started="3/11/2019 22:29:59"><![CDATA[The file header is missing or not located at the top of the file.]]></warning>
</build>

[brianfeucht]

Unit test to reproduce the error:
https://github.com/brianfeucht/Cake.Issues.MsBuild/commit/8bb38b6eeec95850b45349cbb3a180aa8402e991

[pascalberger]

@brianfeucht Thanks for the report and test case.

Can you create a PR with the test case. Then I can implement the fix for this on top of it on your branch (or you can of course also directly provide the implementation in the PR if you want)

[brianfeucht]

@pascalberger PR created.

I may get time to implement a fix today, but poking at it last night it wasn't 100% clear the best way to achieve that yet.

It is throwing due to the repository URL being longer than the local file path at

Cake.Issues.MsBuild/src/Cake.Issues.MsBuild/BaseMsBuildLogFileFormat.cs

Line 80 in 22adbbb

return filePath.Substring(repositorySettings.RepositoryRoot.FullPath.Length);

I think this code would need to check that the string starts with the repository url before doing the substring. However, we'd need to standardize the path separators before doing as one string has / and the other has \\

[pascalberger]

@brianfeucht On which OS are you running it?

In my environment test case works as expected:

1. It tries to read the file from the logfile:

   Cake.Issues.MsBuild/src/Cake.Issues.MsBuild/LogFileFormat/XmlFileLoggerLogFileFormat.cs

   Line 56 in 22adbbb

   if (!this.TryGetFile(warning, repositorySettings, out string fileName))

2. It won't find a parent compile task so it skips this part and file name is just File.cs:
   

   Cake.Issues.MsBuild/src/Cake.Issues.MsBuild/LogFileFormat/XmlFileLoggerLogFileFormat.cs

   Lines 211 to 216 in 22adbbb

   	var parentFileAttr = warning.Parent?.Attribute("file");
   	if (parentFileAttr != null)
   	{
   	var compileTaskDirectory = System.IO.Path.GetDirectoryName(parentFileAttr.Value);
   	fileName = System.IO.Path.Combine(compileTaskDirectory, fileName);
   	}

3. It will validate the file name:
   

   Cake.Issues.MsBuild/src/Cake.Issues.MsBuild/LogFileFormat/XmlFileLoggerLogFileFormat.cs

   Line 221 in 22adbbb

   (result, fileName) = this.ValidateFilePath(fileName, repositorySettings);

4. There it will see that file is not part of the repository and therefore exit and ignore this warning:
   

   Cake.Issues.MsBuild/src/Cake.Issues.MsBuild/BaseMsBuildLogFileFormat.cs

   Lines 31 to 34 in 22adbbb

   	if (!this.CheckIfFileIsInRepository(filePath, repositorySettings))
   	{
   	return (false, string.Empty);
   	}

The call to the MakeFilePathRelativeToRepositoryRoot method comes after the check if the file is part of the repository and is not called on my environment:

Cake.Issues.MsBuild/src/Cake.Issues.MsBuild/BaseMsBuildLogFileFormat.cs

Line 37 in 22adbbb

filePath = this.MakeFilePathRelativeToRepositoryRoot(filePath, repositorySettings);

[brianfeucht]

I am on Window 10. Running the test via Visual Studio 16.0.0.0 Preview 3 and DotCover

This also fails running via the CAKE script on Powershell using MSBuild

Doing a bit of debugging based on those details:

2. parentFileAttr is null
3. CheckIfFileIsInRepository is returning true:
   filePath.IsSubpathOf(repositorySettings.RepositoryRoot.FullPath) returns true
   "File.cs".IsSubpathOf("c:/Source/Cake.Issues.MsBuild") returns true

[pascalberger]

Interesting, CheckIfFileIsInRepository, or rather the IsSubpathOf call, should return false (and does on my machine).

[pascalberger]

On Azure Pipelines the test also fails because it doesn't return an issue and not with an ArgumentOutOfRangeException.

[pascalberger]

The only case I see when "File.cs".IsSubpathOf("c:/Source/Cake.Issues.MsBuild") returns true is when the current working directory is c:\Source\Cake.Issues.MsBuild. But I fail to see yet why this happens in your case.

[brianfeucht]

When I put a stop point at filePath.IsSubpathOf(repositorySettings.RepositoryRoot.FullPath) and run System.IO.Directory.GetCurrentDirectory() in the intermediate window, I get "C:\\source\\Cake.Issues.MsBuild\\src\\Cake.Issues.MsBuild.Tests\\bin\\Debug\\netcoreapp2.0" 🤔

[brianfeucht]

I've been able to track this down further. The issue is indeed related to the working directory.

https://github.com/cake-contrib/Cake.Issues/blob/76456cabb492c60be2932023c5c7c94af9412509/src/Cake.Issues/StringPathExtensions.cs#L71

This appears to prepend the CurrentWorking directory to the file name in the case of relative files. In my build environment this is set to the same as the Repository directory. If I call System.IO.Directory.SetCurrentDirectory to a directory outside my Repository I do not get an exception (giving me a work around for now)

I've updated my PR to hopefully make this reproduce for you.

[pascalberger]

Yeah, it's caused by the working directory since Path.GetFullPath is used.

The point which I don't understand is why you're running into this issue, or is c:\Source\Cake.Issues.MsBuild your actual path where you have cloned the repo to?

[brianfeucht]

Yes, I've cloned my repo to c:\Source\Cake.Issues.MsBuild

[pascalberger]

@brianfeucht I've updated #81 to contain a fix for the issue. Can you please test it and report back if it solves your issue.

If it works for you I'll merge the PR and create a release

[brianfeucht]

@pascalberger I have confirmed this change resolves the issue in my code base where I first ran into it. 🍻 thank you!

[pascalberger]

@brianfeucht Release 0.6.3 is now building and should be available soon on nuget.org

Thanks for reporting this and helping tracking it down. Really appreciated 👍