Merge pull request #36 from bdach/missing-set-id-crash

Fix hard crash when BeatmapSetID is completely missing from an `.osu` file

Author: peppy

osu.Server.BeatmapSubmission.Tests/BeatmapSubmissionControllerTest.cs

@@ -1086,6 +1086,31 @@ public async Task TestUploadFullPackage_FailsIfCreatorDoesNotMatchHostUsername()
             Assert.Contains("At least one difficulty has a specified creator that isn't the beatmap host's username.", (await response.Content.ReadFromJsonAsync<ErrorResponse>())!.Error);
         }
 
+        [Fact]
+        public async Task TestUploadFullPackage_FailsIfBeatmapSetIDMissing()
+        {
+            using var db = await DatabaseAccess.GetConnectionAsync();
+            await db.ExecuteAsync("INSERT INTO `phpbb_users` (`user_id`, `username`, `username_clean`, `country_acronym`, `user_permissions`, `user_sig`, `user_occ`, `user_interests`) VALUES (1000, 'test', 'test', 'JP', '', '', '', '')");
+
+            await db.ExecuteAsync(@"INSERT INTO `osu_beatmapsets` (`beatmapset_id`, `user_id`, `creator`, `approved`, `thread_id`, `active`, `submit_date`) VALUES (241526, 1000, 'test user', -1, 0, -1, CURRENT_TIMESTAMP)");
+
+            foreach (uint beatmapId in new uint[] { 557810 })
+                await db.ExecuteAsync(@"INSERT INTO `osu_beatmaps` (`beatmap_id`, `user_id`, `beatmapset_id`, `approved`) VALUES (@beatmapId, 1000, 241526, -1)", new { beatmapId = beatmapId });
+
+            var request = new HttpRequestMessage(HttpMethod.Put, "/beatmapsets/241526");
+
+            using var content = new MultipartFormDataContent($"{Guid.NewGuid()}----");
+            using var stream = TestResources.GetResource("no-beatmap-set-id.osz")!;
+            content.Add(new StreamContent(stream), "beatmapArchive", osz_filename);
+            request.Content = content;
+            request.Headers.Add(HeaderBasedAuthenticationHandler.USER_ID_HEADER, "1000");
+
+            var response = await Client.SendAsync(request);
+            Assert.False(response.IsSuccessStatusCode);
+            Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode);
+            Assert.Contains("Beatmap has no beatmap set ID inside", (await response.Content.ReadFromJsonAsync<ErrorResponse>())!.Error);
+        }
+
         [Fact]
         public async Task TestUploadFullPackage_FailsIfNonUnicodeMetadataHasMetadataChars()
         {

osu.Server.BeatmapSubmission.Tests/Resources/no-beatmap-set-id.osz

@@ -49,7 +49,13 @@ public BeatmapPackageParseResult Parse(uint beatmapSetId, ArchiveReader archiveR
                 {
                     beatmapContent = getBeatmapContent(filename, stream);
 
-                    if (beatmapContent.Beatmap.BeatmapInfo.BeatmapSet!.OnlineID != beatmapSetId)
+                    // slightly unfortunate but possible if the set ID is completely missing.
+                    // refer to: https://github.com/ppy/osu/blob/a556049c1b29808b5dd9a3d65c7efb5d02315a0c/osu.Game/Beatmaps/Beatmap.cs#L45-L58,
+                    // https://github.com/ppy/osu/blob/a556049c1b29808b5dd9a3d65c7efb5d02315a0c/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs#L377-L379
+                    if (beatmapContent.Beatmap.BeatmapInfo.BeatmapSet == null)
+                        throw new InvariantException($"Beatmap has no beatmap set ID inside ({filename})");
+
+                    if (beatmapContent.Beatmap.BeatmapInfo.BeatmapSet.OnlineID != beatmapSetId)
                         throw new InvariantException($"Beatmap has invalid beatmap set ID inside ({filename})");
 
                     if (!MetadataUtils.IsRomanised(beatmapContent.Beatmap.BeatmapInfo.DifficultyName))