PM-3349 PM-3350 Improved code safety with try...catch, better invoke on main thread and better null handling.

2026-01-05 01:53:17 +00:00 · 2024-01-19 15:01:31 -03:00
parent 01ee1ff845
commit 4717f5e230
21 changed files with 612 additions and 377 deletions
--- a/src/iOS.Core/Handlers/CustomTabbedHandler.cs
+++ b/src/iOS.Core/Handlers/CustomTabbedHandler.cs
@@ -1,5 +1,4 @@
-using Bit.App.Abstractions;
-using Bit.App.Pages;
+using Bit.App.Pages;
 using Bit.App.Utilities;
 using Bit.Core.Abstractions;
 using Bit.Core.Utilities;
@@ -13,7 +12,7 @@ namespace Bit.iOS.Core.Handlers
    public partial class CustomTabbedHandler : TabbedRenderer
    {
        private IBroadcasterService _broadcasterService;
-        private UITabBarItem _previousSelectedItem;
+        private UITabBarItem? _previousSelectedItem;

        public CustomTabbedHandler()
        {
@@ -73,8 +72,7 @@ namespace Bit.iOS.Core.Handlers
        private void UpdateTabBarAppearance()
        {
            // https://developer.apple.com/forums/thread/682420
-            var deviceActionService = ServiceContainer.Resolve<IDeviceActionService>("deviceActionService");
-            if (deviceActionService.SystemMajorVersion() >= 15)
+            if (UIDevice.CurrentDevice.CheckSystemVersion(15,0))
            {
                var appearance = new UITabBarAppearance();
                appearance.ConfigureWithOpaqueBackground();
--- a/src/iOS.Core/Services/LocalizeService.cs
+++ b/src/iOS.Core/Services/LocalizeService.cs
@@ -1,5 +1,4 @@
-using System;
-using System.Diagnostics;
+using System.Diagnostics;
 using System.Globalization;
 using Bit.App.Abstractions;
 using Bit.App.Models;
@@ -20,7 +19,7 @@ namespace Bit.iOS.Core.Services
            }

            // This gets called a lot - try/catch can be expensive so consider caching or something
-            CultureInfo ci = null;
+            CultureInfo? ci;
            try
            {
                ci = new CultureInfo(netLanguage);
@@ -108,7 +107,7 @@ namespace Bit.iOS.Core.Services
            {
                df.Locale = NSLocale.CurrentLocale;
                df.DateStyle = NSDateFormatterStyle.Short;
-                return df.StringFor((NSDate)date);
+                return df.StringFor((NSDate?)date);
            }
        }

@@ -118,7 +117,7 @@ namespace Bit.iOS.Core.Services
            {
                df.Locale = NSLocale.CurrentLocale;
                df.TimeStyle = NSDateFormatterStyle.Short;
-                return df.StringFor((NSDate)time);
+                return df.StringFor((NSDate?)time);
            }
        }
    }
--- a/src/iOS.Core/Utilities/DictionaryExtensions.cs
+++ b/src/iOS.Core/Utilities/DictionaryExtensions.cs
@@ -1,8 +1,4 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using Bit.Core.Models.Domain;
-using Foundation;
+using Foundation;
 using Newtonsoft.Json;

 namespace Bit.iOS.Core.Utilities
@@ -15,6 +11,7 @@ namespace Bit.iOS.Core.Utilities
        }

        public static NSDictionary<KTo,VTo> ToNSDictionary<KFrom,VFrom,KTo,VTo>(this Dictionary<KFrom, VFrom> dict, Func<KFrom, KTo> keyConverter, Func<VFrom, VTo> valueConverter)
+            where KFrom : notnull
            where KTo : NSObject
            where VTo : NSObject
        {
@@ -23,19 +20,20 @@ namespace Bit.iOS.Core.Utilities
            return NSDictionary<KTo, VTo>.FromObjectsAndKeys(NSValues, NSKeys, NSKeys.Count());
        }

