From d211fe085a16854df687e41fbca63bcda63e0860 Mon Sep 17 00:00:00 2001
From: Jean-Pierre Briede <jebriede@microsoft.com>
Date: Wed, 25 Sep 2024 12:34:50 -0700
Subject: [PATCH] - Adding exception handling for converter and test for
 invalid conversion cases. - Only adding to VulnerableVersions dictionary if
 it does not already exist because the same version is installed in another
 project and only if it's added should we fire OnPropertyChanged.

---
 ...eaterThanThresholdToVisibilityConverter.cs | 31 ++++++++++++++++---
 .../ViewModels/PackageItemViewModel.cs        | 10 ++++--
 ...ThanThresholdToVisibilityConverterTests.cs | 20 ++++++++++++
 3 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/GreaterThanThresholdToVisibilityConverter.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/GreaterThanThresholdToVisibilityConverter.cs
index fc4d0b7b978..b80a71c339b 100644
--- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/GreaterThanThresholdToVisibilityConverter.cs
+++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/GreaterThanThresholdToVisibilityConverter.cs
@@ -15,11 +15,14 @@ public object Convert(object value, Type targetType, object parameter, CultureIn
         {
             if (targetType == typeof(Visibility))
             {
-                if (value is not null)
+                if (value is not null && parameter is not null)
                 {
-                    long intValue = System.Convert.ToInt64(value, culture);
-                    long parameterValue = System.Convert.ToInt64(parameter, culture);
-                    if (intValue > parameterValue)
+                    bool valueConverted = TryConvertToInt64(value, culture, out long? initialValue);
+                    bool paramConverted = TryConvertToInt64(parameter, culture, out long? thresholdValue);
+
+                    if (valueConverted && paramConverted
+                        && initialValue is not null && thresholdValue is not null
+                        && initialValue > thresholdValue)
                     {
                         return Visibility.Visible;
                     }
@@ -37,5 +40,25 @@ public object ConvertBack(object value, Type targetType, object parameter, Cultu
             Debug.Fail("Not Implemented");
             return null;
         }
+
+        private static bool TryConvertToInt64(object value, CultureInfo culture, out long? convertedValue)
+        {
+            if (value is null)
+            {
+                convertedValue = null;
+                return false;
+            }
+
+            try
+            {
+                convertedValue = System.Convert.ToInt64(value, culture);
+                return true;
+            }
+            catch (Exception e) when (e is FormatException or InvalidCastException or OverflowException)
+            {
+                convertedValue = null;
+                return false;
+            }
+        }
     }
 }
diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs
index 2a4b68a21c0..a34ef228289 100644
--- a/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs
+++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs
@@ -15,6 +15,7 @@
 using System.Threading.Tasks;
 using System.Windows.Media.Imaging;
 using Microsoft;
+using Microsoft.VisualStudio.Services.Common;
 using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Threading;
 using NuGet.PackageManagement.UI.ViewModels;
@@ -914,12 +915,15 @@ private void SetVulnerabilityMaxSeverity(NuGetVersion version, int maxSeverity)
         {
             if (maxSeverity > -1)
             {
-                VulnerableVersions.Add(version, maxSeverity);
+                if (VulnerableVersions.TryAdd(version, maxSeverity))
+                {
+                    OnPropertyChanged(nameof(VulnerableVersions));
+                    OnPropertyChanged(nameof(VulnerableVersionsString));
+                }
+
                 VulnerabilityMaxSeverity = Math.Max(VulnerabilityMaxSeverity, maxSeverity);
 
                 OnPropertyChanged(nameof(Status));
-                OnPropertyChanged(nameof(VulnerableVersions));
-                OnPropertyChanged(nameof(VulnerableVersionsString));
             }
         }
 
diff --git a/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Converters/GreaterThanThresholdToVisibilityConverterTests.cs b/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Converters/GreaterThanThresholdToVisibilityConverterTests.cs
index 758074f68fa..7f88414cea4 100644
--- a/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Converters/GreaterThanThresholdToVisibilityConverterTests.cs
+++ b/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Converters/GreaterThanThresholdToVisibilityConverterTests.cs
@@ -58,5 +58,25 @@ public void Convert_ValidLongValue_Return_VisibilityCollapsed(long? downloadCoun
 
             Assert.Equal(Visibility.Collapsed, converted);
         }
+
+        [Theory]
+        [InlineData(1, null)]
+        [InlineData(null, 1)]
+        [InlineData(null, null)]
+        [InlineData("non-numeric", 1)]
+        [InlineData(1, "non-numeric")]
+        [InlineData("9223372036854775808", 1)] // test overflowed Int64 number as string value
+        public void Convert_InvalidLongValue_Return_VisibilityCollapsed(object value, object param)
+        {
+            var converter = new GreaterThanThresholdToVisibilityConverter();
+
+            object converted = converter.Convert(
+                value,
+                typeof(Visibility),
+                param,
+                Thread.CurrentThread.CurrentCulture);
+
+            Assert.Equal(Visibility.Collapsed, converted);
+        }
     }
 }