From 10321497cab231eecd6e20531330c8fb9af944a3 Mon Sep 17 00:00:00 2001
From: MillhioreF <millhiore@ppy.sh>
Date: Wed, 9 Aug 2017 21:21:43 -0500
Subject: [PATCH 1/8] Add decoder entries for v3/4

---
 osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs b/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
index 433c23284f..b31e2adee3 100644
--- a/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
+++ b/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
@@ -27,7 +27,9 @@ namespace osu.Game.Beatmaps.Formats
             AddDecoder<OsuLegacyDecoder>(@"osu file format v7");
             AddDecoder<OsuLegacyDecoder>(@"osu file format v6");
             AddDecoder<OsuLegacyDecoder>(@"osu file format v5");
-            // TODO: Not sure how far back to go, or differences between versions
+            AddDecoder<OsuLegacyDecoder>(@"osu file format v4");
+            AddDecoder<OsuLegacyDecoder>(@"osu file format v3");
+            // TODO: differences between versions
         }
 
         private ConvertHitObjectParser parser;

From a8cf7ff93a935138e2d3fb6118bbef7646c060ae Mon Sep 17 00:00:00 2001
From: MillhioreF <millhiore@ppy.sh>
Date: Wed, 9 Aug 2017 23:27:13 -0500
Subject: [PATCH 2/8] Add a better error message for corrupt maps with no .osu
 files

---
 osu.Game/Beatmaps/BeatmapManager.cs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs
index bbb6c975d0..61cd2f0a2b 100644
--- a/osu.Game/Beatmaps/BeatmapManager.cs
+++ b/osu.Game/Beatmaps/BeatmapManager.cs
@@ -307,6 +307,16 @@ namespace osu.Game.Beatmaps
         /// <returns>The imported beatmap, or an existing instance if it is already present.</returns>
         private BeatmapSetInfo importToStorage(ArchiveReader reader)
         {
+            // let's make sure there are actually .osu files to import.
+            try
+            {
+                string mapName = reader.Filenames.First(f => f.EndsWith(".osu"));
+            }
+            catch
+            {
+                throw new InvalidOperationException("No beatmap files found in the map folder.");
+            }
+
             // for now, concatenate all .osu files in the set to create a unique hash.
             MemoryStream hashable = new MemoryStream();
             foreach (string file in reader.Filenames.Where(f => f.EndsWith(".osu")))

From 64d92c1557badd6e7c4a6e358a9b4b5d8ada13f3 Mon Sep 17 00:00:00 2001
From: MillhioreF <millhiore@ppy.sh>
Date: Wed, 9 Aug 2017 23:31:18 -0500
Subject: [PATCH 3/8] Fix infinite loop when importing maps that have
 storyboard elements with '$' in the filename

---
 osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs b/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
index b31e2adee3..39f0a0378c 100644
--- a/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
+++ b/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
@@ -222,7 +222,7 @@ namespace osu.Game.Beatmaps.Formats
         /// <param name="line">The line which may contains variables.</param>
         private void decodeVariables(ref string line)
         {
-            while (line.IndexOf('$') >= 0)
+            if (line.IndexOf('$') >= 0)
             {
                 string[] split = line.Split(',');
                 for (int i = 0; i < split.Length; i++)

From c16dbc05aafc2881cce98f78ddf9afce210df474 Mon Sep 17 00:00:00 2001
From: MillhioreF <millhiore@ppy.sh>
Date: Wed, 9 Aug 2017 23:41:22 -0500
Subject: [PATCH 4/8] Add new error for malformed (too many variables) hit
 objects during import

---
 osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
index a4c319291c..b02a582bec 100644
--- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
+++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
@@ -20,6 +20,9 @@ namespace osu.Game.Rulesets.Objects.Legacy
         public override HitObject Parse(string text)
         {
             string[] split = text.Split(',');
+            if (split.Length > 11)
+                throw new InvalidOperationException("One or more hit objects were malformed.");
+
             ConvertHitObjectType type = (ConvertHitObjectType)int.Parse(split[3]) & ~ConvertHitObjectType.ColourHax;
             bool combo = type.HasFlag(ConvertHitObjectType.NewCombo);
             type &= ~ConvertHitObjectType.NewCombo;

From f819ffce2bbb64e2fdba83d130042d084304625c Mon Sep 17 00:00:00 2001
From: MillhioreF <millhiore@ppy.sh>
Date: Thu, 10 Aug 2017 00:08:39 -0500
Subject: [PATCH 5/8] Make the legacy decoder more resilient against leading
 linebreaks

---
 osu.Game/Beatmaps/Formats/BeatmapDecoder.cs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/osu.Game/Beatmaps/Formats/BeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/BeatmapDecoder.cs
index 1c3eadc91e..234d65eee4 100644
--- a/osu.Game/Beatmaps/Formats/BeatmapDecoder.cs
+++ b/osu.Game/Beatmaps/Formats/BeatmapDecoder.cs
@@ -19,7 +19,9 @@ namespace osu.Game.Beatmaps.Formats
 
         public static BeatmapDecoder GetDecoder(StreamReader stream)
         {
-            string line = stream.ReadLine()?.Trim();
+            string line;
+            do { line = stream.ReadLine()?.Trim(); }
+                while (line != null && line.Length == 0);
 
             if (line == null || !decoders.ContainsKey(line))
                 throw new IOException(@"Unknown file format");

From 2e5a7374a8d6eb3a3bfc56f3c05fbccb8fddcff1 Mon Sep 17 00:00:00 2001
From: MillhioreF <millhiore@ppy.sh>
Date: Thu, 10 Aug 2017 01:49:34 -0500
Subject: [PATCH 6/8] Actually use mapName whoops

---
 osu.Game/Beatmaps/BeatmapManager.cs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs
index 61cd2f0a2b..4b063adde0 100644
--- a/osu.Game/Beatmaps/BeatmapManager.cs
+++ b/osu.Game/Beatmaps/BeatmapManager.cs
@@ -307,10 +307,11 @@ namespace osu.Game.Beatmaps
         /// <returns>The imported beatmap, or an existing instance if it is already present.</returns>
         private BeatmapSetInfo importToStorage(ArchiveReader reader)
         {
+            string mapName;
             // let's make sure there are actually .osu files to import.
             try
             {
-                string mapName = reader.Filenames.First(f => f.EndsWith(".osu"));
+                mapName = reader.Filenames.First(f => f.EndsWith(".osu"));
             }
             catch
             {
@@ -349,7 +350,7 @@ namespace osu.Game.Beatmaps
 
             BeatmapMetadata metadata;
 
-            using (var stream = new StreamReader(reader.GetStream(reader.Filenames.First(f => f.EndsWith(".osu")))))
+            using (var stream = new StreamReader(reader.GetStream(mapName)))
                 metadata = BeatmapDecoder.GetDecoder(stream).Decode(stream).Metadata;
 
             beatmapSet = new BeatmapSetInfo

From e42c279229cd64e43985ebfbdba6ef5256ab137f Mon Sep 17 00:00:00 2001
From: MillhioreF <millhiore@ppy.sh>
Date: Thu, 10 Aug 2017 01:50:20 -0500
Subject: [PATCH 7/8] More generic catching for broken hitobject strings

---
 .../Objects/Legacy/ConvertHitObjectParser.cs  | 237 +++++++++---------
 1 file changed, 121 insertions(+), 116 deletions(-)

diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
index b02a582bec..b5e3f837fc 100644
--- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
+++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs
@@ -19,150 +19,155 @@ namespace osu.Game.Rulesets.Objects.Legacy
     {
         public override HitObject Parse(string text)
         {
-            string[] split = text.Split(',');
-            if (split.Length > 11)
-                throw new InvalidOperationException("One or more hit objects were malformed.");
-
-            ConvertHitObjectType type = (ConvertHitObjectType)int.Parse(split[3]) & ~ConvertHitObjectType.ColourHax;
-            bool combo = type.HasFlag(ConvertHitObjectType.NewCombo);
-            type &= ~ConvertHitObjectType.NewCombo;
-
-            var soundType = (LegacySoundType)int.Parse(split[4]);
-            var bankInfo = new SampleBankInfo();
-
-            HitObject result = null;
-
-            if ((type & ConvertHitObjectType.Circle) > 0)
+            try
             {
-                result = CreateHit(new Vector2(int.Parse(split[0]), int.Parse(split[1])), combo);
+                string[] split = text.Split(',');
 
-                if (split.Length > 5)
-                    readCustomSampleBanks(split[5], bankInfo);
-            }
-            else if ((type & ConvertHitObjectType.Slider) > 0)
-            {
-                CurveType curveType = CurveType.Catmull;
-                double length = 0;
-                var points = new List<Vector2> { new Vector2(int.Parse(split[0]), int.Parse(split[1])) };
+                ConvertHitObjectType type = (ConvertHitObjectType)int.Parse(split[3]) & ~ConvertHitObjectType.ColourHax;
+                bool combo = type.HasFlag(ConvertHitObjectType.NewCombo);
+                type &= ~ConvertHitObjectType.NewCombo;
 
-                string[] pointsplit = split[5].Split('|');
-                foreach (string t in pointsplit)
+                var soundType = (LegacySoundType)int.Parse(split[4]);
+                var bankInfo = new SampleBankInfo();
+
+                HitObject result = null;
+
+                if ((type & ConvertHitObjectType.Circle) > 0)
                 {
-                    if (t.Length == 1)
+                    result = CreateHit(new Vector2(int.Parse(split[0]), int.Parse(split[1])), combo);
+
+                    if (split.Length > 5)
+                        readCustomSampleBanks(split[5], bankInfo);
+                }
+                else if ((type & ConvertHitObjectType.Slider) > 0)
+                {
+                    CurveType curveType = CurveType.Catmull;
+                    double length = 0;
+                    var points = new List<Vector2> { new Vector2(int.Parse(split[0]), int.Parse(split[1])) };
+
+                    string[] pointsplit = split[5].Split('|');
+                    foreach (string t in pointsplit)
                     {
-                        switch (t)
+                        if (t.Length == 1)
                         {
-                            case @"C":
-                                curveType = CurveType.Catmull;
-                                break;
-                            case @"B":
-                                curveType = CurveType.Bezier;
-                                break;
-                            case @"L":
-                                curveType = CurveType.Linear;
-                                break;
-                            case @"P":
-                                curveType = CurveType.PerfectCurve;
-                                break;
+                            switch (t)
+                            {
+                                case @"C":
+                                    curveType = CurveType.Catmull;
+                                    break;
+                                case @"B":
+                                    curveType = CurveType.Bezier;
+                                    break;
+                                case @"L":
+                                    curveType = CurveType.Linear;
+                                    break;
+                                case @"P":
+                                    curveType = CurveType.PerfectCurve;
+                                    break;
+                            }
+                            continue;
                         }
-                        continue;
+
+                        string[] temp = t.Split(':');
+                        points.Add(new Vector2((int)Convert.ToDouble(temp[0], CultureInfo.InvariantCulture), (int)Convert.ToDouble(temp[1], CultureInfo.InvariantCulture)));
                     }
 
-                    string[] temp = t.Split(':');
-                    points.Add(new Vector2((int)Convert.ToDouble(temp[0], CultureInfo.InvariantCulture), (int)Convert.ToDouble(temp[1], CultureInfo.InvariantCulture)));
-                }
+                    int repeatCount = Convert.ToInt32(split[6], CultureInfo.InvariantCulture);
 
-                int repeatCount = Convert.ToInt32(split[6], CultureInfo.InvariantCulture);
+                    if (repeatCount > 9000)
+                        throw new ArgumentOutOfRangeException(nameof(repeatCount), @"Repeat count is way too high");
 
-                if (repeatCount > 9000)
-                    throw new ArgumentOutOfRangeException(nameof(repeatCount), @"Repeat count is way too high");
+                    if (split.Length > 7)
+                        length = Convert.ToDouble(split[7], CultureInfo.InvariantCulture);
 
-                if (split.Length > 7)
-                    length = Convert.ToDouble(split[7], CultureInfo.InvariantCulture);
+                    if (split.Length > 10)
+                        readCustomSampleBanks(split[10], bankInfo);
 
-                if (split.Length > 10)
-                    readCustomSampleBanks(split[10], bankInfo);
+                    // One node for each repeat + the start and end nodes
+                    // Note that the first length of the slider is considered a repeat, but there are no actual repeats happening
+                    int nodes = Math.Max(0, repeatCount - 1) + 2;
 
-                // One node for each repeat + the start and end nodes
-                // Note that the first length of the slider is considered a repeat, but there are no actual repeats happening
-                int nodes = Math.Max(0, repeatCount - 1) + 2;
-
-                // Populate node sample bank infos with the default hit object sample bank
-                var nodeBankInfos = new List<SampleBankInfo>();
-                for (int i = 0; i < nodes; i++)
-                    nodeBankInfos.Add(bankInfo.Clone());
-
-                // Read any per-node sample banks
-                if (split.Length > 9 && split[9].Length > 0)
-                {
-                    string[] sets = split[9].Split('|');
+                    // Populate node sample bank infos with the default hit object sample bank
+                    var nodeBankInfos = new List<SampleBankInfo>();
                     for (int i = 0; i < nodes; i++)
+                        nodeBankInfos.Add(bankInfo.Clone());
+
+                    // Read any per-node sample banks
+                    if (split.Length > 9 && split[9].Length > 0)
                     {
-                        if (i >= sets.Length)
-                            break;
+                        string[] sets = split[9].Split('|');
+                        for (int i = 0; i < nodes; i++)
+                        {
+                            if (i >= sets.Length)
+                                break;
 
-                        SampleBankInfo info = nodeBankInfos[i];
-                        readCustomSampleBanks(sets[i], info);
+                            SampleBankInfo info = nodeBankInfos[i];
+                            readCustomSampleBanks(sets[i], info);
+                        }
                     }
-                }
 
-                // Populate node sound types with the default hit object sound type
-                var nodeSoundTypes = new List<LegacySoundType>();
-                for (int i = 0; i < nodes; i++)
-                    nodeSoundTypes.Add(soundType);
-
-                // Read any per-node sound types
-                if (split.Length > 8 && split[8].Length > 0)
-                {
-                    string[] adds = split[8].Split('|');
+                    // Populate node sound types with the default hit object sound type
+                    var nodeSoundTypes = new List<LegacySoundType>();
                     for (int i = 0; i < nodes; i++)
+                        nodeSoundTypes.Add(soundType);
+
+                    // Read any per-node sound types
+                    if (split.Length > 8 && split[8].Length > 0)
                     {
-                        if (i >= adds.Length)
-                            break;
+                        string[] adds = split[8].Split('|');
+                        for (int i = 0; i < nodes; i++)
+                        {
+                            if (i >= adds.Length)
+                                break;
 
-                        int sound;
-                        int.TryParse(adds[i], out sound);
-                        nodeSoundTypes[i] = (LegacySoundType)sound;
+                            int sound;
+                            int.TryParse(adds[i], out sound);
+                            nodeSoundTypes[i] = (LegacySoundType)sound;
+                        }
                     }
+
+                    // Generate the final per-node samples
+                    var nodeSamples = new List<SampleInfoList>(nodes);
+                    for (int i = 0; i <= repeatCount; i++)
+                        nodeSamples.Add(convertSoundType(nodeSoundTypes[i], nodeBankInfos[i]));
+
+                    result = CreateSlider(new Vector2(int.Parse(split[0]), int.Parse(split[1])), combo, points, length, curveType, repeatCount, nodeSamples);
                 }
-
-                // Generate the final per-node samples
-                var nodeSamples = new List<SampleInfoList>(nodes);
-                for (int i = 0; i <= repeatCount; i++)
-                    nodeSamples.Add(convertSoundType(nodeSoundTypes[i], nodeBankInfos[i]));
-
-                result = CreateSlider(new Vector2(int.Parse(split[0]), int.Parse(split[1])), combo, points, length, curveType, repeatCount, nodeSamples);
-            }
-            else if ((type & ConvertHitObjectType.Spinner) > 0)
-            {
-                result = CreateSpinner(new Vector2(512, 384) / 2, Convert.ToDouble(split[5], CultureInfo.InvariantCulture));
-
-                if (split.Length > 6)
-                    readCustomSampleBanks(split[6], bankInfo);
-            }
-            else if ((type & ConvertHitObjectType.Hold) > 0)
-            {
-                // Note: Hold is generated by BMS converts
-
-                double endTime = Convert.ToDouble(split[2], CultureInfo.InvariantCulture);
-
-                if (split.Length > 5 && !string.IsNullOrEmpty(split[5]))
+                else if ((type & ConvertHitObjectType.Spinner) > 0)
                 {
-                    string[] ss = split[5].Split(':');
-                    endTime = Convert.ToDouble(ss[0], CultureInfo.InvariantCulture);
-                    readCustomSampleBanks(string.Join(":", ss.Skip(1)), bankInfo);
+                    result = CreateSpinner(new Vector2(512, 384) / 2, Convert.ToDouble(split[5], CultureInfo.InvariantCulture));
+
+                    if (split.Length > 6)
+                        readCustomSampleBanks(split[6], bankInfo);
+                }
+                else if ((type & ConvertHitObjectType.Hold) > 0)
+                {
+                    // Note: Hold is generated by BMS converts
+
+                    double endTime = Convert.ToDouble(split[2], CultureInfo.InvariantCulture);
+
+                    if (split.Length > 5 && !string.IsNullOrEmpty(split[5]))
+                    {
+                        string[] ss = split[5].Split(':');
+                        endTime = Convert.ToDouble(ss[0], CultureInfo.InvariantCulture);
+                        readCustomSampleBanks(string.Join(":", ss.Skip(1)), bankInfo);
+                    }
+
+                    result = CreateHold(new Vector2(int.Parse(split[0]), int.Parse(split[1])), combo, endTime);
                 }
 
-                result = CreateHold(new Vector2(int.Parse(split[0]), int.Parse(split[1])), combo, endTime);
+                if (result == null)
+                    throw new InvalidOperationException($@"Unknown hit object type {type}.");
+
+                result.StartTime = Convert.ToDouble(split[2], CultureInfo.InvariantCulture);
+                result.Samples = convertSoundType(soundType, bankInfo);
+
+                return result;
+            }
+            catch (FormatException)
+            {
+                throw new FormatException("One or more hit objects were malformed.");
             }
-
-            if (result == null)
-                throw new InvalidOperationException($@"Unknown hit object type {type}.");
-
-            result.StartTime = Convert.ToDouble(split[2], CultureInfo.InvariantCulture);
-            result.Samples = convertSoundType(soundType, bankInfo);
-
-            return result;
         }
 
         private void readCustomSampleBanks(string str, SampleBankInfo bankInfo)

From e216bfcf105edf58e54369821722ab1b77c22c43 Mon Sep 17 00:00:00 2001
From: MillhioreF <millhiore@ppy.sh>
Date: Sun, 13 Aug 2017 00:40:05 -0500
Subject: [PATCH 8/8] Recommended fixes (obsolete try/catch, fix infinite loop
 during variable parsing in a better way)

---
 osu.Game/Beatmaps/BeatmapManager.cs           | 10 ++--------
 osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs |  4 +++-
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs
index 4b063adde0..e388f5b6d1 100644
--- a/osu.Game/Beatmaps/BeatmapManager.cs
+++ b/osu.Game/Beatmaps/BeatmapManager.cs
@@ -307,16 +307,10 @@ namespace osu.Game.Beatmaps
         /// <returns>The imported beatmap, or an existing instance if it is already present.</returns>
         private BeatmapSetInfo importToStorage(ArchiveReader reader)
         {
-            string mapName;
             // let's make sure there are actually .osu files to import.
-            try
-            {
-                mapName = reader.Filenames.First(f => f.EndsWith(".osu"));
-            }
-            catch
-            {
+            string mapName = reader.Filenames.FirstOrDefault(f => f.EndsWith(".osu"));
+            if (string.IsNullOrEmpty(mapName))
                 throw new InvalidOperationException("No beatmap files found in the map folder.");
-            }
 
             // for now, concatenate all .osu files in the set to create a unique hash.
             MemoryStream hashable = new MemoryStream();
diff --git a/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs b/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
index 39f0a0378c..46cbad2487 100644
--- a/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
+++ b/osu.Game/Beatmaps/Formats/OsuLegacyDecoder.cs
@@ -222,8 +222,9 @@ namespace osu.Game.Beatmaps.Formats
         /// <param name="line">The line which may contains variables.</param>
         private void decodeVariables(ref string line)
         {
-            if (line.IndexOf('$') >= 0)
+            while (line.IndexOf('$') >= 0)
             {
+                string origLine = line;
                 string[] split = line.Split(',');
                 for (int i = 0; i < split.Length; i++)
                 {
@@ -233,6 +234,7 @@ namespace osu.Game.Beatmaps.Formats
                 }
 
                 line = string.Join(",", split);
+                if (line == origLine) break;
             }
         }