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.

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

Respect AppContext.SetData with APP_CONFIG_FILE key #56748

Merged
merged 6 commits into from
Aug 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ public abstract class FileCleanupTestBase : IDisposable
protected static bool IsProcessElevated => s_isElevated.Value;

/// <summary>Initialize the test class base. This creates the associated test directory.</summary>
protected FileCleanupTestBase()
protected FileCleanupTestBase(string tempDirectory = null)
{
tempDirectory ??= Path.GetTempPath();

// Use a unique test directory per test class. The test directory lives in the user's temp directory,
krwq marked this conversation as resolved.
Show resolved Hide resolved
// and includes both the name of the test class and a random string. The test class name is included
// so that it can be easily correlated if necessary, and the random string to helps avoid conflicts if
Expand All @@ -31,7 +33,7 @@ protected FileCleanupTestBase()
string failure = string.Empty;
for (int i = 0; i <= 2; i++)
{
TestDirectory = Path.Combine(Path.GetTempPath(), GetType().Name + "_" + Path.GetRandomFileName());
TestDirectory = Path.Combine(tempDirectory, GetType().Name + "_" + Path.GetRandomFileName());
try
{
Directory.CreateDirectory(TestDirectory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,28 @@ private ClientConfigPaths(string exePath, bool includeUserConfig)
}
}

if (!string.IsNullOrEmpty(ApplicationUri))
string externalConfigPath = AppDomain.CurrentDomain.GetData("APP_CONFIG_FILE") as string;
if (!string.IsNullOrEmpty(externalConfigPath))
{
if (Uri.IsWellFormedUriString(externalConfigPath, UriKind.Absolute))
{
Uri externalConfigUri = new Uri(externalConfigPath, UriKind.Absolute);
if (externalConfigUri.IsFile)
{
ApplicationConfigUri = externalConfigUri.LocalPath;
}
}
else
{
if (!Path.IsPathRooted(externalConfigPath))
{
externalConfigPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, externalConfigPath);
}

ApplicationConfigUri = Path.GetFullPath(externalConfigPath);
Copy link
Member

Choose a reason for hiding this comment

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

This is implicitly prefixing with whatever the current directory is at the time the config system is initialized, I guess?

In the desktop code it is using the appbase to prefix (naming.cpp line 3241 .. assuming I'm looking in the right place)

I think in .NET Core that is AppDomain.BaseDirectory, so should this be Path.Combine(AppDomain.CurrentDomain.BaseDirectory, externalConfigPath) ? There is something similar higher up.

Copy link
Member

Choose a reason for hiding this comment

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

You can change the tests to set Environment.CurrentDirectory to something random before reading the value (I'm guessing the test will fail right now)

Copy link
Member

Choose a reason for hiding this comment

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

When you use Path.Combine you need to be careful with paths that begin with \, as you'll get the second path against the root of the current directory. Trim the start of the externalConfigPath for safety.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have used Path.GetFullPath(externalConfigPath, AppDomain.CurrentDomain.BaseDirectory) which seems to be working fine. The tests are now changing CurrentDirectory to validate we are relative to the BaseDirectory and not CurrentDirectory

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm seems that overload is not available everywhere...

}
}
else if (!string.IsNullOrEmpty(ApplicationUri))
{
string applicationPath = ApplicationUri;
if (isSingleFile)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
<Compile Include="System\Configuration\ConfigurationElementCollectionTests.cs" />
<Compile Include="System\Configuration\ConfigurationElementTests.cs" />
<Compile Include="System\Configuration\ConfigurationPropertyAttributeTests.cs" />
<Compile Include="System\Configuration\ConfigurationPathTests.cs" />
<Compile Include="System\Configuration\CustomHostTests.cs" />
<Compile Include="System\Configuration\ConfigurationPropertyTests.cs" />
<Compile Include="System\Configuration\ConfigurationTests.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Configuration;
using System.IO;
using System.Runtime.CompilerServices;
using Microsoft.DotNet.RemoteExecutor;
using Xunit;

namespace System.ConfigurationTests
{
public class ConfigurationPathTests : FileCleanupTestBase
{
public ConfigurationPathTests() : base(AppDomain.CurrentDomain.BaseDirectory) // We do not want the files go to temporary directory as that will not test the relative paths correctly
{
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
krwq marked this conversation as resolved.
Show resolved Hide resolved
public void CustomAppConfigIsUsedWhenSpecifiedAsRelativePath()
{
const string SettingName = "test_CustomAppConfigIsUsedWhenSpecified";
string expectedSettingValue = Guid.NewGuid().ToString();
string configFilePath = Path.Combine(GetTestDirectoryName(), CreateAppConfigFileWithSetting(SettingName, expectedSettingValue));

RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => {
// We change directory so that if product tries to read from the current directory which usually happens to be same as BaseDirectory the test will fail
Environment.CurrentDirectory = Path.GetFullPath(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, ".."));
AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", configFilePath);
Assert.Equal(expectedSettingValue, ConfigurationManager.AppSettings[SettingName]);
}, configFilePath, expectedSettingValue).Dispose();
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public void CustomAppConfigIsUsedWhenSpecifiedAsAbsolutePath()
{
const string SettingName = "test_CustomAppConfigIsUsedWhenSpecified";
string expectedSettingValue = Guid.NewGuid().ToString();
string configFilePath = Path.Combine(TestDirectory, CreateAppConfigFileWithSetting(SettingName, expectedSettingValue));

RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => {
AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", configFilePath);
Assert.Equal(expectedSettingValue, ConfigurationManager.AppSettings[SettingName]);
}, configFilePath, expectedSettingValue).Dispose();
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public void CustomAppConfigIsUsedWhenSpecifiedAsAbsoluteUri()
{
const string SettingName = "test_CustomAppConfigIsUsedWhenSpecified";
string expectedSettingValue = Guid.NewGuid().ToString();
string configFilePath = new Uri(Path.Combine(TestDirectory, CreateAppConfigFileWithSetting(SettingName, expectedSettingValue))).ToString();

RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => {
AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", configFilePath);
Assert.Equal(expectedSettingValue, ConfigurationManager.AppSettings[SettingName]);
}, configFilePath, expectedSettingValue).Dispose();
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public void NoErrorWhenCustomAppConfigIsSpecifiedAndItDoesNotExist()
{
RemoteExecutor.Invoke(() =>
{
AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", "non-existing-file.config");
Assert.Null(ConfigurationManager.AppSettings["AnySetting"]);
}).Dispose();
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public void MalformedAppConfigCausesException()
{
const string SettingName = "AnySetting";

// Following will cause malformed config file
string configFilePath = Path.Combine(TestDirectory, CreateAppConfigFileWithSetting(SettingName, "\""));

RemoteExecutor.Invoke((string configFilePath) => {
AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", configFilePath);
Assert.Throws<ConfigurationErrorsException>(() => ConfigurationManager.AppSettings[SettingName]);
}, configFilePath).Dispose();
}

private string GetTestDirectoryName()
{
string dir = TestDirectory;
if (dir.EndsWith("\\") || dir.EndsWith("/"))
dir = dir.Substring(0, dir.Length - 1);

return Path.GetFileName(dir);
}

private string CreateAppConfigFileWithSetting(string key, string rawUnquotedValue, [CallerMemberName] string memberName = null, [CallerLineNumber] int lineNumber = 0)
{
string fileName = GetTestFileName(null, memberName, lineNumber) + ".config";
File.WriteAllText(Path.Combine(TestDirectory, fileName),
@$"<?xml version=""1.0"" encoding=""utf-8"" ?>
<configuration>
<appSettings>
<add key=""{key}"" value=""{rawUnquotedValue}""/>
</appSettings>
</configuration>");
return fileName;
}
}
}