From d2d6763decaf8255c9f15ebec66b17e946e0d1ab Mon Sep 17 00:00:00 2001 From: Javad Rahnama Date: Mon, 24 Apr 2023 11:16:57 -0700 Subject: [PATCH 1/5] Addressing Issue #2015 --- .../SqlConnectionEncryptOption.xml | 2 +- .../Data/Common/DbConnectionStringCommon.cs | 8 +++++ .../SqlClient/SqlConnectionEncryptOption.cs | 29 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml index 35b8d24ab6..129243a760 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml @@ -26,7 +26,7 @@ Implicit conversions have been added to maintain backwards compatibility with bo - Converts the specified string representation of a logical value to its equivalent and returns a value that indicates whether the conversion succeeded. + Converts the specified string, boolean representation of a logical value to its equivalent and returns a value that indicates whether the conversion succeeded. A string containing the value to convert. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/DbConnectionStringCommon.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/DbConnectionStringCommon.cs index f8a31231c0..7e21441852 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/DbConnectionStringCommon.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/DbConnectionStringCommon.cs @@ -816,10 +816,18 @@ internal static SqlConnectionEncryptOption ConvertToSqlConnectionEncryptOption(s { return DbConnectionStringDefaults.Encrypt; } + else if(value is SqlConnectionEncryptOption eValue) + { + return eValue; + } else if (value is string sValue) { return SqlConnectionEncryptOption.Parse(sValue); } + else if(value is bool bValue) + { + return SqlConnectionEncryptOption.Parse(bValue); + } throw ADP.InvalidConnectionOptionValue(keyword); } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs index fde49b3980..e26e9ee50c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs @@ -43,6 +43,20 @@ public static SqlConnectionEncryptOption Parse(string value) } } + /// + + public static SqlConnectionEncryptOption Parse(bool value) + { + if (TryParse(value, out SqlConnectionEncryptOption result)) + { + return result; + } + else + { + throw ADP.InvalidConnectionOptionValue(SqlConnectionString.KEY.Encrypt); + } + } + /// public static bool TryParse(string value, out SqlConnectionEncryptOption result) { @@ -73,6 +87,21 @@ public static bool TryParse(string value, out SqlConnectionEncryptOption result) } } + /// + + public static bool TryParse(bool value, out SqlConnectionEncryptOption result) + { + switch(value) + { + case true: + result = Mandatory; + return true; + case false: + result = Optional; + return true; + } + } + /// public static SqlConnectionEncryptOption Optional => s_optional; From 661f7189bc8a527b74171651b428181941e384f3 Mon Sep 17 00:00:00 2001 From: Javad Rahnama Date: Tue, 25 Apr 2023 13:02:03 -0700 Subject: [PATCH 2/5] Adding test and making new function internal --- .../SqlClient/SqlConnectionEncryptOption.cs | 8 +-- .../SqlConnectionStringBuilderTest.cs | 53 ++++++++++++++++++- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs index e26e9ee50c..cfa6d253ff 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs @@ -43,9 +43,7 @@ public static SqlConnectionEncryptOption Parse(string value) } } - /// - - public static SqlConnectionEncryptOption Parse(bool value) + internal static SqlConnectionEncryptOption Parse(bool value) { if (TryParse(value, out SqlConnectionEncryptOption result)) { @@ -87,9 +85,7 @@ public static bool TryParse(string value, out SqlConnectionEncryptOption result) } } - /// - - public static bool TryParse(bool value, out SqlConnectionEncryptOption result) + internal static bool TryParse(bool value, out SqlConnectionEncryptOption result) { switch(value) { diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs index 0825760e45..3e05e3bde3 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs @@ -253,7 +253,7 @@ public void SetInvalidPacketSize_Throws(int invalid) } [Theory] - [InlineData("AttachDBFilename","somefile.db")] + [InlineData("AttachDBFilename", "somefile.db")] public void SetKeyword(string keyword, string value) { SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(); @@ -380,7 +380,7 @@ public void ConnectionBuilderEncryptBackwardsCompatibility() builder.Encrypt = false; Assert.Equal("Encrypt=False", builder.ConnectionString); Assert.False(builder.Encrypt); - + builder.Encrypt = true; Assert.Equal("Encrypt=True", builder.ConnectionString); Assert.True(builder.Encrypt); @@ -402,6 +402,25 @@ public void ConnectionBuilderEncryptBackwardsCompatibility() Assert.True(builder.Encrypt); } + [Fact] + public void EncryptParserValidValuesPropertyIndexerForEncryptionOption() + { + SqlConnectionStringBuilder builder = new(); + try + { + builder["Encrypt"] = SqlConnectionEncryptOption.Strict; + CheckEncryptType(builder, SqlConnectionEncryptOption.Strict); + builder["Encrypt"] = SqlConnectionEncryptOption.Optional; + CheckEncryptType(builder, SqlConnectionEncryptOption.Optional); + builder["Encrypt"] = SqlConnectionEncryptOption.Mandatory; + CheckEncryptType(builder, SqlConnectionEncryptOption.Mandatory); + } + catch (SqlException ex) + { + Assert.Fail(ex.Message); + } + } + [Theory] [InlineData("true", "True")] [InlineData("mandatory", "True")] @@ -413,6 +432,30 @@ public void ConnectionBuilderEncryptBackwardsCompatibility() public void EncryptParserValidValuesParsesSuccessfully(string value, string expectedValue) => Assert.Equal(expectedValue, SqlConnectionEncryptOption.Parse(value).ToString()); + [Theory] + [InlineData(true)] + [InlineData(false)] + public void EncryptParserValidValuesPropertyIndexerForBoolean(bool value) + { + SqlConnectionStringBuilder builder = new(); + try + { + builder["Encrypt"] = value; + if(value == true) + { + CheckEncryptType(builder, SqlConnectionEncryptOption.Mandatory); + } + else + { + CheckEncryptType(builder, SqlConnectionEncryptOption.Optional); + } + } + catch (Exception ex) + { + Assert.Fail(ex.Message); + } + } + [Theory] [InlineData("something")] [InlineData("")] @@ -467,5 +510,11 @@ internal void ExecuteConnectionStringTests(string connectionString) Assert.NotNull(connection); } } + + internal void CheckEncryptType(SqlConnectionStringBuilder builder, SqlConnectionEncryptOption expectedValue) + { + Assert.IsType(builder.Encrypt); + Assert.Equal(expectedValue, builder.Encrypt); + } } } From c443f4ff69b6b3b04d39a353a4052437d9e37d33 Mon Sep 17 00:00:00 2001 From: Javad Date: Wed, 26 Apr 2023 12:58:57 -0700 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: David Engel --- .../SqlClient/SqlConnectionEncryptOption.cs | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs index cfa6d253ff..ecabdb9f04 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionEncryptOption.cs @@ -45,14 +45,7 @@ public static SqlConnectionEncryptOption Parse(string value) internal static SqlConnectionEncryptOption Parse(bool value) { - if (TryParse(value, out SqlConnectionEncryptOption result)) - { - return result; - } - else - { - throw ADP.InvalidConnectionOptionValue(SqlConnectionString.KEY.Encrypt); - } + return value ? Mandatory : Optional; } /// @@ -85,19 +78,6 @@ public static bool TryParse(string value, out SqlConnectionEncryptOption result) } } - internal static bool TryParse(bool value, out SqlConnectionEncryptOption result) - { - switch(value) - { - case true: - result = Mandatory; - return true; - case false: - result = Optional; - return true; - } - } - /// public static SqlConnectionEncryptOption Optional => s_optional; From c2506adcba4941b71ec26f81f01f6809b3a696c0 Mon Sep 17 00:00:00 2001 From: Javad Rahnama Date: Wed, 26 Apr 2023 12:59:31 -0700 Subject: [PATCH 4/5] reverting doc changes. --- .../Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml index 129243a760..35b8d24ab6 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionEncryptOption.xml @@ -26,7 +26,7 @@ Implicit conversions have been added to maintain backwards compatibility with bo - Converts the specified string, boolean representation of a logical value to its equivalent and returns a value that indicates whether the conversion succeeded. + Converts the specified string representation of a logical value to its equivalent and returns a value that indicates whether the conversion succeeded. A string containing the value to convert. From aa4b3b7965d6fc1562a8978e15d511ae956dd4ce Mon Sep 17 00:00:00 2001 From: Javad Date: Wed, 26 Apr 2023 19:04:14 -0700 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: DavoudEshtehari <61173489+DavoudEshtehari@users.noreply.github.com> --- .../SqlConnectionStringBuilderTest.cs | 39 +++++-------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs index 3e05e3bde3..2bc842566a 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs @@ -406,19 +406,12 @@ public void ConnectionBuilderEncryptBackwardsCompatibility() public void EncryptParserValidValuesPropertyIndexerForEncryptionOption() { SqlConnectionStringBuilder builder = new(); - try - { - builder["Encrypt"] = SqlConnectionEncryptOption.Strict; - CheckEncryptType(builder, SqlConnectionEncryptOption.Strict); - builder["Encrypt"] = SqlConnectionEncryptOption.Optional; - CheckEncryptType(builder, SqlConnectionEncryptOption.Optional); - builder["Encrypt"] = SqlConnectionEncryptOption.Mandatory; - CheckEncryptType(builder, SqlConnectionEncryptOption.Mandatory); - } - catch (SqlException ex) - { - Assert.Fail(ex.Message); - } + builder["Encrypt"] = SqlConnectionEncryptOption.Strict; + CheckEncryptType(builder, SqlConnectionEncryptOption.Strict); + builder["Encrypt"] = SqlConnectionEncryptOption.Optional; + CheckEncryptType(builder, SqlConnectionEncryptOption.Optional); + builder["Encrypt"] = SqlConnectionEncryptOption.Mandatory; + CheckEncryptType(builder, SqlConnectionEncryptOption.Mandatory); } [Theory] @@ -438,22 +431,8 @@ public void EncryptParserValidValuesParsesSuccessfully(string value, string expe public void EncryptParserValidValuesPropertyIndexerForBoolean(bool value) { SqlConnectionStringBuilder builder = new(); - try - { - builder["Encrypt"] = value; - if(value == true) - { - CheckEncryptType(builder, SqlConnectionEncryptOption.Mandatory); - } - else - { - CheckEncryptType(builder, SqlConnectionEncryptOption.Optional); - } - } - catch (Exception ex) - { - Assert.Fail(ex.Message); - } + builder["Encrypt"] = value; + CheckEncryptType(builder, value ? SqlConnectionEncryptOption.Mandatory : SqlConnectionEncryptOption.Optional); } [Theory] @@ -511,7 +490,7 @@ internal void ExecuteConnectionStringTests(string connectionString) } } - internal void CheckEncryptType(SqlConnectionStringBuilder builder, SqlConnectionEncryptOption expectedValue) + internal static void CheckEncryptType(SqlConnectionStringBuilder builder, SqlConnectionEncryptOption expectedValue) { Assert.IsType(builder.Encrypt); Assert.Equal(expectedValue, builder.Encrypt);