-        public static Dictionary<string, object> ToDictionary(this NSDictionary<NSString, NSObject> nsDict)
+        public static Dictionary<string, object?> ToDictionary(this NSDictionary<NSString, NSObject> nsDict)
        {
-            return nsDict.ToDictionary(v => v?.ToString() as object);
+            return nsDict.ToDictionary(v => v?.ToString());
        }

-        public static Dictionary<string, object> ToDictionary(this NSDictionary<NSString, NSObject> nsDict, Func<NSObject, object> valueTransformer)
+        public static Dictionary<string, object?> ToDictionary(this NSDictionary<NSString, NSObject> nsDict, Func<NSObject, object?> valueTransformer)
        {
            return nsDict.ToDictionary(k => k.ToString(), v => valueTransformer(v));
        }

-        public static Dictionary<KTo, VTo> ToDictionary<KFrom, VFrom, KTo, VTo>(this NSDictionary<KFrom, VFrom> nsDict, Func<KFrom, KTo> keyConverter, Func<VFrom, VTo> valueConverter)
+        public static Dictionary<KTo, VTo?> ToDictionary<KFrom, VFrom, KTo, VTo>(this NSDictionary<KFrom, VFrom> nsDict, Func<KFrom, KTo> keyConverter, Func<VFrom, VTo?> valueConverter)
            where KFrom : NSObject
            where VFrom : NSObject
