Merge pull request #53 from peppy/disallow-duplicate-filenames

Ensure that filenames are globally unique

Author: bdach

osu.Server.BeatmapSubmission.Tests/BeatmapSubmissionControllerTest.cs

@@ -1049,6 +1049,37 @@ await db.ExecuteAsync(
             Assert.Contains("Beatmap has invalid ID inside", (await response.Content.ReadFromJsonAsync<ErrorResponse>())!.Error);
         }
 
+        [Fact]
+        public async Task TestUploadFullPackage_FailsIfDuplicateFilename()
+        {
+            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[] { 557815, 557814, 557821, 557816, 557817, 557818, 557812, 557810, 557811, 557820, 557813, 557819 })
+                await db.ExecuteAsync(@"INSERT INTO `osu_beatmaps` (`beatmap_id`, `user_id`, `beatmapset_id`, `approved`) VALUES (@beatmapId, 1000, 241526, -1)", new { beatmapId = beatmapId });
+
+            // A different beatmap exists with a filename from the upload.
+            // Filenames should be unique globally.
+            await db.ExecuteAsync(@"INSERT INTO `osu_beatmaps` (`beatmap_id`, `filename`) VALUES (@beatmapId, @filename)", new { beatmapId = 123456, filename = "Soleily - Renatus (test) [Platter].osu" });
+
+            var request = new HttpRequestMessage(HttpMethod.Put, "/beatmapsets/241526");
+
+            using var content = new MultipartFormDataContent($"{Guid.NewGuid()}----");
+            using var stream = TestResources.GetResource(osz_filename)!;
+            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("Filename already exists as part of different beatmap set", (await response.Content.ReadFromJsonAsync<ErrorResponse>())!.Error);
+        }
+
         [Theory]
         [InlineData("../suspicious")]
         [InlineData("..\\suspicious")]

osu.Server.BeatmapSubmission/BeatmapSubmissionController.cs

@@ -438,7 +438,12 @@ private async Task<bool> updateBeatmapSetFromArchiveAsync(osu_beatmapset beatmap
 
                 if (packageFile.BeatmapContent is BeatmapContent content)
                 {
-                    if (!beatmapIds.Remove((uint)content.Beatmap.BeatmapInfo.OnlineID))
+                    uint beatmapId = (uint)content.Beatmap.BeatmapInfo.OnlineID;
+
+                    if (await db.FilenameExistsForDifferentBeatmapAsync(beatmapId, packageFile.VersionFile.filename, transaction))
+                        throw new InvariantException($"Filename already exists as part of different beatmap set ({packageFile.VersionFile.filename})", LogLevel.Warning);
+
+                    if (!beatmapIds.Remove(beatmapId))
                         throw new InvariantException($"Beatmap has invalid ID inside ({packageFile.VersionFile.filename}).", LogLevel.Warning);
 
                     await db.UpdateBeatmapAsync(content.GetDatabaseRow(), transaction);
@@ -456,7 +461,10 @@ private async Task<bool> updateBeatmapSetFromArchiveAsync(osu_beatmapset beatmap
             await beatmapStorage.StoreBeatmapSetAsync(beatmapSet.beatmapset_id, await beatmapStream.ReadAllBytesToArrayAsync(), parseResult);
 
             if (await db.IsBeatmapSetNominatedAsync(beatmapSet.beatmapset_id))
-                await sharedInterop.DisqualifyBeatmapSetAsync(beatmapSet.beatmapset_id, "This beatmap set was updated by the mapper after a nomination. Please ensure to re-check the beatmaps for new issues. If you are the mapper, please comment in this thread on what you changed.");
+            {
+                await sharedInterop.DisqualifyBeatmapSetAsync(beatmapSet.beatmapset_id,
+                    "This beatmap set was updated by the mapper after a nomination. Please ensure to re-check the beatmaps for new issues. If you are the mapper, please comment in this thread on what you changed.");
+            }
 
             await mirrorService.PurgeBeatmapSetAsync(db, beatmapSet.beatmapset_id);
 

osu.Server.BeatmapSubmission/DatabaseOperationExtensions.cs

@@ -394,6 +394,18 @@ public static Task InsertBeatmapsetVersionFileAsync(this MySqlConnection db, bea
                 transaction);
         }
 
+        public static async Task<bool> FilenameExistsForDifferentBeatmapAsync(this MySqlConnection db, uint beatmapId, string filename, MySqlTransaction? transaction = null)
+        {
+            return await db.QuerySingleAsync<uint>(
+                "SELECT COUNT(1) FROM osu_beatmaps WHERE filename = @filename AND beatmap_id != @beatmap_id AND deleted_at IS NULL",
+                new
+                {
+                    beatmap_id = beatmapId,
+                    filename = filename,
+                },
+                transaction) > 0;
+        }
+
         public static async Task<bool> IsBeatmapSetInProcessingQueueAsync(this MySqlConnection db, uint beatmapSetId, MySqlTransaction? transaction = null)
         {
             return await db.QuerySingleAsync<uint>(