- Adding exception handling for converter and test for invalid conver…

…sion 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.
NuGet · Sep 25, 2024 · d211fe0 · d211fe0
1 parent aae9f6d
commit d211fe0
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 7 deletions.
diff --git a/...lients/NuGet.PackageManagement.UI/Converters/GreaterThanThresholdToVisibilityConverter.cs b/...lients/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
@@ -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/...et.PackageManagement.UI.Test/Converters/GreaterThanThresholdToVisibilityConverterTests.cs b/...et.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);
+        }
     }
 }