Address Copilot AI review: SongPreview playback, WebSocket UTF-8/boundary/Dispose

Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/a7e9baba-d7f9-4d66-bac6-a6540c230360

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>

Author: Copilot

README.md

@@ -189,6 +189,8 @@ This fork includes several hardening fixes on top of upstream:
 - **JNI surface safety** — proper lifecycle management with atomic swaps and timeouts to prevent race conditions between Android surface creation and destruction
 - **Trimmer-safe builds** — critical reflection-heavy assemblies are protected from .NET IL trimming so release builds behave the same as debug
 - **Architecture-correct native libraries (v144+)** — `osu.Android.props` strips desktop runtime `.so` files (`runtimes/{linux,osx,ios,maccatalyst,win,…}-*/native/`) from the Android publish set and only marks Android-RID assets as `AssetType=native`, so the proper Android arm64 BASS libraries from `ppy.osu.Framework.Android`'s AAR (`jni/arm64-v8a/`) always win over the desktop `.so` files transitively pulled in by `ppy.osu.Framework.NativeLibs`. The release workflow scans every shipped `libbass*.so` for `GLIBC_*` versioned symbols (only present in glibc-linked Linux ELFs) and fails the build if any are found, so an architecture mismatch can never reach a release.
+- **IPC / WebSocket hardening (v144+)** — desktop external-integrations server (env-var gated, `localhost`-only) tightened on top of upstream: `WebSocketChannel` now uses a strict `UTF8Encoding(throwOnInvalidBytes: true)` decoder so malformed payloads are rejected with `InvalidPayloadData` instead of being silently replaced with `U+FFFD`, and the message-size guard accepts payloads of exactly `max_message_size` bytes (was off-by-one); `WebSocketServer.Dispose()` now cancels the request loop and waits briefly for it to exit before tearing down the cancellation/reset-event handles to avoid an `ObjectDisposedException` race on shutdown; `OsuWebSocketProvider.Dispose()` swaps the server reference under a local, properly disposes the bounded `CancellationTokenSource` via `using`, and always disposes the `WebSocketServer` in a `finally` so listener handles can't leak across screen transitions.
+- **Ranked-play song-preview playback (v144+)** — restored the `Enabled`/`CardHovered` → `PreviewTrack.Start()/Stop()` wiring on `RankedPlayCard.SongPreviewContainer` that was lost in upstream's "playback rewrite" merge. The bind now happens in the `LoadComponentAsync` continuation (so previews never race the track's async load), with both bindables driving a single `updatePlaybackState()` callback.
 
 ---
 

osu.Desktop/IPC/OsuWebSocketProvider.cs

@@ -63,12 +63,23 @@ protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
 
-            if (server?.IsRunning == true)
+            var localServer = server;
+            server = null;
+
+            if (localServer == null)
+                return;
+
+            try
+            {
+                if (localServer.IsRunning)
+                {
+                    using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
+                    localServer.StopAsync(cts.Token).WaitSafely();
+                }
+            }
+            finally
             {
-                var cts = new CancellationTokenSource();
-                cts.CancelAfter(TimeSpan.FromSeconds(10));
-                server.StopAsync(cts.Token).WaitSafely();
-                server = null;
+                localServer.Dispose();
             }
         }
     }

osu.Game/IPC/WebSocketChannel.cs

@@ -23,6 +23,10 @@ public sealed class WebSocketChannel : IDisposable
         private readonly byte[] receiveBuffer = new byte[max_message_size];
         private int currentBufferPosition;
 
+        // strict UTF-8 decoder so malformed payloads throw `DecoderFallbackException` rather than being
+        // silently replaced with U+FFFD (which would defeat the `InvalidPayloadData` close path below).
+        private static readonly Encoding strict_utf8 = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true);
+
         private readonly WebSocket webSocket;
         private Task? readWriteTask;
         private readonly CancellationTokenSource runningTokenSource = new CancellationTokenSource();
@@ -100,7 +104,7 @@ private async Task readWriteLoop()
                     return;
                 }
 
-                if (currentBufferPosition >= max_message_size)
+                if (currentBufferPosition > max_message_size)
                 {
                     await webSocket.CloseOutputAsync(WebSocketCloseStatus.MessageTooBig, $@"Exceeded maximum message size of {max_message_size} bytes.", token).ConfigureAwait(false);
                     ClosedPrematurely?.Invoke();
@@ -113,9 +117,9 @@ private async Task readWriteLoop()
 
                     try
                     {
-                        message = Encoding.UTF8.GetString(receiveBuffer, 0, currentBufferPosition);
+                        message = strict_utf8.GetString(receiveBuffer, 0, currentBufferPosition);
                     }
-                    catch (ArgumentException)
+                    catch (DecoderFallbackException)
                     {
                         await webSocket.CloseOutputAsync(WebSocketCloseStatus.InvalidPayloadData, @"UTF-8 encoded strings expected.", token).ConfigureAwait(false);
                         ClosedPrematurely?.Invoke();

osu.Game/IPC/WebSocketServer.cs

@@ -265,11 +265,34 @@ public void Dispose()
 
             isDisposed = true;
 
+            // ensure the request loop is unblocked and stops touching `runningTokenSource`/`contextResetEvent`
+            // before we dispose them. callers that didn't call `StopAsync()` first would otherwise race the loop
+            // and trip `ObjectDisposedException` on shutdown.
+            try
+            {
+                if (!runningTokenSource.IsCancellationRequested)
+                    runningTokenSource.Cancel();
+            }
+            catch (ObjectDisposedException)
+            {
+            }
+
             // no clue why this isn't accessible without casting.
             // sidebar: `Stop()` unregisters addresses on Windows, but `Abort()` doesn't!
             // this `Dispose()` implementation calls the former.
             (listener as IDisposable)?.Dispose();
 
+            // give the request loop a brief opportunity to exit gracefully before we yank the wait handles out
+            // from under it. this is best-effort; we don't want `Dispose()` to ever block for long.
+            try
+            {
+                handleRequestTask?.Wait(TimeSpan.FromSeconds(2));
+            }
+            catch
+            {
+                // any cancellation/aggregate exceptions here are expected during shutdown.
+            }
+
             foreach (var channel in channels.Values)
                 channel.Dispose();
 

osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Card/RankedPlayCard.SongPreview.cs

@@ -97,11 +97,24 @@ public void LoadPreview(APIBeatmap beatmap)
                     {
                         TrackRunning = { BindTarget = trackRunning }
                     });
+
+                    // bind start/stop to hover + enable state once the track has actually loaded,
+                    // to avoid attempting to start playback while the track is still being prepared in flaky network conditions.
+                    Enabled.BindValueChanged(_ => updatePlaybackState());
+                    CardHovered.BindValueChanged(_ => updatePlaybackState(), true);
                 });
             }
 
+            private void updatePlaybackState()
+            {
+                if (previewTrack == null)
+                    return;
 
-            private void startPreviewIfAvailable() => previewTrack?.Start();
+                if (Enabled.Value && CardHovered.Value)
+                    previewTrack.Start();
+                else
+                    previewTrack.Stop();
+            }
 
             #region IBeatSyncProvider implementation