search

microsoft / sarif-visualstudio-extension

SARIF Microsoft Visual Studio Viewer Extension
MIT License
46 stars 19 forks source link

Fix path resolution for relative SARIF artifact locations relative to %SRCROOT% #615

Closed chrishuynhc closed 7 months ago

chrishuynhc commented 7 months ago

Overview

Issue

This change addresses an issue found when trying to manually open a SARIF log containing a relative URI for artifactLocation such as:

"artifactLocation": {
     "uri": "Foo/Bar.cs",
     "uriBaseId": "%SRCROOT%"
}

in which the SARIF log was not successfully processed with the following error:

Processing the SARIF log file `<SARIF log path>`.
Failed to process log file. Reason: Invalid URI: The format of the URI could not be determined.
The SARIF log file `<SARIF log path>` processed.

The issue was caused by the path resolution mechanism returning a relative path as the "resolved path" which caused an exception to be thrown when trying to convert that relative resolved path to an absolute Uri (System.UriFormatException - Invalid URI: The format of the URI could not be determined).

To dive deeper, the reason the relative path was returned was because _fileSystem.FileExists was utilized to verify the resolved path existed on the file system. For more context, _fileSystem.FileExists is simply a wrapper around File.Exists. If File.Exists is passed a relative path, which is possible in this scenario, it is interpreted as relative to the current working directory and passes the check. As a result, the relative path is assigned as the "resolved path" where as we expect resolved path to be a full path. This relative resolved path then causes issues later in the flow.

Fix

The fix modifies src/Sarif.Viewer.VisualStudio.Core/CodeAnalysisResultManager.cs to return a fully resolved and normalized path by verifying that the path is rooted. If it is rooted, great, simply return it. If not, make sure to combine (Path.Combine) the relative path with the base path before returning.

The GetCommonSuffix utility, which is used later in the path resolution flow, was also modified to normalize both paths before doing a comparison. Although this might cause redundancy if a path is already normalized, it removes the responsibility from the caller to handle path normalization.