From 25bbb02999481de9adecaa38f51e3e6293018bcd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 May 2020 22:57:41 +0900 Subject: [PATCH 1/5] Throw better exceptions from OsuStorage --- osu.Game/IO/OsuStorage.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 71b01ce479..8109631ef9 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -48,11 +48,14 @@ namespace osu.Game.IO var source = new DirectoryInfo(GetFullPath(".")); var destination = new DirectoryInfo(newLocation); + if (source.FullName == destination.FullName) + throw new ArgumentException("Destination provided is already the current location", nameof(newLocation)); + // ensure the new location has no files present, else hard abort if (destination.Exists) { if (destination.GetFiles().Length > 0 || destination.GetDirectories().Length > 0) - throw new InvalidOperationException("Migration destination already has files present"); + throw new ArgumentException("Destination provided already has files or directories present", nameof(newLocation)); deleteRecursive(destination); } From 19f117ae53fe3058ed251bd687a7cb8266e75cf8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 May 2020 20:18:57 +0900 Subject: [PATCH 2/5] Update test logic for new exception type --- osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index ef2b20de64..7a20bd364b 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -211,7 +211,7 @@ namespace osu.Game.Tests.NonVisual var osu = loadOsu(host); Assert.DoesNotThrow(() => osu.Migrate(customPath)); - Assert.Throws(() => osu.Migrate(customPath)); + Assert.Throws(() => osu.Migrate(customPath)); } finally { From 0690d81bbb1bc7ee969b7e07050a565138b5b5c0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 May 2020 22:42:42 +0900 Subject: [PATCH 3/5] Add protection against migrating to a nested folder --- .../NonVisual/CustomDataDirectoryTest.cs | 48 +++++++++++++++++++ osu.Game/IO/OsuStorage.cs | 9 +++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index 7a20bd364b..2a98a6dbc6 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -220,6 +220,54 @@ namespace osu.Game.Tests.NonVisual } } + [Test] + public void TestMigrationToNestedTargetFails() + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestMigrationToSameTargetFails))) + { + try + { + var osu = loadOsu(host); + + Assert.DoesNotThrow(() => osu.Migrate(customPath)); + + string subFolder = Path.Combine(customPath, "sub"); + + Directory.CreateDirectory(subFolder); + + Assert.Throws(() => osu.Migrate(subFolder)); + } + finally + { + host.Exit(); + } + } + } + + [Test] + public void TestMigrationToSeeminglyNestedTarget() + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestMigrationToSeeminglyNestedTarget))) + { + try + { + var osu = loadOsu(host); + + Assert.DoesNotThrow(() => osu.Migrate(customPath)); + + string subFolder = customPath + "sub"; + + Directory.CreateDirectory(subFolder); + + osu.Migrate(subFolder); + } + finally + { + host.Exit(); + } + } + } + private OsuGameBase loadOsu(GameHost host) { var osu = new OsuGameBase(); diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 8109631ef9..ac28a05375 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -48,9 +48,16 @@ namespace osu.Game.IO var source = new DirectoryInfo(GetFullPath(".")); var destination = new DirectoryInfo(newLocation); - if (source.FullName == destination.FullName) + // using Uri is the easiest way to check equality and contains (https://stackoverflow.com/a/7710620) + var sourceUri = new Uri(source.FullName + Path.DirectorySeparatorChar); + var destinationUri = new Uri(destination.FullName + Path.DirectorySeparatorChar); + + if (sourceUri == destinationUri) throw new ArgumentException("Destination provided is already the current location", nameof(newLocation)); + if (sourceUri.IsBaseOf(destinationUri)) + throw new ArgumentException("Destination provided is inside the source", nameof(newLocation)); + // ensure the new location has no files present, else hard abort if (destination.Exists) { From 7641507c90f770bab9e096dad63132001cc532d5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 May 2020 10:45:57 +0900 Subject: [PATCH 4/5] Ensure test directories are deleted before subsequent run --- osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index 2a98a6dbc6..d69bf94ee2 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -233,6 +233,9 @@ namespace osu.Game.Tests.NonVisual string subFolder = Path.Combine(customPath, "sub"); + if (Directory.Exists(subFolder)) + Directory.Delete(subFolder, true); + Directory.CreateDirectory(subFolder); Assert.Throws(() => osu.Migrate(subFolder)); @@ -255,11 +258,14 @@ namespace osu.Game.Tests.NonVisual Assert.DoesNotThrow(() => osu.Migrate(customPath)); - string subFolder = customPath + "sub"; + string seeminglySubFolder = customPath + "sub"; - Directory.CreateDirectory(subFolder); + if (Directory.Exists(seeminglySubFolder)) + Directory.Delete(seeminglySubFolder, true); - osu.Migrate(subFolder); + Directory.CreateDirectory(seeminglySubFolder); + + osu.Migrate(seeminglySubFolder); } finally { From aea192080a51f014d79da5caad8ce70507d65a74 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 May 2020 13:02:46 +0900 Subject: [PATCH 5/5] Fix incorrect storage name --- osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index d69bf94ee2..743c924bbd 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -223,7 +223,7 @@ namespace osu.Game.Tests.NonVisual [Test] public void TestMigrationToNestedTargetFails() { - using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestMigrationToSameTargetFails))) + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestMigrationToNestedTargetFails))) { try {