Merge pull request #15447 from peppy/fix-invalid-characters-zip-export

Fix beatmaps being exported with malformed filenames inside `.osz` zip files

osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs

@@ -846,6 +846,42 @@ namespace osu.Game.Tests.Beatmaps.IO
             }
         }
 
+        // TODO: needs to be pulled across to realm implementation when this file is nuked.
+        [Test]
+        public void TestSaveRemovesInvalidCharactersFromPath()
+        {
+            // unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here.
+            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportBeatmapTest)))
+            {
+                try
+                {
+                    var osu = LoadOsuIntoHost(host);
+
+                    var manager = osu.Dependencies.Get<BeatmapManager>();
+
+                    var working = manager.CreateNew(new OsuRuleset().RulesetInfo, User.SYSTEM_USER);
+
+                    var beatmap = working.Beatmap;
+
+                    beatmap.BeatmapInfo.Version = "difficulty";
+                    beatmap.BeatmapInfo.Metadata = new BeatmapMetadata
+                    {
+                        Artist = "Artist/With\\Slashes",
+                        Title = "Title",
+                        AuthorString = "mapper",
+                    };
+
+                    manager.Save(beatmap.BeatmapInfo, working.Beatmap);
+
+                    Assert.AreEqual("Artist_With_Slashes - Title (mapper) [difficulty].osu", beatmap.BeatmapInfo.Path);
+                }
+                finally
+                {
+                    host.Exit();
+                }
+            }
+        }
+
         [Test]
         public void TestCreateNewEmptyBeatmap()
         {

osu.Game/Beatmaps/BeatmapManager.cs

@@ -114,7 +114,8 @@ namespace osu.Game.Beatmaps
         /// <param name="info">The <see cref="BeatmapInfo"/> to save the content against. The file referenced by <see cref="BeatmapInfo.Path"/> will be replaced.</param>
         /// <param name="beatmapContent">The <see cref="IBeatmap"/> content to write.</param>
         /// <param name="beatmapSkin">The beatmap <see cref="ISkin"/> content to write, null if to be omitted.</param>
-        public virtual void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null) => beatmapModelManager.Save(info, beatmapContent, beatmapSkin);
+        public virtual void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null) =>
+            beatmapModelManager.Save(info, beatmapContent, beatmapSkin);
 
         /// <summary>
         /// Returns a list of all usable <see cref="BeatmapSetInfo"/>s.

osu.Game/Beatmaps/BeatmapModelManager.cs

@@ -216,7 +216,8 @@ namespace osu.Game.Beatmaps
                     var fileInfo = setInfo.Files.SingleOrDefault(f => string.Equals(f.Filename, beatmapInfo.Path, StringComparison.OrdinalIgnoreCase)) ?? new BeatmapSetFileInfo();
 
                     // metadata may have changed; update the path with the standard format.
-                    beatmapInfo.Path = $"{metadata.Artist} - {metadata.Title} ({metadata.Author}) [{beatmapInfo.Version}].osu";
+                    beatmapInfo.Path = GetValidFilename($"{metadata.Artist} - {metadata.Title} ({metadata.Author}) [{beatmapInfo.Version}].osu");
+
                     beatmapInfo.MD5Hash = stream.ComputeMD5Hash();
 
                     // update existing or populate new file's filename.

osu.Game/Database/ArchiveModelManager.cs

@@ -466,7 +466,7 @@ namespace osu.Game.Database
             if (retrievedItem == null)
                 throw new ArgumentException(@"Specified model could not be found", nameof(item));
 
-            string filename = $"{getValidFilename(item.ToString())}{HandledExtensions.First()}";
+            string filename = $"{GetValidFilename(item.ToString())}{HandledExtensions.First()}";
 
             using (var stream = exportStorage.GetStream(filename, FileAccess.Write, FileMode.Create))
                 ExportModelTo(retrievedItem, stream);
@@ -913,9 +913,15 @@ namespace osu.Game.Database
             return Guid.NewGuid().ToString();
         }
 
-        private string getValidFilename(string filename)
+        private readonly char[] invalidFilenameCharacters = Path.GetInvalidFileNameChars()
+                                                                // Backslash is added to avoid issues when exporting to zip.
+                                                                // See SharpCompress filename normalisation https://github.com/adamhathcock/sharpcompress/blob/a1e7c0068db814c9aa78d86a94ccd1c761af74bd/src/SharpCompress/Writers/Zip/ZipWriter.cs#L143.
+                                                                .Append('\\')
+                                                                .ToArray();
+
+        protected string GetValidFilename(string filename)
         {
-            foreach (char c in Path.GetInvalidFileNameChars())
+            foreach (char c in invalidFilenameCharacters)
                 filename = filename.Replace(c, '_');
             return filename;
         }