Feature/eliminate warning and sophicated exception handling for building flatbuffer packets by jun92 · Pull Request #83 · jun92/ABMGS

jun92 · 2026-03-07T12:37:02Z

No description provided.

gemini-code-assist · 2026-03-07T12:37:20Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the stability and maintainability of the server by refining its error handling mechanisms and improving code quality. It introduces a dedicated exception type for FlatBuffer packet construction failures, centralizing error reporting for this critical component. Additionally, it addresses potential compiler warnings by ensuring proper initialization and validation of data transfer objects, contributing to a cleaner and more robust codebase.

Highlights

Warning Elimination: Addressed potential nullable reference type warnings by applying [Required] attributes and initializing properties to String.Empty in DTOs for Google and Syncnet authentication configurations and requests.
Custom Exception: Introduced a new custom exception, FlatBufferPacketBuildException, to specifically handle errors occurring during the FlatBuffer packet building process.
Centralized Exception Handling: Implemented robust exception handling in SyncnetPacketBuilder to catch general exceptions during packet construction and re-throw them as FlatBufferPacketBuildException.
Exception Integration: Integrated the new FlatBufferPacketBuildException into the GameSessionService to log critical errors when FlatBuffer packet building fails within the game loop.
Project Structure: Added an Exceptions folder to the project to organize custom exception classes.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

Source/ABMGS.Serverv2.Package/ABMGS.Serverv2.Package.csproj
- Added a new Exceptions folder.
Source/ABMGS.Serverv2.Package/Authentication/GooglePlay/GoogleAuthentication.cs
- Added using System.ComponentModel.DataAnnotations;.
- Applied [Required] attribute and default String.Empty initialization to properties in GoogleAuthenticationConfiguration, GoogleTokenRequest, and GoogleTokenResponse.
Source/ABMGS.Serverv2.Package/Authentication/SyncnetAuthProvider/SyncnetAuth.cs
- Added using System.ComponentModel.DataAnnotations;.
- Applied [Required] attribute and default string.Empty initialization to properties in SyncnetAuthenticationOptions.
Source/ABMGS.Serverv2.Package/Exceptions/FlatBufferPacketBuildException.cs
- Added a new custom exception class FlatBufferPacketBuildException.
Source/ABMGS.Serverv2.Package/Network/Sessions/GameSessionService.cs
- Added a catch block for FlatBufferPacketBuildException to log critical errors in RunGameLoop.
Source/ABMGS.Serverv2.Package/PacketBuilder/SyncnetPacketBuilder.cs
- Wrapped the Build method logic in a try-catch block to re-throw general exceptions as FlatBufferPacketBuildException.

Activity

No specific activity has been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request aims to improve exception handling for FlatBuffer packet building and resolve some compiler warnings. However, a significant security vulnerability was identified in the Google authentication logic where a new configuration instance is created instead of using the injected one, potentially breaking authentication or allowing unauthorized access. Additionally, a race condition and potential type cast exception were found in the packet builder's caching mechanism, which could lead to denial of service under high load. Beyond these security concerns, the new custom exception (FlatBufferPacketBuildException) is caught in a location where it will never be thrown, and a necessary using directive is missing, which will cause a compilation failure. Please also review suggestions for improving code consistency and project structure.

gemini-code-assist · 2026-03-07T12:40:02Z

+    [Required] public string ClientId { get; set; } = String.Empty;
+    [Required] public string ClientSecret { get; set; } = String.Empty;


A critical vulnerability exists here: the ValidateGoogleJwt method (line 132) incorrectly instantiates a new GoogleAuthenticationConfiguration object instead of using the injected _googleAuthenticationOptions field. This leads to Audience validation for Google ID tokens being performed against an empty string, which could result in either Denial of Service (all authentication attempts fail) or Authentication Bypass (tokens issued for other applications are accepted). Additionally, for consistency with other parts of the codebase (e.g., SyncnetAuth.cs) and C# style conventions, it's better to use string.Empty instead of String.Empty for these properties.

[Required] public string ClientId { get; set; } = string.Empty; [Required] public string ClientSecret { get; set; } = string.Empty;

gemini-code-assist · 2026-03-07T12:40:02Z

                }
