From 415a65bf59cdce4b8fab7d836fe3cdc4f7a2a168 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 17 Feb 2024 19:00:30 +0800 Subject: [PATCH 1/2] Add failing tests for beatmap inconsistencies --- .../Navigation/TestSceneScreenNavigation.cs | 63 +++++++++++++++++++ osu.Game/Tests/Visual/OsuGameTestScene.cs | 4 +- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 8ff4fd5ecf..7e42d4781d 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -4,6 +4,7 @@ #nullable disable using System; +using System.IO; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -18,6 +19,7 @@ using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Collections; using osu.Game.Configuration; +using osu.Game.Extensions; using osu.Game.Graphics.Containers; using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; @@ -221,6 +223,67 @@ namespace osu.Game.Tests.Visual.Navigation AddAssert("Overlay was shown", () => songSelect.ModSelectOverlay.State.Value == Visibility.Visible); } + [Test] + public void TestAttemptPlayBeatmapWrongHashFails() + { + Screens.Select.SongSelect songSelect = null; + + AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).GetResultSafely()); + PushAndConfirm(() => songSelect = new TestPlaySongSelect()); + AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); + + AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + + AddStep("change beatmap files", () => + { + foreach (var file in Game.Beatmap.Value.BeatmapSetInfo.Files.Where(f => Path.GetExtension(f.Filename) == ".osu")) + { + using (var stream = Game.Storage.GetStream(Path.Combine("files", file.File.GetStoragePath()), FileAccess.ReadWrite)) + stream.WriteByte(0); + } + }); + + AddStep("invalidate cache", () => + { + ((IWorkingBeatmapCache)Game.BeatmapManager).Invalidate(Game.Beatmap.Value.BeatmapSetInfo); + }); + + AddStep("select next difficulty", () => InputManager.Key(Key.Down)); + AddStep("press enter", () => InputManager.Key(Key.Enter)); + + AddUntilStep("wait for player loader", () => Game.ScreenStack.CurrentScreen is PlayerLoader); + AddUntilStep("wait for song select", () => songSelect.IsCurrentScreen()); + } + + [Test] + public void TestAttemptPlayBeatmapMissingFails() + { + Screens.Select.SongSelect songSelect = null; + + AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).GetResultSafely()); + PushAndConfirm(() => songSelect = new TestPlaySongSelect()); + AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); + + AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + + AddStep("delete beatmap files", () => + { + foreach (var file in Game.Beatmap.Value.BeatmapSetInfo.Files.Where(f => Path.GetExtension(f.Filename) == ".osu")) + Game.Storage.Delete(Path.Combine("files", file.File.GetStoragePath())); + }); + + AddStep("invalidate cache", () => + { + ((IWorkingBeatmapCache)Game.BeatmapManager).Invalidate(Game.Beatmap.Value.BeatmapSetInfo); + }); + + AddStep("select next difficulty", () => InputManager.Key(Key.Down)); + AddStep("press enter", () => InputManager.Key(Key.Enter)); + + AddUntilStep("wait for player loader", () => Game.ScreenStack.CurrentScreen is PlayerLoader); + AddUntilStep("wait for song select", () => songSelect.IsCurrentScreen()); + } + [Test] public void TestRetryCountIncrements() { diff --git a/osu.Game/Tests/Visual/OsuGameTestScene.cs b/osu.Game/Tests/Visual/OsuGameTestScene.cs index 6069fe4fb0..b86273b4a3 100644 --- a/osu.Game/Tests/Visual/OsuGameTestScene.cs +++ b/osu.Game/Tests/Visual/OsuGameTestScene.cs @@ -153,6 +153,8 @@ namespace osu.Game.Tests.Visual public new Bindable> SelectedMods => base.SelectedMods; + public new Storage Storage => base.Storage; + public new SpectatorClient SpectatorClient => base.SpectatorClient; // if we don't apply these changes, when running under nUnit the version that gets populated is that of nUnit. @@ -166,7 +168,7 @@ namespace osu.Game.Tests.Visual public TestOsuGame(Storage storage, IAPIProvider api, string[] args = null) : base(args) { - Storage = storage; + base.Storage = storage; API = api; } From 7f82f103171223518fe34341f07dd07ab4be6c8f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 17 Feb 2024 19:00:43 +0800 Subject: [PATCH 2/2] Fix beatmap potentially loading in a bad state Over-caching could mean that a beatmap could load and cause a late crash. Let's catch it early to avoid such a crash occurring. --- osu.Game/Beatmaps/WorkingBeatmapCache.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmapCache.cs b/osu.Game/Beatmaps/WorkingBeatmapCache.cs index 74a85cde7c..8af74d11d8 100644 --- a/osu.Game/Beatmaps/WorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/WorkingBeatmapCache.cs @@ -9,6 +9,7 @@ using System.Linq; using JetBrains.Annotations; using osu.Framework.Audio; using osu.Framework.Audio.Track; +using osu.Framework.Extensions; using osu.Framework.Graphics.Rendering; using osu.Framework.Graphics.Rendering.Dummy; using osu.Framework.Graphics.Textures; @@ -143,8 +144,6 @@ namespace osu.Game.Beatmaps { string fileStorePath = BeatmapSetInfo.GetPathForFile(BeatmapInfo.Path); - // TODO: check validity of file - var stream = GetStream(fileStorePath); if (stream == null) @@ -153,6 +152,12 @@ namespace osu.Game.Beatmaps return null; } + if (stream.ComputeMD5Hash() != BeatmapInfo.MD5Hash) + { + Logger.Log($"Beatmap failed to load (file {BeatmapInfo.Path} does not have the expected hash).", level: LogLevel.Error); + return null; + } + using (var reader = new LineBufferedReader(stream)) return Decoder.GetDecoder(reader).Decode(reader); }