Skip to content

Replace usages of Assembly.CodeBase with Assembly.Location #22258

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

Merged
merged 1 commit into from
May 27, 2020
Merged

Replace usages of Assembly.CodeBase with Assembly.Location #22258

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
32 changes: 12 additions & 20 deletions src/Hosting/Hosting/src/StaticWebAssets/StaticWebAssetsLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
#nullable enable
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using Microsoft.AspNetCore.Hosting;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.FileProviders;

Expand All @@ -26,12 +25,10 @@ public class StaticWebAssetsLoader
/// <param name="configuration">The host <see cref="IConfiguration"/>.</param>
public static void UseStaticWebAssets(IWebHostEnvironment environment, IConfiguration configuration)
{
using (var manifest = ResolveManifest(environment, configuration))
using var manifest = ResolveManifest(environment, configuration);
if (manifest != null)
{
if (manifest != null)
{
UseStaticWebAssetsCore(environment, manifest);
}
UseStaticWebAssetsCore(environment, manifest);
}
}

Expand All @@ -56,14 +53,14 @@ internal static void UseStaticWebAssetsCore(IWebHostEnvironment environment, Str
}
}

internal static Stream ResolveManifest(IWebHostEnvironment environment, IConfiguration configuration)
internal static Stream? ResolveManifest(IWebHostEnvironment environment, IConfiguration configuration)
{
try
{
var manifestPath = configuration.GetValue<string>(WebHostDefaults.StaticWebAssetsKey);
var filePath = manifestPath ?? ResolveRelativeToAssembly(environment);
if (File.Exists(filePath))

if (filePath != null && File.Exists(filePath))
{
return File.OpenRead(filePath);
}
Expand All @@ -81,21 +78,16 @@ internal static Stream ResolveManifest(IWebHostEnvironment environment, IConfigu
}
}

private static string ResolveRelativeToAssembly(IWebHostEnvironment environment)
private static string? ResolveRelativeToAssembly(IWebHostEnvironment environment)
{
var assembly = Assembly.Load(environment.ApplicationName);
return Path.Combine(Path.GetDirectoryName(GetAssemblyLocation(assembly)), $"{environment.ApplicationName}.StaticWebAssets.xml");
}

internal static string GetAssemblyLocation(Assembly assembly)
{
if (Uri.TryCreate(assembly.CodeBase, UriKind.Absolute, out var result) &&
result.IsFile && string.IsNullOrWhiteSpace(result.Fragment))
if (string.IsNullOrEmpty(assembly.Location))
{
return result.LocalPath;
return null;
}

return assembly.Location;
return Path.Combine(Path.GetDirectoryName(assembly.Location)!, $"{environment.ApplicationName}.StaticWebAssets.xml");
}
}
}
#nullable restore
6 changes: 3 additions & 3 deletions src/Hosting/Hosting/test/StaticWebAssets/StaticWebAssetsFileProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public void GetFileInfo_DoesNotMatch_IncompletePrefixSegments()
var expectedResult = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
var provider = new StaticWebAssetsFileProvider(
"_cont",
Path.GetDirectoryName(new Uri(typeof(StaticWebAssetsFileProviderTests).Assembly.CodeBase).LocalPath));
Path.GetDirectoryName(typeof(StaticWebAssetsFileProviderTests).Assembly.Location));

// Act
var file = provider.GetFileInfo("/_content/Microsoft.AspNetCore.TestHost.StaticWebAssets.xml");
Expand All @@ -169,7 +169,7 @@ public void GetFileInfo_Prefix_RespectsOsCaseSensitivity()
var expectedResult = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
var provider = new StaticWebAssetsFileProvider(
"_content",
Path.GetDirectoryName(new Uri(typeof(StaticWebAssetsFileProviderTests).Assembly.CodeBase).LocalPath));
Path.GetDirectoryName(typeof(StaticWebAssetsFileProviderTests).Assembly.Location));

// Act
var file = provider.GetFileInfo("/_CONTENT/Microsoft.AspNetCore.Hosting.StaticWebAssets.xml");
Expand All @@ -185,7 +185,7 @@ public void GetDirectoryContents_Prefix_RespectsOsCaseSensitivity()
var expectedResult = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
var provider = new StaticWebAssetsFileProvider(
"_content",
Path.GetDirectoryName(new Uri(typeof(StaticWebAssetsFileProviderTests).Assembly.CodeBase).LocalPath));
Path.GetDirectoryName(typeof(StaticWebAssetsFileProviderTests).Assembly.Location));