+            where KTo : notnull
        {
            var keys = nsDict.Keys.Select(k => keyConverter(k)).ToArray();
            var values = nsDict.Values.Select(v => valueConverter(v)).ToArray();
--- a/src/iOS.Core/Utilities/WCSessionManager.cs
+++ b/src/iOS.Core/Utilities/WCSessionManager.cs
@@ -1,14 +1,8 @@
-using System;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.Linq;
-using System.Threading.Tasks;
-using Bit.Core.Models.Domain;
+using System.Diagnostics;
 using Bit.Core.Services;
 using Bit.iOS.Core.Utilities;
 using Foundation;
 using Newtonsoft.Json;
-using ObjCRuntime;

 namespace WatchConnectivity
 {
@@ -17,35 +11,45 @@ namespace WatchConnectivity
        // Setup is converted from https://www.natashatherobot.com/watchconnectivity-say-hello-to-wcsession/ 
        // with some extra bits
        private static readonly WCSessionManager sharedManager = new WCSessionManager();
-        private static WCSession session = WCSession.IsSupported ? WCSession.DefaultSession : null;
+        private static WCSession? session = WCSession.IsSupported ? WCSession.DefaultSession : null;

-        public event WCSessionReceiveDataHandler OnApplicationContextUpdated;
-        public event WCSessionReceiveDataHandler OnMessagedReceived;
-        public delegate void WCSessionReceiveDataHandler(WCSession session, Dictionary<string, object> data);
+        public event WCSessionReceiveDataHandler? OnApplicationContextUpdated;
+        public event WCSessionReceiveDataHandler? OnMessagedReceived;
+        public delegate void WCSessionReceiveDataHandler(WCSession session, Dictionary<string, object?> data);

-        WCSessionUserInfoTransfer _transf;
+        WCSessionUserInfoTransfer? _transf;

-        private WCSession validSession
+        private WCSession? validSession
        {
            get
            {
+                if (session is null)
+                {
+                    return null;
+                }
+
                Debug.WriteLine($"Paired status:{(session.Paired ? '✓' : '✗')}\n");
                Debug.WriteLine($"Watch App Installed status:{(session.WatchAppInstalled ? '✓' : '✗')}\n");
                return (session.Paired && session.WatchAppInstalled) ? session : null;
            }
        }

-        private WCSession validReachableSession
+        private WCSession? validReachableSession
        {
            get
            {
+                if (session is null)
+                {
+                    return null;
+                }
+
                return session.Reachable ? validSession : null;
            }
        }

        public bool IsValidSession => validSession != null;

-        public bool IsSessionReachable => session.Reachable;
+        public bool IsSessionReachable => session?.Reachable ?? false;

        public bool IsSessionActivated => validSession?.ActivationState == WCSessionActivationState.Activated;

@@ -71,7 +75,7 @@ namespace WatchConnectivity

        public override void SessionReachabilityDidChange(WCSession session)
        {
-            Debug.WriteLine($"Watch connectivity Reachable:{(session.Reachable ? '✓' : '✗')}");
+            Debug.WriteLine($"Watch connectivity Reachable:{(session?.Reachable == true ? '✓' : '✗')}");
        }

        public void SendBackgroundHighPriorityMessage(NSDictionary<NSString, NSObject> applicationContext)
@@ -102,7 +106,7 @@ namespace WatchConnectivity

        public void SendBackgroundFifoHighPriorityMessage(Dictionary<string, object> message)
        {
-            if(validSession is null || validSession.ActivationState != WCSessionActivationState.Activated)
+            if (session is null || validSession is null || validSession.ActivationState != WCSessionActivationState.Activated)
            {
                return;
            }
@@ -112,6 +116,10 @@ namespace WatchConnectivity
            Debug.WriteLine("Started transferring user info");

            _transf = session.TransferUserInfo(message.ToNSDictionary());
+            if (_transf is null)
+            {
+                return;
+            }

            Task.Run(async () =>
            {
@@ -136,7 +144,7 @@ namespace WatchConnectivity
            if (OnApplicationContextUpdated != null)
            {
                var keys = applicationContext.Keys.Select(k => k.ToString()).ToArray();
-                var values = applicationContext.Values.Select(v => JsonConvert.DeserializeObject(v.ToString())).ToArray();
+                var values = applicationContext.Values.Select(v => v != null ? JsonConvert.DeserializeObject(v.ToString()) : null).ToArray();
                var dictionary = keys.Zip(values, (k, v) => new { Key = k, Value = v })
                                     .ToDictionary(x => x.Key, x => x.Value);

--- a/src/iOS.Core/Utilities/iOSCoreHelpers.cs
+++ b/src/iOS.Core/Utilities/iOSCoreHelpers.cs
@@ -116,9 +116,13 @@ namespace Bit.iOS.Core.Utilities
                ServiceContainer.Register<INativeLogService>("nativeLogService", new ConsoleLogService());
            }

-            ILogger logger = null;
-            if (ServiceContainer.Resolve<ILogger>("logger", true) == null)
+            ILogger? logger = null;
+            if (ServiceContainer.TryResolve<ILogger>(out var resolvedLogger))
            {
+                logger = resolvedLogger;
+            }
+            else
+            { 
 #if DEBUG
                logger = DebugLogger.Instance;
 #else
@@ -129,6 +133,12 @@ namespace Bit.iOS.Core.Utilities

            var preferencesStorage = new PreferencesStorageService(AppGroupId);
            var appGroupContainer = new NSFileManager().GetContainerUrl(AppGroupId);
+            if (appGroupContainer?.Path is null)
+            {
+                var nreAppGroupContainer = new NullReferenceException("appGroupContainer or its Path is null when registering local services");
+                logger!.Exception(nreAppGroupContainer);
+                throw nreAppGroupContainer;
+            }
            var liteDbStorage = new LiteDbStorageService(
                Path.Combine(appGroupContainer.Path, "Library", "bitwarden.db"));
            var localizeService = new LocalizeService();
@@ -187,14 +197,14 @@ namespace Bit.iOS.Core.Utilities
                ServiceContainer.Resolve<ILogger>()));
        }

-        public static void Bootstrap(Func<Task> postBootstrapFunc = null)
+        public static void Bootstrap(Func<Task>? postBootstrapFunc = null)
        {
            var locale = ServiceContainer.Resolve<IStateService>().GetLocale();
            (ServiceContainer.Resolve<II18nService>("i18nService") as MobileI18nService)
-                .Init(locale != null ? new System.Globalization.CultureInfo(locale) : null);
+                ?.Init(locale != null ? new System.Globalization.CultureInfo(locale) : null);
            ServiceContainer.Resolve<IAuthService>("authService").Init();
            (ServiceContainer.
-                Resolve<IPlatformUtilsService>("platformUtilsService") as MobilePlatformUtilsService).Init();
+                Resolve<IPlatformUtilsService>("platformUtilsService") as MobilePlatformUtilsService)?.Init();

            var accountsManager = new AccountsManager(
                ServiceContainer.Resolve<IBroadcasterService>("broadcasterService"),
@@ -231,20 +241,31 @@ namespace Bit.iOS.Core.Utilities
                if (message.Command == "showDialog")
                {
                    var details = message.Data as DialogDetails;
+                    if (details is null)
+                    {
+                        return;
+                    }
                    var confirmText = string.IsNullOrWhiteSpace(details.ConfirmText) ?
                        AppResources.Ok : details.ConfirmText;

                    NSRunLoop.Main.BeginInvokeOnMainThread(async () =>
                    {
-                        var result = await deviceActionService.DisplayAlertAsync(details.Title, details.Text,
-                           details.CancelText, confirmText);
-                        var confirmed = result == details.ConfirmText;
-                        messagingService.Send("showDialogResolve", new Tuple<int, bool>(details.DialogId, confirmed));
+                        try
+                        {
+                            var result = await deviceActionService.DisplayAlertAsync(details.Title, details.Text,
+                               details.CancelText, confirmText);
+                            var confirmed = result == details.ConfirmText;
+                            messagingService.Send("showDialogResolve", new Tuple<int, bool>(details.DialogId, confirmed));
+                        }
+                        catch (Exception ex)
+                        {
+                            LoggerHelper.LogEvenIfCantBeResolved(ex);
+                        }
                    });
                }
-                else if (message.Command == "listenYubiKeyOTP")
+                else if (message.Command == "listenYubiKeyOTP" && message.Data is bool listen)
                {
-                    ListenYubiKey((bool)message.Data, deviceActionService, nfcSession, nfcDelegate);
+                    ListenYubiKey(listen, deviceActionService, nfcSession, nfcDelegate);
                }
            });
        }
@@ -268,29 +289,36 @@ namespace Bit.iOS.Core.Utilities
            }
        }

