Skip to content

Commit 8cce810

Browse files
phil-allen-msftphil-allen-msft
authored
[release/dev17.14] Tooling: Don't throw exceptions when generating code for file rooted outside of project (#11868)
Backport of #11864 to release/dev17.14 ## Customer Impact ## Regression - [x] Yes - #11320 (This change inadvertently removed some exception swallowing that "hid" the problem from the user.) - [ ] No ## Testing - Manual testing of scenario with known repro project. The issue is present without the fix and resolved with the fix. - Tested mainline scenarios, such as OrchardCore and MudBlazor. These continue working with the fix. ## Risk Medium. This affects how every Razor file is compiled in Visual Studio at design-time. However, this risk is mitigated by testing large solutions such as OrchardCore and MudBlazor.
2 parents 3c473a9 + 6fdf93f commit 8cce810

File tree

11 files changed

+282
-20
lines changed

11 files changed

+282
-20
lines changed

‎src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorProjectFileSystem.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Diagnostics;
77
using System.IO;
8+
using Microsoft.AspNetCore.Razor.Utilities;
89

910
namespace Microsoft.AspNetCore.Razor.Language;
1011

@@ -93,6 +94,23 @@ protected override string NormalizeAndEnsureValidPath(string path)
9394

9495
var normalizedPath = path.Replace('\\', '/');
9596