// Act
var directory = provider.GetDirectoryContents("/_CONTENT");
Expand Down
2 changes: 1 addition & 1 deletion src/Hosting/Hosting/test/StaticWebAssets/StaticWebAssetsLoaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void ResolveManifest_UsesConfigurationKey_WhenProvided()
<ContentRoot Path=""/Path"" BasePath=""/BasePath"" />
</StaticWebAssets>
";
var path = Path.ChangeExtension(new Uri(typeof(StaticWebAssetsLoader).Assembly.CodeBase).LocalPath, ".StaticWebAssets.xml");
var path = Path.ChangeExtension(typeof(StaticWebAssetsLoader).Assembly.Location, ".StaticWebAssets.xml");
var environment = new HostingEnvironment()
{
ApplicationName = "NonExistingDll"
Expand Down
2 changes: 1 addition & 1 deletion src/Identity/test/Identity.FunctionalTests/Infrastructure/ServerFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private void UpdateApplicationParts(IWebHostBuilder builder) =>

private void UpdateStaticAssets(IWebHostBuilder builder)
{
var manifestPath = Path.GetDirectoryName(new Uri(typeof(ServerFactory<,>).Assembly.CodeBase).LocalPath);
var manifestPath = Path.GetDirectoryName(typeof(ServerFactory<,>).Assembly.Location);
builder.ConfigureAppConfiguration((ctx, cb) =>
{
if (ctx.HostingEnvironment.WebRootFileProvider is CompositeFileProvider composite)
Expand Down
15 changes: 2 additions & 13 deletions src/Mvc/Mvc.Core/src/ApplicationParts/RelatedAssemblyAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ internal static IReadOnlyList<Assembly> GetRelatedAssemblies(

// MVC will specifically look for related parts in the same physical directory as the assembly.
// No-op if the assembly does not have a location.
if (assembly.IsDynamic || string.IsNullOrEmpty(assembly.CodeBase))
if (assembly.IsDynamic || string.IsNullOrEmpty(assembly.Location))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right? Will this not fail with the new publish single file work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure how that works, but we should tackle that separately. This doesn't make the code more incorrect than before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair, can we file an issue to track that work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why check IsDynamic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had issues where users had controllers in dynamically generated assemblies.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what does that check have to do with anything here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The way you add the controller assembly is by registering it as an application part.
  • MVC will attempt to look up related parts by using the assembly's location.
  • Accessing the location property on a dynamically generated assembly throws.

If the stars align, MVC will throw. Does that help?

{
return Array.Empty<Assembly>();
}
Expand All @@ -78,7 +78,7 @@ internal static IReadOnlyList<Assembly> GetRelatedAssemblies(
}

var assemblyName = assembly.GetName().Name;
var assemblyLocation = GetAssemblyLocation(assembly);
var assemblyLocation = assembly.Location;
var assemblyDirectory = Path.GetDirectoryName(assemblyLocation);

var relatedAssemblies = new List<Assembly>();
Expand Down Expand Up @@ -112,16 +112,5 @@ internal static IReadOnlyList<Assembly> GetRelatedAssemblies(

return relatedAssemblies;
}

internal static string GetAssemblyLocation(Assembly assembly)
{
if (Uri.TryCreate(assembly.CodeBase, UriKind.Absolute, out var result) &&
result.IsFile && string.IsNullOrWhiteSpace(result.Fragment))
{
return result.LocalPath;
}

return assembly.Location;
}
}
}
86 changes: 0 additions & 86 deletions src/Mvc/Mvc.Core/test/ApplicationParts/RelatedAssemblyPartTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,88 +75,6 @@ public void GetRelatedAssemblies_LoadsRelatedAssembly()
Assert.Equal(new[] { relatedAssembly }, result);
}

[Fact]
public void GetAssemblyLocation_UsesCodeBase()
{
// Arrange
var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll");
var codeBase = "file://x:/file/Assembly.dll";
var expected = new Uri(codeBase).LocalPath;
var assembly = new TestAssembly
{
CodeBaseSettable = codeBase,
};

// Act
var actual = RelatedAssemblyAttribute.GetAssemblyLocation(assembly);
Assert.Equal(expected, actual);
}

[Fact]
public void GetAssemblyLocation_UsesLocation_IfCodeBaseIsNotLocal()
{
// Arrange
var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll");
var expected = Path.Combine(AssemblyDirectory, "Some-Dir", "Assembly.dll");
var assembly = new TestAssembly
{
CodeBaseSettable = "https://www.microsoft.com/test.dll",
LocationSettable = expected,
};

// Act
var actual = RelatedAssemblyAttribute.GetAssemblyLocation(assembly);
Assert.Equal(expected, actual);
}

[Fact]
public void GetAssemblyLocation_CodeBase_HasPoundCharacterUnixPath()
{
var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll");
var expected = @"/etc/#NIN/dotnetcore/tryx/try1.dll";
var assembly = new TestAssembly
{
CodeBaseSettable = "file:///etc/#NIN/dotnetcore/tryx/try1.dll",
LocationSettable = expected,
};

// Act
var actual = RelatedAssemblyAttribute.GetAssemblyLocation(assembly);
Assert.Equal(expected, actual);
}

[Fact]
public void GetAssemblyLocation_CodeBase_HasPoundCharacterUNCPath()
{
var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll");
var expected = @"\\server\#NIN\dotnetcore\tryx\try1.dll";
var assembly = new TestAssembly
{
CodeBaseSettable = "file://server/#NIN/dotnetcore/tryx/try1.dll",
LocationSettable = expected,
};

// Act
var actual = RelatedAssemblyAttribute.GetAssemblyLocation(assembly);
Assert.Equal(expected, actual);
}

[Fact]
public void GetAssemblyLocation_CodeBase_HasPoundCharacterDOSPath()
{
var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll");
var expected = @"C:\#NIN\dotnetcore\tryx\try1.dll";
var assembly = new TestAssembly
{
CodeBaseSettable = "file:///C:/#NIN/dotnetcore/tryx/try1.dll",
LocationSettable = expected,
};

// Act
var actual = RelatedAssemblyAttribute.GetAssemblyLocation(assembly);
Assert.Equal(expected, actual);
}

private class TestAssembly : Assembly
{
public override AssemblyName GetName()
Expand All @@ -166,10 +84,6 @@ public override AssemblyName GetName()

public string AttributeAssembly { get; set; }

public string CodeBaseSettable { get; set; } = Path.Combine(AssemblyDirectory, "MyAssembly.dll");

public override string CodeBase => CodeBaseSettable;

public string LocationSettable { get; set; } = Path.Combine(AssemblyDirectory, "MyAssembly.dll");

public override string Location => LocationSettable;
Expand Down
8 changes: 4 additions & 4 deletions src/Servers/IIS/IIS/test/IIS.Tests/Utilities/TestServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class TestServer: IDisposable, IStartup
private const string HWebCoreDll = "hwebcore.dll";

internal static string HostableWebCoreLocation => Environment.ExpandEnvironmentVariables($@"%windir%\system32\inetsrv\{HWebCoreDll}");
internal static string BasePath => Path.Combine(Path.GetDirectoryName(new Uri(typeof(TestServer).Assembly.CodeBase).AbsolutePath),
internal static string BasePath => Path.Combine(Path.GetDirectoryName(typeof(TestServer).Assembly.Location),
"ANCM",
Environment.Is64BitProcess ? "x64" : "x86");

Expand All @@ -45,7 +45,7 @@ public class TestServer: IDisposable, IStartup
private readonly Action<IApplicationBuilder> _appBuilder;
private readonly ILoggerFactory _loggerFactory;
private readonly hostfxr_main_fn _hostfxrMainFn;

private Uri BaseUri => new Uri("http://localhost:" + _currentPort);
public HttpClient HttpClient { get; private set; }
public TestConnection CreateConnection() => new TestConnection(_currentPort);
Expand Down Expand Up @@ -88,7 +88,7 @@ private void Start()
{
LoadLibrary(HostableWebCoreLocation);
_appHostConfigPath = Path.GetTempFileName();

set_main_handler(_hostfxrMainFn);

Retry(() =>
Expand Down Expand Up @@ -159,7 +159,7 @@ public void Dispose()

// WebCoreShutdown occasionally AVs
// This causes the dotnet test process to crash
// To avoid this, we have to wait to shutdown
// To avoid this, we have to wait to shutdown
// and pass in true to immediately shutdown the hostable web core
// Both of these seem to be required.
Thread.Sleep(100);
Expand Down