SNOW-937188 fixes to the unquoting and comparison of final session re…

…turned by the server in case of values \"final\"
snowflakedb · May 7, 2024 · 2849369 · 2849369
1 parent a73b38d
commit 2849369
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 29 deletions.
diff --git a/Snowflake.Data.Tests/UnitTests/SFSessionTest.cs b/Snowflake.Data.Tests/UnitTests/SFSessionTest.cs
@@ -93,5 +93,35 @@ public void TestThatConfiguresEasyLogging(string configPath)
             // assert
             easyLoggingStarter.Verify(starter => starter.Init(configPath));
         }
+
+        [TestCase(null, "accountDefault", "accountDefault", false)]
+        [TestCase("initial", "initial", "initial", false)]
+        [TestCase("initial", null, "initial", false)]
+        [TestCase("initial", "IniTiaL", "initial", false)]
+        [TestCase("initial", "final", "final", true)]
+        [TestCase("initial", "\\\"final\\\"", "\"final\"", true)]
+        [TestCase("initial", "\\\"Final\\\"", "\"Final\"", true)]
+        [TestCase("\"Ini\\t\"ial\"", "\\\"Ini\\t\"ial\\\"", "\"Ini\\t\"ial\"", false)]
+        [TestCase("\"initial\"", "initial", "initial", true)]
+        [TestCase("\"initial\"", "\\\"initial\\\"", "\"initial\"", false)]
+        [TestCase("init\"ial", "init\"ial", "init\"ial", false)]
+        [TestCase("\"init\"ial\"", "\\\"init\"ial\\\"", "\"init\"ial\"", false)]
+        [TestCase("\"init\"ial\"", "\\\"Init\"ial\\\"", "\"Init\"ial\"", true)]
+        public void TestSessionPropertyQuotationSafeUpdateOnServerResponse(string sessionInitialValue, string serverResponseFinalSessionValue, string unquotedExpectedFinalValue, bool wasChanged)
+        {
+            // Arrange
+            SFSession sfSession = new SFSession("account=test;user=test;password=test", null);
+            var changedSessionValue = sessionInitialValue;
+
+            // Act
+            sfSession.UpdateSessionProperty(ref changedSessionValue, serverResponseFinalSessionValue);
+
+            // Assert
+            Assert.AreEqual(sfSession.SessionPropertiesChanged, wasChanged);
+            if (wasChanged || sessionInitialValue is null)
+                Assert.AreEqual(unquotedExpectedFinalValue, changedSessionValue);
+            else
+                Assert.AreEqual(sessionInitialValue, changedSessionValue);
+        }
     }
 }
diff --git a/Snowflake.Data/Core/Session/SFSession.cs b/Snowflake.Data/Core/Session/SFSession.cs
@@ -473,47 +473,35 @@ internal void UpdateSessionProperties(QueryExecResponseData responseData)
             UpdateSessionProperty(ref warehouse, responseData.finalWarehouseName);
         }
 
-        private void UpdateSessionProperty(ref string initialSessionValue, string finalSessionValue)
+        internal void UpdateSessionProperty(ref string initialSessionValue, string finalSessionValue)
         {
             // with HTAP session metadata removal database/schema might be not returned in query result
             if (!string.IsNullOrEmpty(finalSessionValue))
             {
-                if (!string.IsNullOrEmpty(initialSessionValue) && !IsEqualSkippingQuotes(initialSessionValue, finalSessionValue))
+                bool quoted = false;
+                string unquotedFinalValue = UnquoteJson(finalSessionValue, ref quoted);
+                if (!string.IsNullOrEmpty(initialSessionValue))
                 {
-                    sessionPropertiesChanged = true;
+                    quoted |= initialSessionValue.StartsWith('\"');
+                    if (!string.Equals(initialSessionValue, unquotedFinalValue, quoted ? StringComparison.Ordinal : StringComparison.InvariantCultureIgnoreCase))
+                    {
+                        sessionPropertiesChanged = true;
+                        initialSessionValue = unquotedFinalValue;
+                    }
+                }
+                else // null session value gets populated and is not treated as a session property change
+                {
+                    initialSessionValue = unquotedFinalValue;
                 }
-                initialSessionValue = finalSessionValue;
             }
         }
 
-        private static bool IsEqualSkippingQuotes(string value1, string value2)
-        {
-            bool wasUnquoted1 = false, wasUnquoted2 = false;
-            var unquotedValue1  = Unquote(value1, ref wasUnquoted1);
-            var unquotedValue2  = Unquote(value2, ref wasUnquoted2);
-            return string.Equals(unquotedValue1,
-                unquotedValue2,
-                wasUnquoted1 || wasUnquoted2 ? StringComparison.InvariantCulture : StringComparison.InvariantCultureIgnoreCase);
-        }
-
-        private static string Unquote(string value, ref bool unquoted)
+        private static string UnquoteJson(string value, ref bool unquoted)
         {
             if (value is null)
                 return value;
-            var valueLength = value.Length;
-            var unquotedJson = valueLength >= 4 && value.StartsWith("\\\"") && value.EndsWith("\\\"");
-            var unquotedStr = valueLength >= 2 && value[0] == '"' && value[valueLength-1] == '"';
-
-            var ret =  value;
-            if (unquotedJson)
-            {
-                ret = value.Substring(2, valueLength - 4);
-            } else if (unquotedStr)
-            {
-                ret = value.Substring(1, valueLength - 2);
-            }
-            unquoted = unquotedJson || unquotedStr;
-            return ret;
+            unquoted = value.Length >= 2 && value.StartsWith("\\\"") && value.EndsWith("\\\"");
+            return unquoted ? value.Replace("\\\"", "\"") : value;
         }
 
         internal bool SessionPropertiesChanged => sessionPropertiesChanged;