-        private static async Task BootstrapAsync(Func<Task> postBootstrapFunc = null)
+        private static async Task BootstrapAsync(Func<Task>? postBootstrapFunc = null)
        {
-            await ServiceContainer.Resolve<IEnvironmentService>("environmentService").SetUrlsFromStorageAsync();
-
-            InitializeAppSetup();
-            // TODO: Update when https://github.com/bitwarden/mobile/pull/1662 gets merged
-            var deleteAccountActionFlowExecutioner = new DeleteAccountActionFlowExecutioner(
-                ServiceContainer.Resolve<IApiService>("apiService"),
-                ServiceContainer.Resolve<IMessagingService>("messagingService"),
-                ServiceContainer.Resolve<IPlatformUtilsService>("platformUtilsService"),
-                ServiceContainer.Resolve<IDeviceActionService>("deviceActionService"),
-                ServiceContainer.Resolve<ILogger>("logger"));
-            ServiceContainer.Register<IDeleteAccountActionFlowExecutioner>("deleteAccountActionFlowExecutioner", deleteAccountActionFlowExecutioner);
-
-            var verificationActionsFlowHelper = new VerificationActionsFlowHelper(
-                ServiceContainer.Resolve<IPasswordRepromptService>("passwordRepromptService"),
-                ServiceContainer.Resolve<ICryptoService>("cryptoService"),
-                ServiceContainer.Resolve<IUserVerificationService>());
-            ServiceContainer.Register<IVerificationActionsFlowHelper>("verificationActionsFlowHelper", verificationActionsFlowHelper);
-
-            if (postBootstrapFunc != null)
+            try
            {
-                await postBootstrapFunc.Invoke();
+                await ServiceContainer.Resolve<IEnvironmentService>("environmentService").SetUrlsFromStorageAsync();
+
+                InitializeAppSetup();
+                // TODO: Update when https://github.com/bitwarden/mobile/pull/1662 gets merged
+                var deleteAccountActionFlowExecutioner = new DeleteAccountActionFlowExecutioner(
+                    ServiceContainer.Resolve<IApiService>("apiService"),
+                    ServiceContainer.Resolve<IMessagingService>("messagingService"),
+                    ServiceContainer.Resolve<IPlatformUtilsService>("platformUtilsService"),
+                    ServiceContainer.Resolve<IDeviceActionService>("deviceActionService"),
+                    ServiceContainer.Resolve<ILogger>("logger"));
+                ServiceContainer.Register<IDeleteAccountActionFlowExecutioner>("deleteAccountActionFlowExecutioner", deleteAccountActionFlowExecutioner);
+
+                var verificationActionsFlowHelper = new VerificationActionsFlowHelper(
+                    ServiceContainer.Resolve<IPasswordRepromptService>("passwordRepromptService"),
+                    ServiceContainer.Resolve<ICryptoService>("cryptoService"),
+                    ServiceContainer.Resolve<IUserVerificationService>());
+                ServiceContainer.Register<IVerificationActionsFlowHelper>("verificationActionsFlowHelper", verificationActionsFlowHelper);
+
+                if (postBootstrapFunc != null)
+                {
+                    await postBootstrapFunc.Invoke();
+                }
+            }
+            catch (Exception ex)
+            {
+                LoggerHelper.LogEvenIfCantBeResolved(ex);
            }
        }

--- a/src/iOS.Core/Views/ExtensionSearchDelegate.cs
+++ b/src/iOS.Core/Views/ExtensionSearchDelegate.cs
@@ -1,6 +1,4 @@
-using System;
-using System.Threading;
-using System.Threading.Tasks;
+using Bit.Core.Services;
 using Foundation;
 using UIKit;

@@ -9,7 +7,7 @@ namespace Bit.iOS.Core.Views
    public class ExtensionSearchDelegate : UISearchBarDelegate
    {
        private readonly UITableView _tableView;
-        private CancellationTokenSource _filterResultsCancellationTokenSource;
+        private CancellationTokenSource? _filterResultsCancellationTokenSource;

        public ExtensionSearchDelegate(UITableView tableView)
        {
@@ -23,25 +21,34 @@ namespace Bit.iOS.Core.Views
            {
                NSRunLoop.Main.BeginInvokeOnMainThread(async () =>
                {
-                    if (!string.IsNullOrWhiteSpace(searchText))
-                    {
-                        await Task.Delay(300);
-                        if (searchText != searchBar.Text)
-                        {
-                            return;
-                        }
-                        else
-                        {
-                            _filterResultsCancellationTokenSource?.Cancel();
-                        }
-                    }
                    try
                    {
-                        ((ExtensionTableSource)_tableView.Source).FilterResults(searchText, cts.Token);
-                        _tableView.ReloadData();
+                        if (!string.IsNullOrWhiteSpace(searchText))
+                        {
+                            await Task.Delay(300);
+                            if (searchText != searchBar.Text)
+                            {
+                                return;
+                            }
+                            else
+                            {
+                                _filterResultsCancellationTokenSource?.Cancel();
+                            }
+                        }
+                        try
+                        {
+                            ((ExtensionTableSource)_tableView.Source).FilterResults(searchText, cts.Token);
+                            _tableView.ReloadData();
+                        }
+                        catch (OperationCanceledException) { }
+                        _filterResultsCancellationTokenSource = cts;
+                    }
+                    catch (Exception ex)
+                    {
+                        LoggerHelper.LogEvenIfCantBeResolved(ex);
+                        _filterResultsCancellationTokenSource?.Cancel();
+                        cts?.Cancel();
                    }
-                    catch (OperationCanceledException) { }
-                    _filterResultsCancellationTokenSource = cts;
                });
            }, cts.Token);
        }