+                catch(FlatBufferPacketBuildException e)
+                {
+                    _logger.LogCritical(e, "FlatBuffer Exception");
+                    return;


This catch block for FlatBufferPacketBuildException is misplaced. It is inside a try block that only wraps the SocketObject.ReceiveAsync() call, which will not throw this type of exception. The FlatBufferPacketBuildException is expected to be thrown from packetHandlingActor.PushRecievedData() on line 160. As it is, this exception will never be caught, leading to an unhandled exception. The try-catch logic should be moved to wrap the grain call on line 160.

gemini-code-assist · 2026-03-07T12:40:02Z

        {
-            var type = Assembly.GetExecutingAssembly().GetTypes().FirstOrDefault(
-                    t =>
-                        t.GetInterfaces().Any(i =>
-                            i.IsGenericType &&
-                            i.GetGenericTypeDefinition() == typeof(IPacketByteArrayBuilder<>) &&
-                            i.GetGenericArguments()[0] == typeof(TArgs)
-                        )
+            if (!_cache.TryGetValue(args.GetType(), out var builder))
+            {
+                var type = Assembly.GetExecutingAssembly().GetTypes().FirstOrDefault(
+                        t =>
+                            t.GetInterfaces().Any(i =>
+                                i.IsGenericType &&
+                                i.GetGenericTypeDefinition() == typeof(IPacketByteArrayBuilder<>) &&
+                                i.GetGenericArguments()[0] == typeof(TArgs)
+                            )

-                ) ?? throw new MissingMethodException($"No builder found for {typeof(TArgs).Name}");
-            builder = Activator.CreateInstance(type) ?? throw new BadImageFormatException();
-            _cache.Add(args.GetType(), builder);
+                    ) ?? throw new MissingMethodException($"No builder found for {typeof(TArgs).Name}");
+                builder = Activator.CreateInstance(type) ?? throw new BadImageFormatException();
+                _cache.Add(args.GetType(), builder);
+            }


The Build method uses a non-atomic check-and-set pattern on a ConcurrentDictionary (TryGetValue followed by Add). In a multi-threaded environment, this can lead to an ArgumentException if multiple threads attempt to add the same key simultaneously, causing the game session to terminate. Additionally, the cache uses args.GetType() as the key while resolving the builder based on typeof(TArgs). If the method is called with the same argument type but different generic type parameters, it may retrieve a cached builder that cannot be cast to the expected interface, leading to an InvalidCastException and session termination.

jun92 added 3 commits March 7, 2026 21:31

Suppress warnings

11c86c5

Add exception handling for packet building failture

75d7617

Move the exception class to a file

9a79e38

jun92 added this to the Prototype milestone Mar 7, 2026

jun92 self-assigned this Mar 7, 2026

jun92 added this to ABMGS Mar 7, 2026

jun92 added the bug Something isn't working label Mar 7, 2026

jun92 linked an issue Mar 7, 2026 that may be closed by this pull request

Eliminate warning and sophicated exception handling for building flatbuffer packets #82

Closed

github-project-automation Bot moved this to Backlog in ABMGS Mar 7, 2026

Fixed namespace

3c32cda

gemini-code-assist Bot reviewed Mar 7, 2026

View reviewed changes

Added Serializer decorator

7e2598d

jun92 mentioned this pull request Mar 10, 2026

![security-medium](https://www.gstatic.com/codereviewagent/security-medium-priority.svg) ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) #84

Open

jun92 merged commit e9a3bda into main Mar 10, 2026
3 checks passed

github-project-automation Bot moved this from Backlog to Done in ABMGS Mar 10, 2026

jun92 deleted the feature/eliminate-warning-and-sophicated-exception-handling-for-building-flatbuffer-packets branch March 10, 2026 06:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/eliminate warning and sophicated exception handling for building flatbuffer packets#83

Feature/eliminate warning and sophicated exception handling for building flatbuffer packets#83
jun92 merged 5 commits into
mainfrom
feature/eliminate-warning-and-sophicated-exception-handling-for-building-flatbuffer-packets

jun92 commented Mar 7, 2026

Uh oh!

gemini-code-assist Bot commented Mar 7, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

gemini-code-assist Bot Mar 7, 2026

Uh oh!

gemini-code-assist Bot Mar 7, 2026

Uh oh!

gemini-code-assist Bot Mar 7, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		[Required] public string ClientId { get; set; } = String.Empty;
		[Required] public string ClientSecret { get; set; } = String.Empty;

Conversation

jun92 commented Mar 7, 2026

Uh oh!

gemini-code-assist Bot commented Mar 7, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist Bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant