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.

Sign up for GitHub

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

Jump to bottom

Filter framework-mismatched assemblies from NUGET_PLUGIN_PATHS #2859

Closed
wants to merge 1 commit into from
Closed

Filter framework-mismatched assemblies from NUGET_PLUGIN_PATHS #2859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoverer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,10 @@ private IEnumerable<string> GetPluginFilePaths()
return PluginDiscoveryUtility.GetConventionBasedPlugins(directories);
}

return _rawPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries);
// even if there are no relevant plugins, we will not fall back to built-ins
// to maintain some typo-guard consistency with the previous no-filter flow
return _rawPluginPaths.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries)
.Where(PluginDiscoveryUtility.IsPluginRelevant);
}
}
}
9 changes: 9 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Plugins/PluginDiscoveryUtility.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,5 +95,14 @@ public static IEnumerable<string> GetConventionBasedPlugins(IEnumerable<string>
}
return paths;
}

public static bool IsPluginRelevant(string path)
{
#if IS_DESKTOP
return path.EndsWith(".exe", StringComparison.OrdinalIgnoreCase);
#else
return path.EndsWith(".dll", StringComparison.OrdinalIgnoreCase);
#endif
}
}
}
53 changes: 45 additions & 8 deletions test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/PluginDiscovererTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ namespace NuGet.Protocol.Plugins.Tests
{
public class PluginDiscovererTests
{
#if IS_DESKTOP
private const string _extension = ".exe";
#else
private const string _extension = ".dll";
#endif

[Theory]
[InlineData(null)]
[InlineData("")]
Expand Down Expand Up @@ -52,7 +58,7 @@ public async Task DiscoverAsync_ThrowsPlatformNotSupportedIfEmbeddedSignatureVer
{
using (var testDirectory = TestDirectory.Create())
{
var pluginPath = Path.Combine(testDirectory.Path, "a");
var pluginPath = Path.Combine(testDirectory.Path, $"a{_extension}");

File.WriteAllText(pluginPath, string.Empty);

Expand Down Expand Up @@ -83,7 +89,7 @@ public async Task DiscoverAsync_PerformsDiscoveryOnlyOnce()
{
using (var testDirectory = TestDirectory.Create())
{
var pluginPath = Path.Combine(testDirectory.Path, "a");
var pluginPath = Path.Combine(testDirectory.Path, $"a{_extension}");

File.WriteAllText(pluginPath, string.Empty);

Expand Down Expand Up @@ -118,7 +124,7 @@ public async Task DiscoverAsync_SignatureIsVerifiedLazily()
{
using (var testDirectory = TestDirectory.Create())
{
var pluginPath = Path.Combine(testDirectory.Path, "a");
var pluginPath = Path.Combine(testDirectory.Path, $"a{_extension}");

File.WriteAllText(pluginPath, string.Empty);

Expand Down Expand Up @@ -148,19 +154,20 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates()
using (var testDirectory = TestDirectory.Create())
{
var pluginPaths = new[] { "a", "b", "c", "d" }
.Select(fileName => Path.Combine(testDirectory.Path, fileName))
.Select(fileName => Path.Combine(testDirectory.Path, fileName+_extension))
.ToArray();

File.WriteAllText(pluginPaths[1], string.Empty);
File.WriteAllText(pluginPaths[2], string.Empty);

var pluginE = $"e{_extension}";
var responses = new Dictionary<string, bool>()
{
{ pluginPaths[0], false },
{ pluginPaths[1], false },
{ pluginPaths[2], true },
{ pluginPaths[3], false },
{ "e", true }
{ pluginE, true }
};
var verifierStub = new EmbeddedSignatureVerifierStub(responses);
var rawPluginPaths = string.Join(";", responses.Keys);
Expand All @@ -187,9 +194,9 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates()
Assert.Equal(PluginFileState.NotFound, results[3].PluginFile.State.Value);
Assert.Equal($"A plugin was not found at path '{pluginPaths[3]}'.", results[3].Message);

Assert.Equal("e", results[4].PluginFile.Path);
Assert.Equal(pluginE, results[4].PluginFile.Path);
Assert.Equal(PluginFileState.InvalidFilePath, results[4].PluginFile.State.Value);
Assert.Equal($"The plugin file path 'e' is invalid.", results[4].Message);
Assert.Equal($"The plugin file path '{pluginE}' is invalid.", results[4].Message);
}
}
}
Expand All @@ -201,6 +208,7 @@ public async Task DiscoverAsync_HandlesAllPluginFileStates()
[InlineData(@"..\a")]
public async Task DiscoverAsync_DisallowsNonRootedFilePaths(string pluginPath)
{
pluginPath += _extension;
var responses = new Dictionary<string, bool>() { { pluginPath, true } };
var verifierStub = new EmbeddedSignatureVerifierStub(responses);

Expand All @@ -214,12 +222,41 @@ public async Task DiscoverAsync_DisallowsNonRootedFilePaths(string pluginPath)
}
}

[Fact]
public async Task DiscoverAsync_RemovesIrrelevantPluginsForFramework()
{
using (var testDirectory = TestDirectory.Create())
{
var pluginPaths = new[] { $"a{_extension}", "b.xxx", $"c{_extension}" }
.Select(fileName => Path.Combine(testDirectory.Path, fileName))
.ToArray();

var responses = new Dictionary<string, bool>();
foreach (var path in pluginPaths)
{
File.WriteAllText(path, string.Empty);
responses[path] = true;
}

var verifierStub = new EmbeddedSignatureVerifierStub(responses);
var pluginPath = string.Join(";", pluginPaths);
using (var discoverer = new PluginDiscoverer(pluginPath, verifierStub))
{
var results = (await discoverer.DiscoverAsync(CancellationToken.None)).ToArray();

Assert.Equal(2, results.Length);
Assert.Equal(pluginPaths[0], results[0].PluginFile.Path);
Assert.Equal(pluginPaths[2], results[1].PluginFile.Path);
}
}
}

[Fact]
public async Task DiscoverAsync_IsIdempotent()
{
using (var testDirectory = TestDirectory.Create())
{
var pluginPath = Path.Combine(testDirectory.Path, "a");
var pluginPath = Path.Combine(testDirectory.Path, $"a{_extension}");

File.WriteAllText(pluginPath, string.Empty);

Expand Down