Skip to content

Validate MessagePack enum values#38204

Open
smoogipoo wants to merge 2 commits into
ppy:masterfrom
smoogipoo:messagepack-enum-validation
Open

Validate MessagePack enum values#38204
smoogipoo wants to merge 2 commits into
ppy:masterfrom
smoogipoo:messagepack-enum-validation

Conversation

@smoogipoo

Copy link
Copy Markdown
Contributor

No description provided.

@smoogipoo smoogipoo added the area:online functionality Deals with online fetching / sending but don't change much on a surface UI level. label Jul 1, 2026
Comment on lines -55 to +64
baseFormatter = StandardResolver.Instance.GetFormatter<TBase>();
baseFormatter = StandardResolver.Instance.GetFormatterWithVerify<TBase>();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "WithVerify" part of this checks that the formatter is not null. Was just looking through the MsgPack source and this is kind of how they do it, plus I needed the non-nullability.

Comment on lines +108 to +109
if (Enum.IsDefined(typeof(T), value))
return;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Flags] enums need to be exempted from this because there not being "defined" is actually a feature if you have a union of 2 or more flags.

This is actually relevant because we have one of those that goes over the wire to spectator server and it's ReplayButtonState.

diff --git a/osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs b/osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs
index 6891b214d0..08e3e3a660 100644
--- a/osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs
+++ b/osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs
@@ -7,6 +7,7 @@
 using osu.Game.Online;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Multiplayer.MatchTypes.TeamVersus;
+using osu.Game.Replays.Legacy;
 
 namespace osu.Game.Tests.Online
 {
@@ -86,6 +87,9 @@ public void TestEnumValidation()
                 MessagePackSerializer.Deserialize<SimpleEnum?>(MessagePackSerializer.Serialize((SimpleEnum?)null, SignalRUnionWorkaroundResolver.OPTIONS), SignalRUnionWorkaroundResolver.OPTIONS));
             Assert.Throws<MessagePackSerializationException>(() =>
                 MessagePackSerializer.Deserialize<SimpleEnum?>(MessagePackSerializer.Serialize((SimpleEnum)100, SignalRUnionWorkaroundResolver.OPTIONS), SignalRUnionWorkaroundResolver.OPTIONS));
+
+            Assert.DoesNotThrow(() =>
+                MessagePackSerializer.Deserialize<ReplayButtonState>(MessagePackSerializer.Serialize(ReplayButtonState.Left1 | ReplayButtonState.Right2, SignalRUnionWorkaroundResolver.OPTIONS), SignalRUnionWorkaroundResolver.OPTIONS));
         }
 
         public enum SimpleEnum

The above fails when run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:online functionality Deals with online fetching / sending but don't change much on a surface UI level. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants