From 2d3d4744ff9052e4c965550c870794610309d2b1 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Mon, 2 Aug 2021 21:27:33 +0200 Subject: [PATCH 1/6] Respect AppContext.SetData with APP_CONFIG_FILE key --- .../System/Configuration/ClientConfigPaths.cs | 18 +++- ...guration.ConfigurationManager.Tests.csproj | 1 + .../Configuration/ConfigurationPathTests.cs | 92 +++++++++++++++++++ 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ConfigurationPathTests.cs diff --git a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs index a92612ca0b0f4..224b134a8a147 100644 --- a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs +++ b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs @@ -92,7 +92,23 @@ 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); + if (externalConfigUri.IsFile) + { + ApplicationConfigUri = externalConfigUri.LocalPath; + } + } + else + { + ApplicationConfigUri = Path.GetFullPath(externalConfigPath); + } + } + else if (!string.IsNullOrEmpty(ApplicationUri)) { string applicationPath = ApplicationUri; if (isSingleFile) diff --git a/src/libraries/System.Configuration.ConfigurationManager/tests/System.Configuration.ConfigurationManager.Tests.csproj b/src/libraries/System.Configuration.ConfigurationManager/tests/System.Configuration.ConfigurationManager.Tests.csproj index 94670bcb997df..816d4ca00292e 100644 --- a/src/libraries/System.Configuration.ConfigurationManager/tests/System.Configuration.ConfigurationManager.Tests.csproj +++ b/src/libraries/System.Configuration.ConfigurationManager/tests/System.Configuration.ConfigurationManager.Tests.csproj @@ -65,6 +65,7 @@ + diff --git a/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ConfigurationPathTests.cs b/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ConfigurationPathTests.cs new file mode 100644 index 0000000000000..5d2442dffbfe0 --- /dev/null +++ b/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ConfigurationPathTests.cs @@ -0,0 +1,92 @@ +// 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 Microsoft.DotNet.RemoteExecutor; +using Xunit; + +namespace System.ConfigurationTests +{ + public class ConfigurationPathTests + { + private const string ConfigName = "APP_CONFIG_FILE"; + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void CustomAppConfigIsUsedWhenSpecifiedAsRelativePath() + { + const string SettingName = "test_CustomAppConfigIsUsedWhenSpecified"; + string expectedSettingValue = Guid.NewGuid().ToString(); + string configFilePath = CreateAppConfigFileWithSetting(SettingName, expectedSettingValue); + + RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => { + AppDomain.CurrentDomain.SetData(ConfigName, 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.GetFullPath(CreateAppConfigFileWithSetting(SettingName, expectedSettingValue)); + + RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => { + AppDomain.CurrentDomain.SetData(ConfigName, 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.GetFullPath(CreateAppConfigFileWithSetting(SettingName, expectedSettingValue))).ToString(); + + RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => { + AppDomain.CurrentDomain.SetData(ConfigName, configFilePath); + Assert.Equal(expectedSettingValue, ConfigurationManager.AppSettings[SettingName]); + }, configFilePath, expectedSettingValue).Dispose(); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void NoErrorWhenCustomAppConfigIsSpecifiedAndItDoesNotExist() + { + RemoteExecutor.Invoke(() => + { + AppDomain.CurrentDomain.SetData(ConfigName, "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 = CreateAppConfigFileWithSetting(SettingName, "\""); + + RemoteExecutor.Invoke((string configFilePath) => { + AppDomain.CurrentDomain.SetData(ConfigName, configFilePath); + Assert.Throws(() => ConfigurationManager.AppSettings[SettingName]); + }, configFilePath).Dispose(); + } + + private static string CreateAppConfigFileWithSetting(string key, string rawUnquotedValue) + { + string fileName = Path.GetRandomFileName() + ".config"; + File.WriteAllText(fileName, + @$" + + + + +"); + + return fileName; + } + } +} From 0a0230370a212e8c389756e4f5e93ece9ab527ac Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Tue, 3 Aug 2021 13:54:39 +0200 Subject: [PATCH 2/6] apply feedback (use BaseDirectory and FileCleanupTestBase) --- .../System/IO/FileCleanupTestBase.cs | 6 ++- .../System/Configuration/ClientConfigPaths.cs | 4 +- .../Configuration/ConfigurationPathTests.cs | 43 ++++++++++++------- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs b/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs index a45aab12165a5..f98ecd13c710f 100644 --- a/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs +++ b/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs @@ -18,8 +18,10 @@ public abstract class FileCleanupTestBase : IDisposable protected static bool IsProcessElevated => s_isElevated.Value; /// Initialize the test class base. This creates the associated test directory. - protected FileCleanupTestBase() + protected FileCleanupTestBase(string cleanupRootDirectory = null) { + cleanupRootDirectory ??= Path.GetTempPath(); + // Use a unique test directory per test class. The test directory lives in the user's temp directory, // 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 @@ -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(cleanupRootDirectory, GetType().Name + "_" + Path.GetRandomFileName()); try { Directory.CreateDirectory(TestDirectory); diff --git a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs index 224b134a8a147..d71a77813ce34 100644 --- a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs +++ b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs @@ -105,7 +105,9 @@ private ClientConfigPaths(string exePath, bool includeUserConfig) } else { - ApplicationConfigUri = Path.GetFullPath(externalConfigPath); + // Uri constructor would throw for relative paths but full framework doesn't. + // We will mimic full framework behavior here. + ApplicationConfigUri = Path.GetFullPath(externalConfigPath, AppDomain.CurrentDomain.BaseDirectory); } } else if (!string.IsNullOrEmpty(ApplicationUri)) diff --git a/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ConfigurationPathTests.cs b/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ConfigurationPathTests.cs index 5d2442dffbfe0..a802cccb393f5 100644 --- a/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ConfigurationPathTests.cs +++ b/src/libraries/System.Configuration.ConfigurationManager/tests/System/Configuration/ConfigurationPathTests.cs @@ -3,24 +3,29 @@ using System.Configuration; using System.IO; +using System.Runtime.CompilerServices; using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.ConfigurationTests { - public class ConfigurationPathTests + public class ConfigurationPathTests : FileCleanupTestBase { - private const string ConfigName = "APP_CONFIG_FILE"; + 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))] public void CustomAppConfigIsUsedWhenSpecifiedAsRelativePath() { const string SettingName = "test_CustomAppConfigIsUsedWhenSpecified"; string expectedSettingValue = Guid.NewGuid().ToString(); - string configFilePath = CreateAppConfigFileWithSetting(SettingName, expectedSettingValue); + string configFilePath = Path.Combine(GetTestDirectoryName(), CreateAppConfigFileWithSetting(SettingName, expectedSettingValue)); RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => { - AppDomain.CurrentDomain.SetData(ConfigName, configFilePath); + // 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(); } @@ -30,10 +35,10 @@ public void CustomAppConfigIsUsedWhenSpecifiedAsAbsolutePath() { const string SettingName = "test_CustomAppConfigIsUsedWhenSpecified"; string expectedSettingValue = Guid.NewGuid().ToString(); - string configFilePath = Path.GetFullPath(CreateAppConfigFileWithSetting(SettingName, expectedSettingValue)); + string configFilePath = Path.Combine(TestDirectory, CreateAppConfigFileWithSetting(SettingName, expectedSettingValue)); RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => { - AppDomain.CurrentDomain.SetData(ConfigName, configFilePath); + AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", configFilePath); Assert.Equal(expectedSettingValue, ConfigurationManager.AppSettings[SettingName]); }, configFilePath, expectedSettingValue).Dispose(); } @@ -43,10 +48,10 @@ public void CustomAppConfigIsUsedWhenSpecifiedAsAbsoluteUri() { const string SettingName = "test_CustomAppConfigIsUsedWhenSpecified"; string expectedSettingValue = Guid.NewGuid().ToString(); - string configFilePath = new Uri(Path.GetFullPath(CreateAppConfigFileWithSetting(SettingName, expectedSettingValue))).ToString(); + string configFilePath = new Uri(Path.Combine(TestDirectory, CreateAppConfigFileWithSetting(SettingName, expectedSettingValue))).ToString(); RemoteExecutor.Invoke((string configFilePath, string expectedSettingValue) => { - AppDomain.CurrentDomain.SetData(ConfigName, configFilePath); + AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", configFilePath); Assert.Equal(expectedSettingValue, ConfigurationManager.AppSettings[SettingName]); }, configFilePath, expectedSettingValue).Dispose(); } @@ -56,7 +61,7 @@ public void NoErrorWhenCustomAppConfigIsSpecifiedAndItDoesNotExist() { RemoteExecutor.Invoke(() => { - AppDomain.CurrentDomain.SetData(ConfigName, "non-existing-file.config"); + AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", "non-existing-file.config"); Assert.Null(ConfigurationManager.AppSettings["AnySetting"]); }).Dispose(); } @@ -67,25 +72,33 @@ public void MalformedAppConfigCausesException() const string SettingName = "AnySetting"; // Following will cause malformed config file - string configFilePath = CreateAppConfigFileWithSetting(SettingName, "\""); + string configFilePath = Path.Combine(TestDirectory, CreateAppConfigFileWithSetting(SettingName, "\"")); RemoteExecutor.Invoke((string configFilePath) => { - AppDomain.CurrentDomain.SetData(ConfigName, configFilePath); + AppDomain.CurrentDomain.SetData("APP_CONFIG_FILE", configFilePath); Assert.Throws(() => ConfigurationManager.AppSettings[SettingName]); }, configFilePath).Dispose(); } - private static string CreateAppConfigFileWithSetting(string key, string rawUnquotedValue) + private string GetTestDirectoryName() { - string fileName = Path.GetRandomFileName() + ".config"; - File.WriteAllText(fileName, + 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), @$" "); - return fileName; } } From cdbceb123535511557cbfb1398346b4cb0bfe16a Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Tue, 3 Aug 2021 15:24:16 +0200 Subject: [PATCH 3/6] avoid using GetFullPath with 2 arguments as it's not available everywhere --- .../src/System/Configuration/ClientConfigPaths.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs index d71a77813ce34..976e567647d3e 100644 --- a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs +++ b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs @@ -107,7 +107,13 @@ private ClientConfigPaths(string exePath, bool includeUserConfig) { // Uri constructor would throw for relative paths but full framework doesn't. // We will mimic full framework behavior here. - ApplicationConfigUri = Path.GetFullPath(externalConfigPath, AppDomain.CurrentDomain.BaseDirectory); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && externalConfigPath.Length >= 2 && externalConfigPath.StartsWith('\\') && externalConfigPath[1] != '\\') + { + // if path starts with a single backslash Path.Combine would combine with a root of BaseDirectory so we trim that character to avoid that + externalConfigPath = externalConfigPath.Substring(0, externalConfigPath.Length - 1); + } + + ApplicationConfigUri = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, externalConfigPath); } } else if (!string.IsNullOrEmpty(ApplicationUri)) From afcf7a55f6f70aa7fffe1ed191a093c57eb53887 Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 5 Aug 2021 14:29:33 +0200 Subject: [PATCH 4/6] trim all directory separators rather than single --- .../src/System/Configuration/ClientConfigPaths.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs index 976e567647d3e..ba2ce5c546f73 100644 --- a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs +++ b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs @@ -107,11 +107,10 @@ private ClientConfigPaths(string exePath, bool includeUserConfig) { // Uri constructor would throw for relative paths but full framework doesn't. // We will mimic full framework behavior here. - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && externalConfigPath.Length >= 2 && externalConfigPath.StartsWith('\\') && externalConfigPath[1] != '\\') - { - // if path starts with a single backslash Path.Combine would combine with a root of BaseDirectory so we trim that character to avoid that - externalConfigPath = externalConfigPath.Substring(0, externalConfigPath.Length - 1); - } + + // If path starts with directory separator Path.Combine would combine with a root of BaseDirectory so we trim that character to avoid that + // For absolute path we should already be covered by the other code path + externalConfigPath = externalConfigPath.TrimStart(Path.DirectorySeparatorChar); ApplicationConfigUri = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, externalConfigPath); } From 79fe28e98382840594ccbbd0e7fd58c0e840ddce Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 5 Aug 2021 14:42:32 +0200 Subject: [PATCH 5/6] cleanupRootDirectory -> tempDirectory --- .../tests/TestUtilities/System/IO/FileCleanupTestBase.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs b/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs index f98ecd13c710f..04b27885d4d38 100644 --- a/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs +++ b/src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs @@ -18,9 +18,9 @@ public abstract class FileCleanupTestBase : IDisposable protected static bool IsProcessElevated => s_isElevated.Value; /// Initialize the test class base. This creates the associated test directory. - protected FileCleanupTestBase(string cleanupRootDirectory = null) + protected FileCleanupTestBase(string tempDirectory = null) { - cleanupRootDirectory ??= Path.GetTempPath(); + tempDirectory ??= Path.GetTempPath(); // Use a unique test directory per test class. The test directory lives in the user's temp directory, // and includes both the name of the test class and a random string. The test class name is included @@ -33,7 +33,7 @@ protected FileCleanupTestBase(string cleanupRootDirectory = null) string failure = string.Empty; for (int i = 0; i <= 2; i++) { - TestDirectory = Path.Combine(cleanupRootDirectory, GetType().Name + "_" + Path.GetRandomFileName()); + TestDirectory = Path.Combine(tempDirectory, GetType().Name + "_" + Path.GetRandomFileName()); try { Directory.CreateDirectory(TestDirectory); From eec3437b70df8844caef85e1c7ecd5218fccb1ec Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 5 Aug 2021 15:19:51 +0200 Subject: [PATCH 6/6] fix absolute paths on linux --- .../src/System/Configuration/ClientConfigPaths.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs index ba2ce5c546f73..66646d1b3bbd9 100644 --- a/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs +++ b/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs @@ -97,7 +97,7 @@ private ClientConfigPaths(string exePath, bool includeUserConfig) { if (Uri.IsWellFormedUriString(externalConfigPath, UriKind.Absolute)) { - Uri externalConfigUri = new Uri(externalConfigPath); + Uri externalConfigUri = new Uri(externalConfigPath, UriKind.Absolute); if (externalConfigUri.IsFile) { ApplicationConfigUri = externalConfigUri.LocalPath; @@ -105,14 +105,12 @@ private ClientConfigPaths(string exePath, bool includeUserConfig) } else { - // Uri constructor would throw for relative paths but full framework doesn't. - // We will mimic full framework behavior here. - - // If path starts with directory separator Path.Combine would combine with a root of BaseDirectory so we trim that character to avoid that - // For absolute path we should already be covered by the other code path - externalConfigPath = externalConfigPath.TrimStart(Path.DirectorySeparatorChar); + if (!Path.IsPathRooted(externalConfigPath)) + { + externalConfigPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, externalConfigPath); + } - ApplicationConfigUri = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, externalConfigPath); + ApplicationConfigUri = Path.GetFullPath(externalConfigPath); } } else if (!string.IsNullOrEmpty(ApplicationUri))