Skip to content

Validate MessagePack enum values #38204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
smoogipoo wants to merge 2 commits into
base: master
Choose a base branch
from
+82 −9
Open

Validate MessagePack enum values #38204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1a2252
Add failing tests
smoogipoo Jul 1, 2026
b7205d1
Add enum validation to MessagePack serializer
smoogipoo Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,28 @@ public void TestSerialiseUnionSucceedsWithWorkaround()
var deserialized = MessagePackSerializer.Deserialize<MatchUserState>(serialized, SignalRUnionWorkaroundResolver.OPTIONS);
ClassicAssert.True(deserialized is TeamVersusUserState);
}

[Test]
public void TestEnumValidation()
{
Assert.DoesNotThrow(() =>
MessagePackSerializer.Deserialize<SimpleEnum>(MessagePackSerializer.Serialize(SimpleEnum.Value, SignalRUnionWorkaroundResolver.OPTIONS), SignalRUnionWorkaroundResolver.OPTIONS));
Assert.Throws<MessagePackSerializationException>(() =>
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<SimpleEnum?>(MessagePackSerializer.Serialize(SimpleEnum.Value, SignalRUnionWorkaroundResolver.OPTIONS), SignalRUnionWorkaroundResolver.OPTIONS));
Assert.DoesNotThrow(() =>
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));
}

public enum SimpleEnum
{
Value
}
}
}
68 changes: 59 additions & 9 deletions osu.Game/Online/SignalRUnionWorkaroundResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

#nullable disable

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using MessagePack;
using MessagePack.Formatters;
using MessagePack.Resolvers;
using osu.Framework.Extensions.TypeExtensions;

namespace osu.Game.Online
{
Expand All @@ -22,6 +22,7 @@ public class SignalRUnionWorkaroundResolver : IFormatterResolver
MessagePackSerializerOptions.Standard.WithResolver(new SignalRUnionWorkaroundResolver());

private static readonly IReadOnlyDictionary<Type, IMessagePackFormatter> formatter_map = createFormatterMap();
private static readonly ConcurrentDictionary<Type, IMessagePackFormatter> enum_formatter_map = [];

private static IReadOnlyDictionary<Type, IMessagePackFormatter> createFormatterMap()
{
Expand All @@ -33,7 +34,7 @@ private static IReadOnlyDictionary<Type, IMessagePackFormatter> createFormatterM

return new Dictionary<Type, IMessagePackFormatter>(baseMap.Select(t =>
{
var formatter = (IMessagePackFormatter)Activator.CreateInstance(typeof(TypeRedirectingFormatter<,>).MakeGenericType(t.derivedType, t.baseType));
var formatter = (IMessagePackFormatter)Activator.CreateInstance(typeof(TypeRedirectingFormatter<,>).MakeGenericType(t.derivedType, t.baseType))!;
return new KeyValuePair<Type, IMessagePackFormatter>(t.derivedType, formatter);
}));
}
Expand All @@ -43,7 +44,15 @@ public IMessagePackFormatter<T> GetFormatter<T>()
if (formatter_map.TryGetValue(typeof(T), out var formatter))
return (IMessagePackFormatter<T>)formatter;

return StandardResolver.Instance.GetFormatter<T>();
if (typeof(T).IsEnum)
{
if (enum_formatter_map.TryGetValue(typeof(T), out formatter))
return (IMessagePackFormatter<T>)formatter;

return (IMessagePackFormatter<T>)(enum_formatter_map[typeof(T)] = (IMessagePackFormatter)Activator.CreateInstance(typeof(EnumFormatter<>).MakeGenericType(typeof(T)))!);
}

return StandardResolver.Instance.GetFormatterWithVerify<T>();
}

public class TypeRedirectingFormatter<TActual, TBase> : IMessagePackFormatter<TActual>
Expand All @@ -52,14 +61,55 @@ public class TypeRedirectingFormatter<TActual, TBase> : IMessagePackFormatter<TA

public TypeRedirectingFormatter()
{
baseFormatter = StandardResolver.Instance.GetFormatter<TBase>();
baseFormatter = StandardResolver.Instance.GetFormatterWithVerify<TBase>();
Comment on lines -55 to +64

@smoogipoo smoogipoo Jul 1, 2026

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.

}

public void Serialize(ref MessagePackWriter writer, TActual value, MessagePackSerializerOptions options)
=> baseFormatter.Serialize(ref writer, (TBase)(object)value!, StandardResolver.Options);

public TActual Deserialize(ref MessagePackReader reader, MessagePackSerializerOptions options)
=> (TActual)(object)baseFormatter.Deserialize(ref reader, StandardResolver.Options)!;
}

public class EnumFormatter<T> : IMessagePackFormatter<T>
where T : Enum
{
private readonly IMessagePackFormatter<T> formatter;

public EnumFormatter()
{
formatter = StandardResolver.Instance.GetFormatterWithVerify<T>();
}

public void Serialize(ref MessagePackWriter writer, TActual value, MessagePackSerializerOptions options) =>
baseFormatter.Serialize(ref writer, (TBase)(object)value, StandardResolver.Options);
public void Serialize(ref MessagePackWriter writer, T value, MessagePackSerializerOptions options)
{
EnumValueOutOfRangeException<T>.ThrowIfNotDefined(value);
formatter.Serialize(ref writer, value, StandardResolver.Options);
}

public TActual Deserialize(ref MessagePackReader reader, MessagePackSerializerOptions options) =>
(TActual)(object)baseFormatter.Deserialize(ref reader, StandardResolver.Options);
public T Deserialize(ref MessagePackReader reader, MessagePackSerializerOptions options)
{
T result = formatter.Deserialize(ref reader, StandardResolver.Options);
EnumValueOutOfRangeException<T>.ThrowIfNotDefined(result);
return result;
}
}

public class EnumValueOutOfRangeException<T> : Exception
where T : Enum
{
public EnumValueOutOfRangeException(T value)
: base($"Enum value '{value}' out of range for type '{typeof(T).ReadableName()}'")
{
}

public static void ThrowIfNotDefined(T value)
{
if (Enum.IsDefined(typeof(T), value))
return;
Comment on lines +108 to +109

@bdach bdach Jul 1, 2026

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.


throw new EnumValueOutOfRangeException<T>(value);
}
}
}
}
Loading