From 697b4c39ca618c51c624c59e39e734454db9a24b Mon Sep 17 00:00:00 2001
From: Stanley Dimant <root.darkarchon@outlook.com>
Date: Mon, 15 Aug 2022 02:22:02 +0200
Subject: [PATCH] add rate limiting for disconnects, logging exceptions and
 minor query optimizations for disconnects

---
 .../MareSynchronosServer/Hubs/MareHub.cs      | 25 +++++++++--------
 .../Throttling/SignalRLimitFilter.cs          | 27 ++++++++++++++-----
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/MareSynchronosServer/MareSynchronosServer/Hubs/MareHub.cs b/MareSynchronosServer/MareSynchronosServer/Hubs/MareHub.cs
index dab1b25..8fabac4 100644
--- a/MareSynchronosServer/MareSynchronosServer/Hubs/MareHub.cs
+++ b/MareSynchronosServer/MareSynchronosServer/Hubs/MareHub.cs
@@ -91,26 +91,29 @@ namespace MareSynchronosServer.Hubs
         {
             MareMetrics.Connections.Dec();
 
-            var user = await _dbContext.Users.AsNoTracking().SingleOrDefaultAsync(u => u.UID == AuthenticatedUserId);
+            var user = await _dbContext.Users.SingleOrDefaultAsync(u => u.UID == AuthenticatedUserId);
             if (user != null && !string.IsNullOrEmpty(user.CharacterIdentification))
             {
                 MareMetrics.AuthorizedConnections.Dec();
+
                 _logger.LogInformation("Disconnect from " + AuthenticatedUserId);
 
                 var otherUsers = await _dbContext.ClientPairs.AsNoTracking()
-                    .Include(u => u.User)
                     .Include(u => u.OtherUser)
-                    .Where(w => w.User.UID == user.UID && !w.IsPaused)
-                    .Where(w => !string.IsNullOrEmpty(w.OtherUser.CharacterIdentification))
-                    .Select(e => e.OtherUser).ToListAsync();
-                var otherEntries = await _dbContext.ClientPairs.AsNoTracking().Include(u => u.User)
-                    .Where(u => otherUsers.Any(e => e == u.User) && u.OtherUser.UID == user.UID && !u.IsPaused).ToListAsync();
-                await Clients.Users(otherEntries.Select(e => e.User.UID)).SendAsync(Api.OnUserRemoveOnlinePairedPlayer, user.CharacterIdentification);
+                    .Where(self => self.UserUID == user.UID && !self.IsPaused && self.OtherUser.CharacterIdentification != null)
+                    .Select(e => e.OtherUserUID)
+                    .ToListAsync();
 
-                var notUploadedFiles = _dbContext.Files.Where(f => !f.Uploaded && f.Uploader.UID == user.UID).ToList();
-                _dbContext.RemoveRange(notUploadedFiles);
+                var otherEntries = await _dbContext.ClientPairs.AsNoTracking()
+                    .Where(other => otherUsers.Contains(other.UserUID) && other.OtherUserUID == user.UID && !other.IsPaused)
+                    .Select(u => u.UserUID)
+                    .ToListAsync();
 
-                (await _dbContext.Users.SingleAsync(u => u.UID == AuthenticatedUserId)).CharacterIdentification = null;
+                await Clients.Users(otherEntries).SendAsync(Api.OnUserRemoveOnlinePairedPlayer, user.CharacterIdentification);
+
+                _dbContext.RemoveRange(_dbContext.Files.Where(f => !f.Uploaded && f.Uploader.UID == user.UID));
+
+                user.CharacterIdentification = null;
                 await _dbContext.SaveChangesAsync();
             }
 
diff --git a/MareSynchronosServer/MareSynchronosServer/Throttling/SignalRLimitFilter.cs b/MareSynchronosServer/MareSynchronosServer/Throttling/SignalRLimitFilter.cs
index a28d138..37ac72d 100644
--- a/MareSynchronosServer/MareSynchronosServer/Throttling/SignalRLimitFilter.cs
+++ b/MareSynchronosServer/MareSynchronosServer/Throttling/SignalRLimitFilter.cs
@@ -15,7 +15,8 @@ public class SignalRLimitFilter : IHubFilter
     private readonly IRateLimitProcessor _processor;
     private readonly IHttpContextAccessor accessor;
     private readonly ILogger<SignalRLimitFilter> logger;
-    private static SemaphoreSlim ConnectionLimiterSemaphore = new SemaphoreSlim(20);
+    private static SemaphoreSlim ConnectionLimiterSemaphore = new(20);
+    private static SemaphoreSlim DisconnectLimiterSemaphore = new(20);
 
     public SignalRLimitFilter(
         IOptions<IpRateLimitOptions> options, IProcessingStrategy processing, IIpPolicyStore policyStore, IHttpContextAccessor accessor, ILogger<SignalRLimitFilter> logger)
@@ -76,20 +77,34 @@ public class SignalRLimitFilter : IHubFilter
 
         try
         {
-            await Task.Delay(100);
+            await Task.Delay(250);
             await next(context);
         }
-        catch { }
+        catch (Exception ex)
+        {
+            logger.LogWarning(ex, "Error on OnConnectedAsync");
+        }
         finally
         {
             ConnectionLimiterSemaphore.Release();
         }
     }
 
-    // Optional method
-    public Task OnDisconnectedAsync(
+    public async Task OnDisconnectedAsync(
         HubLifetimeContext context, Exception exception, Func<HubLifetimeContext, Exception, Task> next)
     {
-        return next(context, exception);
+        await DisconnectLimiterSemaphore.WaitAsync();
+        try
+        {
+            await next(context, exception);
+        }
+        catch
+        {
+            logger.LogWarning(exception, "Error on OnDisconnectedAsync");
+        }
+        finally
+        {
+            DisconnectLimiterSemaphore.Release();
+        }
     }
 }