Throw ArgumentException if path contains \

Check that path doesn't contain \ rather than automatically convert to use /.
github · StanleyGoldman · Mar 26, 2019 · Dec 18, 2018 · Dec 18, 2018 · Dec 18, 2018
commit f685a22d3831ffd6e88c5966850630d9ff9f877b
diff --git a/src/GitHub.Exports/Services/GitService.cs b/src/GitHub.Exports/Services/GitService.cs
@@ -267,15 +267,18 @@ public Task<ContentChanges> CompareWith(IRepository repository, string sha1, str
             Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
             Guard.ArgumentNotEmptyString(sha2, nameof(sha1));
             Guard.ArgumentNotEmptyString(path, nameof(path));
+            if (path.Contains('\\'))
+            {
+                throw new ArgumentException($"The value for '{nameof(path)}' must use '/' not '\\' as directory separator", nameof(path));
+            }
 
             return Task.Run(() =>
             {
                 var commit1 = repository.Lookup<Commit>(sha1);
                 var commit2 = repository.Lookup<Commit>(sha2);
 
                 var treeChanges = repository.Diff.Compare<TreeChanges>(commit1.Tree, commit2.Tree, defaultCompareOptions);
-                var normalizedPath = path.Replace("\\", "/");
-                var change = treeChanges.FirstOrDefault(x => x.Path == normalizedPath);
+                var change = treeChanges.FirstOrDefault(x => x.Path == path);
                 var oldPath = change?.OldPath;
 
                 if (commit1 != null && oldPath != null)

diff --git a/src/GitHub.Exports/Services/IGitService.cs b/src/GitHub.Exports/Services/IGitService.cs
@@ -90,11 +90,12 @@ public interface IGitService
         /// <param name="repository">The repository</param>
         /// <param name="sha1">The SHA of the first commit.</param>
         /// <param name="sha2">The SHA of the second commit.</param>
-        /// <param name="path">The relative path to the file.</param>
+        /// <param name="path">The relative path to the file (using '/' directory separator).</param>
         /// <param name="contents">The contents to compare with the file.</param>
         /// <returns>
         /// A <see cref="Patch"/> object or null if the commit could not be found in the repository.
         /// </returns>
+        /// <exception cref="ArgumentException">If <paramref name="path"/> contains a '\'.</exception>
         Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents);
 
         /// <summary>

diff --git a/test/GitHub.Exports.UnitTests/GitServiceIntegrationTests.cs b/test/GitHub.Exports.UnitTests/GitServiceIntegrationTests.cs
@@ -97,10 +97,10 @@ public async Task Indent_Heuristic_Is_Enabled(string content1, string content2,
         }
 
         [TestCase("foo.txt", "a.b.", "bar.txt", "a.b.c.d.", 2)]
-        [TestCase(@"dir\foo.txt", "a.b.", @"dir\bar.txt", "a.b.c.d.", 2)]
-        [TestCase(@"dir\foo.txt", "a.b.", @"dir\foo.txt", "a.b.c.d.", 2)]
-        [TestCase(@"dir\unrelated.txt", "x.x.x.x.", @"dir\foo.txt", "a.b.c.d.", 4)]
-        public async Task Rename(string oldPath, string oldContent, string newPath, string newContent, int expectLinesAdded)
+        [TestCase(@"dir/foo.txt", "a.b.", @"dir/bar.txt", "a.b.c.d.", 2)]
+        [TestCase(@"dir/foo.txt", "a.b.", @"dir/foo.txt", "a.b.c.d.", 2)]
+        [TestCase(@"dir/unrelated.txt", "x.x.x.x.", @"dir/foo.txt", "a.b.c.d.", 4)]
+        public async Task Can_Handle_Renames(string oldPath, string oldContent, string newPath, string newContent, int expectLinesAdded)
         {
             using (var temp = new TempRepository())
             {
@@ -114,6 +114,24 @@ public async Task Rename(string oldPath, string oldContent, string newPath, stri
                 Assert.That(changes?.LinesAdded, Is.EqualTo(expectLinesAdded));
             }
         }
+
+        [Test]
+        public void Path_Must_Not_Use_Windows_Directory_Separator()
+        {
+            using (var temp = new TempRepository())
+            {
+                var path = @"dir\foo.txt";
+                var oldContent = "oldContent";
+                var newContent = "newContent";
+                var commit1 = AddCommit(temp.Repository, path, oldContent);
+                var commit2 = AddCommit(temp.Repository, path, newContent);
+                var contentBytes = new UTF8Encoding(false).GetBytes(newContent);
+                var target = new GitService(new RepositoryFacade());
+
+                Assert.ThrowsAsync<ArgumentException>(() =>
+                    target.CompareWith(temp.Repository, commit1.Sha, commit2.Sha, path, contentBytes));
+            }
+        }
     }
 
     public class TheCreateLocalRepositoryModelMethod