97+
// On Windows, check to see if this is a rooted file path. If it is, just return it.
98+
// This covers the following cases:
99+
//
100+
// 1. It is rooted within the project root. That's valid and we would have checked
101+
// specifically for that case below.
102+
// 2. It is rooted outside of the project root. That's invalid, and we don't want to
103+
// concatenate it with the project root. That would potentially produce an invalid
104+
// Windows path like 'C:/project/C:/other-project/some-file.cshtml'.
105+
//
106+
// Note that returning a path that is rooted outside of the project root will cause
107+
// the GetItem(...) method to throw, but it could be overridden by a descendant file
108+
// system.
109+
if (PlatformInformation.IsWindows && PathUtilities.IsPathFullyQualified(path))
110+
{
111+
return normalizedPath;
112+
}
113+
96114
// Check if the given path is an absolute path. It is absolute if...
97115
//
98116
// 1. It is a network share path and starts with a '//' (e.g. //server/some/network/folder) or...

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/CompilationHelpers.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ internal static async Task<RazorCodeDocument> GenerateCodeDocumentAsync(
2020
{
2121
var importSources = await GetImportSourcesAsync(document, projectEngine, cancellationToken).ConfigureAwait(false);
2222
var tagHelpers = await document.Project.GetTagHelpersAsync(cancellationToken).ConfigureAwait(false);
23-
var source = await document.GetSourceAsync(projectEngine, cancellationToken).ConfigureAwait(false);
23+
var source = await document.GetSourceAsync(cancellationToken).ConfigureAwait(false);
2424

2525
var generator = new CodeDocumentGenerator(projectEngine, compilerOptions);
2626
return generator.Generate(source, document.FileKind, importSources, tagHelpers, cancellationToken);
@@ -33,15 +33,18 @@ internal static async Task<RazorCodeDocument> GenerateDesignTimeCodeDocumentAsyn
3333
{
3434
var importSources = await GetImportSourcesAsync(document, projectEngine, cancellationToken).ConfigureAwait(false);
3535
var tagHelpers = await document.Project.GetTagHelpersAsync(cancellationToken).ConfigureAwait(false);
36-
var source = await document.GetSourceAsync(projectEngine, cancellationToken).ConfigureAwait(false);
36+
var source = await document.GetSourceAsync(cancellationToken).ConfigureAwait(false);
3737

3838
var generator = new CodeDocumentGenerator(projectEngine, RazorCompilerOptions.None);
3939
return generator.GenerateDesignTime(source, document.FileKind, importSources, tagHelpers, cancellationToken);
4040
}
4141

4242
private static async Task<ImmutableArray<RazorSourceDocument>> GetImportSourcesAsync(IDocumentSnapshot document, RazorProjectEngine projectEngine, CancellationToken cancellationToken)
4343
{
44-
var projectItem = projectEngine.FileSystem.GetItem(document.FilePath, document.FileKind);
44+
// We don't use document.FilePath when calling into GetItem(...) because
45+
// it could be rooted outside of the project root. document.TargetPath should
46+
// represent the logical relative path within the project root.
47+
var projectItem = projectEngine.FileSystem.GetItem(document.TargetPath, document.FileKind);
4548

4649
using var importProjectItems = new PooledArrayBuilder<RazorProjectItem>();
4750
projectEngine.CollectImports(projectItem, ref importProjectItems.AsRef());

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IDocumentSnapshotExtensions.cs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,12 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem;
1111

1212
internal static class IDocumentSnapshotExtensions
1313
{
14-
public static async Task<RazorSourceDocument> GetSourceAsync(this IDocumentSnapshot document, RazorProjectEngine projectEngine, CancellationToken cancellationToken)
14+
public static async Task<RazorSourceDocument> GetSourceAsync(
15+
this IDocumentSnapshot document,
16+
CancellationToken cancellationToken)
1517
{
16-
var projectItem = document is { FilePath: string filePath, FileKind: var fileKind }
17-
? projectEngine.FileSystem.GetItem(filePath, fileKind)
18-
: null;
19-
2018
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
21-
var properties = RazorSourceDocumentProperties.Create(document.FilePath, projectItem?.RelativePhysicalPath);
19+
var properties = RazorSourceDocumentProperties.Create(document.FilePath, document.TargetPath);
2220
return RazorSourceDocument.Create(text, properties);
2321
}
2422

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/Legacy/ILegacyDocumentSnapshot.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,6 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem.Legacy;
1111
/// </remarks>
1212
internal interface ILegacyDocumentSnapshot
1313
{
14+
string TargetPath { get; }
1415
string FileKind { get; }
1516
}

‎src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/Discovery/ProjectStateUpdater.cs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,26 @@ private void ReleaseSemaphore(ProjectKey projectKey)
311311
.ConfigureAwait(false);
312312
watch.Stop();
313313

314-
// don't report success if the work was cancelled
314+
// Don't report success if the work was cancelled
315315
cancellationToken.ThrowIfCancellationRequested();
316316

317+
// Don't report success if the call failed.
318+
// If the ImmutableArray that was returned is default, then the call failed.
319+
320+
if (tagHelpers.IsDefault)
321+
{
322+
_telemetryReporter.ReportEvent("taghelperresolve/end", Severity.Normal,
323+
new("id", telemetryId),
324+
new("result", "error"));
325+
326+
_logger.LogError($"""
327+
Tag helper discovery failed.
328+
Project: {projectSnapshot.FilePath}
329+
""");
330+
331+
return (null, configuration);
332+
}
333+
317334
_telemetryReporter.ReportEvent("taghelperresolve/end", Severity.Normal,
318335
new("id", telemetryId),
319336
new("ellapsedms", watch.ElapsedMilliseconds),

‎src/Razor/src/Microsoft.VisualStudio.LegacyEditor.Razor/ImportDocumentManager.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,16 @@ public void OnUnsubscribed(IVisualStudioDocumentTracker documentTracker)
9797
private static ImmutableArray<RazorProjectItem> GetPhysicalImportItems(string filePath, ILegacyProjectSnapshot projectSnapshot)
9898
{
9999
var projectEngine = projectSnapshot.GetProjectEngine();
100-
var documentSnapshot = projectSnapshot.GetDocument(filePath);
101-
var projectItem = projectEngine.FileSystem.GetItem(filePath, documentSnapshot?.FileKind);
102100

103-
return projectEngine.GetImports(projectItem, static i => i.PhysicalPath is not null);
101+
// If we can get the document, use it's target path to find the project item
102+
// to avoid GetItem(...) throwing an exception if the file path is rooted outside
103+
// of the project. If we can't get the document, we'll just go ahead and use
104+
// the file path, since it's probably OK.
105+
var projectItem = projectSnapshot.GetDocument(filePath) is { } document
106+
? projectEngine.FileSystem.GetItem(document.TargetPath, document.FileKind)
107+
: projectEngine.FileSystem.GetItem(filePath, fileKind: null);
108+
109+
return projectEngine.GetImports(projectItem, static i => i is not DefaultImportProjectItem);
104110
}
105111

106112
private void FileChangeTracker_Changed(object sender, FileChangeEventArgs args)

‎src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/TestRazorProjectService.cs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
using Microsoft.AspNetCore.Razor.Language;
77
using Microsoft.AspNetCore.Razor.LanguageServer.Common;
88
using Microsoft.AspNetCore.Razor.Test.Common;
9+
using Microsoft.CodeAnalysis.Razor;
910
using Microsoft.CodeAnalysis.Razor.Logging;
1011
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
1112
using Microsoft.CodeAnalysis.Razor.Serialization;
13+
using Microsoft.CodeAnalysis.Razor.Utilities;
1214
using Moq;
1315

1416
namespace Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
@@ -37,13 +39,19 @@ private static IRazorProjectInfoDriver CreateProjectInfoDriver()
3739
return mock.Object;
3840
}
3941

40-
public async Task AddDocumentToPotentialProjectsAsync(string textDocumentPath, CancellationToken cancellationToken)
42+
public async Task AddDocumentToPotentialProjectsAsync(string filePath, CancellationToken cancellationToken)
4143
{
42-
var document = new DocumentSnapshotHandle(
43-
textDocumentPath, textDocumentPath, FileKinds.GetFileKindFromFilePath(textDocumentPath));
44-
45-
foreach (var projectSnapshot in _projectManager.FindPotentialProjects(textDocumentPath))
44+
foreach (var projectSnapshot in _projectManager.FindPotentialProjects(filePath))
4645
{
46+
var projectDirectory = FilePathNormalizer.GetNormalizedDirectoryName(projectSnapshot.FilePath);
47+
var normalizedFilePath = FilePathNormalizer.Normalize(filePath);
48+
49+
var targetPath = normalizedFilePath.StartsWith(projectDirectory, FilePathComparison.Instance)
50+
? normalizedFilePath[projectDirectory.Length..]
51+
: normalizedFilePath;
52+
53+
var document = new DocumentSnapshotHandle(filePath, targetPath, FileKinds.GetFileKindFromFilePath(filePath));
54+
4755
var projectInfo = projectSnapshot.ToRazorProjectInfo();
4856

4957
projectInfo = projectInfo with

‎src/Shared/Microsoft.AspNetCore.Razor.Test.Common/ConditionalFactAttribute.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ public static class Is
121121
/// </summary>
122122
public const string FreeBSD = nameof(FreeBSD);
123123

124+
/// <summary>
125+
/// Only execute if the current operating system platform is Unix-based.
126+
/// </summary>
127+
public const string AnyUnix = nameof(AnyUnix);
128+
124129
public static class Not
125130
{
126131
/// <summary>
@@ -142,6 +147,12 @@ public static class Not
142147
/// Only execute if the current operating system platform is not FreeBSD.
143148
/// </summary>
144149
public const string FreeBSD = $"!{nameof(FreeBSD)}";
150+
151+
/// <summary>
152+
/// Only execute if the current operating system platform is not Unix-based.
153+
/// </summary>
154+
public const string AnyUnix = $"!{nameof(AnyUnix)}";
155+
145156
}
146157
}
147158

@@ -157,6 +168,9 @@ private static FrozenDictionary<string, Func<bool>> CreateConditionMap()
157168
Add(Is.Linux, static () => PlatformInformation.IsLinux);
158169
Add(Is.MacOS, static () => PlatformInformation.IsMacOS);
159170
Add(Is.FreeBSD, static () => PlatformInformation.IsFreeBSD);
171+
Add(Is.AnyUnix, static () => PlatformInformation.IsLinux ||
172+
PlatformInformation.IsMacOS ||
173+
PlatformInformation.IsFreeBSD);
160174

161175
return map.ToFrozenDictionary();
162176

‎src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/PathUtilitiesTests.cs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,84 @@ public void GetExtension_Span(string path, string expected)
4242
Assert.Equal(!string.IsNullOrEmpty(expected), PathUtilities.HasExtension(path.AsSpan()));
4343
}
4444

45+
// The tests below are derived from the .NET Runtime:
46+
// - https://github.com/dotnet/runtime/blob/91195a7948a16c769ccaf7fd8ca84b1d210f6841/src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/IO/Path.IsPathFullyQualified.cs
47+
48+
[Fact]
49+
public static void IsPathFullyQualified_NullArgument()
50+
{
51+
Assert.Throws<ArgumentNullException>(() => PathUtilities.IsPathFullyQualified(null!));
52+
}
53+
54+
[Fact]
55+
public static void IsPathFullyQualified_Empty()
56+
{
57+
Assert.False(PathUtilities.IsPathFullyQualified(""));
58+
Assert.False(PathUtilities.IsPathFullyQualified(ReadOnlySpan<char>.Empty));
59+
}
60+
61+
[ConditionalTheory(Is.Windows)]
62+
[InlineData("/")]
63+
[InlineData(@"\")]
64+
[InlineData(".")]
65+
[InlineData("C:")]
66+
[InlineData("C:foo.txt")]
67+
public static void IsPathFullyQualified_Windows_Invalid(string path)
68+
{
69+
Assert.False(PathUtilities.IsPathFullyQualified(path));
70+
Assert.False(PathUtilities.IsPathFullyQualified(path.AsSpan()));
71+
}
72+
73+
[ConditionalTheory(Is.Windows)]
74+
[InlineData(@"\\")]
75+
[InlineData(@"\\\")]
76+
[InlineData(@"\\Server")]
77+
[InlineData(@"\\Server\Foo.txt")]
78+
[InlineData(@"\\Server\Share\Foo.txt")]
79+
[InlineData(@"\\Server\Share\Test\Foo.txt")]
80+
[InlineData(@"C:\")]
81+
[InlineData(@"C:\foo1")]
82+
[InlineData(@"C:\\")]
83+
[InlineData(@"C:\\foo2")]
84+
[InlineData(@"C:/")]
85+
[InlineData(@"C:/foo1")]
86+
[InlineData(@"C://")]
87+
[InlineData(@"C://foo2")]
88+
public static void IsPathFullyQualified_Windows_Valid(string path)
89+
{
90+
Assert.True(PathUtilities.IsPathFullyQualified(path));
91+
Assert.True(PathUtilities.IsPathFullyQualified(path.AsSpan()));
92+
}
93+
94+
[ConditionalTheory(Is.AnyUnix)]
95+
[InlineData(@"\")]
96+
[InlineData(@"\\")]
97+
[InlineData(".")]
98+
[InlineData("./foo.txt")]
99+
[InlineData("..")]
100+
[InlineData("../foo.txt")]
101+
[InlineData(@"C:")]
102+
[InlineData(@"C:/")]
103+
[InlineData(@"C://")]
104+
public static void IsPathFullyQualified_Unix_Invalid(string path)
105+
{
106+
Assert.False(PathUtilities.IsPathFullyQualified(path));
107+
Assert.False(PathUtilities.IsPathFullyQualified(path.AsSpan()));
108+
}
109+
110+
[ConditionalTheory(Is.AnyUnix)]
111+
[InlineData("/")]
112+
[InlineData("/foo.txt")]
113+
[InlineData("/..")]
114+
[InlineData("//")]
115+
[InlineData("//foo.txt")]
116+
[InlineData("//..")]
117+
public static void IsPathFullyQualified_Unix_Valid(string path)
118+
{
119+
Assert.True(PathUtilities.IsPathFullyQualified(path));
120+
Assert.True(PathUtilities.IsPathFullyQualified(path.AsSpan()));
121+
}
122+
45123
private static void AssertEqual(ReadOnlySpan<char> expected, ReadOnlySpan<char> actual)
46124
{
47125
if (!actual.SequenceEqual(expected))

‎src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/PathUtilities.cs

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public static ReadOnlySpan<char> GetExtension(ReadOnlySpan<char> path)
3636
{
3737
return i != length - 1
3838
? path[i..length]
39-
: [];
39+
: [];
4040
}
4141

4242
if (IsDirectorySeparator(ch))
@@ -70,5 +70,95 @@ public static bool HasExtension(ReadOnlySpan<char> path)
7070
private static bool IsDirectorySeparator(char ch)
7171
=> ch == Path.DirectorySeparatorChar ||
7272
(PlatformInformation.IsWindows && ch == Path.AltDirectorySeparatorChar);
73+
74+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
75+
private static bool IsValidDriveChar(char value)
76+
=> (uint)((value | 0x20) - 'a') <= (uint)('z' - 'a');
77+
#endif
78+
79+
public static bool IsPathFullyQualified(string path)
80+
{
81+
ArgHelper.ThrowIfNull(path);
82+
83+
return IsPathFullyQualified(path.AsSpan());
84+
}
85+
86+
public static bool IsPathFullyQualified(ReadOnlySpan<char> path)
87+
{
88+
#if NET
89+
return Path.IsPathFullyQualified(path);
90+
#else
91+
if (PlatformInformation.IsWindows)
92+
{
93+
// Derived from .NET Runtime:
94+
// - https://github.com/dotnet/runtime/blob/c7c961a330395152e5ec4000032cd3204ceb4a10/src/libraries/Common/src/System/IO/PathInternal.Windows.cs#L250-L274
95+
96+
if (path.Length < 2)
97+
{
98+
// It isn't fixed, it must be relative. There is no way to specify a fixed
99+
// path with one character (or less).
100+
return false;
101+
}
102+
103+
if (IsDirectorySeparator(path[0]))
104+
{
105+
// There is no valid way to specify a relative path with two initial slashes or
106+
// \? as ? isn't valid for drive relative paths and \??\ is equivalent to \\?\
107+
return path[1] == '?' || IsDirectorySeparator(path[1]);
108+
}
109+
110+
// The only way to specify a fixed path that doesn't begin with two slashes
111+
// is the drive, colon, slash format- i.e. C:\
112+
return (path.Length >= 3)
113+
&& (path[1] == Path.VolumeSeparatorChar)
114+
&& IsDirectorySeparator(path[2])
115+
// To match old behavior we'll check the drive character for validity as the path is technically
116+
// not qualified if you don't have a valid drive. "=:\" is the "=" file's default data stream.
117+
&& IsValidDriveChar(path[0]);
118+
}
119+
else
120+
{
121+
// Derived from .NET Runtime:
122+
// - https://github.com/dotnet/runtime/blob/c7c961a330395152e5ec4000032cd3204ceb4a10/src/libraries/Common/src/System/IO/PathInternal.Unix.cs#L77-L82
123+
124+
// This is much simpler than Windows where paths can be rooted, but not fully qualified (such as Drive Relative)
125+
// As long as the path is rooted in Unix it doesn't use the current directory and therefore is fully qualified.
126+
return IsPathRooted(path);
127+
}
128+
#endif
129+
}
130+
131+
public static bool IsPathRooted(string path)
132+
{
133+
#if NET
134+
return Path.IsPathRooted(path);
135+
#else
136+
return IsPathRooted(path.AsSpan());
137+
#endif
138+
}
139+
140+
public static bool IsPathRooted(ReadOnlySpan<char> path)
141+
{
142+
#if NET
143+
return Path.IsPathRooted(path);
144+
145+
#else
146+
if (PlatformInformation.IsWindows)
147+
{
148+
// Derived from .NET Runtime
149+
// - https://github.com/dotnet/runtime/blob/850c0ab4519b904a28f2d67abdaba1ac78c955ff/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs#L271-L276
150+
151+
var length = path.Length;
152+
return (length >= 1 && IsDirectorySeparator(path[0]))
153+
|| (length >= 2 && IsValidDriveChar(path[0]) && path[1] == Path.VolumeSeparatorChar);
154+
}
155+
else
156+
{
157+
// Derived from .NET Runtime
158+
// - https://github.com/dotnet/runtime/blob/850c0ab4519b904a28f2d67abdaba1ac78c955ff/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs#L132-L135
159+
160+
return path.StartsWith(Path.DirectorySeparatorChar);
161+
}
73162
#endif
74163
}
164+
}

0 commit comments

Comments
 (0)