Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit f685a22

Browse files
committed
Throw ArgumentException if path contains \
Check that path doesn't contain \ rather than automatically convert to use /.
1 parent 7362f56 commit f685a22

File tree

3 files changed

+29
-7
lines changed

3 files changed

+29
-7
lines changed

src/GitHub.Exports/Services/GitService.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,15 +267,18 @@ public Task<ContentChanges> CompareWith(IRepository repository, string sha1, str
267267
Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
268268
Guard.ArgumentNotEmptyString(sha2, nameof(sha1));
269269
Guard.ArgumentNotEmptyString(path, nameof(path));
270+
if (path.Contains('\\'))
271+
{
272+
throw new ArgumentException($"The value for '{nameof(path)}' must use '/' not '\\' as directory separator", nameof(path));
273+
}
270274

271275
return Task.Run(() =>
272276
{
273277
var commit1 = repository.Lookup<Commit>(sha1);
274278
var commit2 = repository.Lookup<Commit>(sha2);
275279

276280
var treeChanges = repository.Diff.Compare<TreeChanges>(commit1.Tree, commit2.Tree, defaultCompareOptions);
277-
var normalizedPath = path.Replace("\\", "/");
278-
var change = treeChanges.FirstOrDefault(x => x.Path == normalizedPath);
281+
var change = treeChanges.FirstOrDefault(x => x.Path == path);
279282
var oldPath = change?.OldPath;
280283

281284
if (commit1 != null && oldPath != null)

src/GitHub.Exports/Services/IGitService.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,12 @@ public interface IGitService
9090
/// <param name="repository">The repository</param>
9191
/// <param name="sha1">The SHA of the first commit.</param>
9292
/// <param name="sha2">The SHA of the second commit.</param>
93-
/// <param name="path">The relative path to the file.</param>
93+
/// <param name="path">The relative path to the file (using '/' directory separator).</param>
9494
/// <param name="contents">The contents to compare with the file.</param>
9595
/// <returns>
9696
/// A <see cref="Patch"/> object or null if the commit could not be found in the repository.
9797
/// </returns>
98+
/// <exception cref="ArgumentException">If <paramref name="path"/> contains a '\'.</exception>
9899
Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents);
99100

100101
/// <summary>

test/GitHub.Exports.UnitTests/GitServiceIntegrationTests.cs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,10 @@ public async Task Indent_Heuristic_Is_Enabled(string content1, string content2,
9797
}
9898

9999
[TestCase("foo.txt", "a.b.", "bar.txt", "a.b.c.d.", 2)]
100-
[TestCase(@"dir\foo.txt", "a.b.", @"dir\bar.txt", "a.b.c.d.", 2)]
101-
[TestCase(@"dir\foo.txt", "a.b.", @"dir\foo.txt", "a.b.c.d.", 2)]
102-
[TestCase(@"dir\unrelated.txt", "x.x.x.x.", @"dir\foo.txt", "a.b.c.d.", 4)]
103-
public async Task Rename(string oldPath, string oldContent, string newPath, string newContent, int expectLinesAdded)
100+
[TestCase(@"dir/foo.txt", "a.b.", @"dir/bar.txt", "a.b.c.d.", 2)]
101+
[TestCase(@"dir/foo.txt", "a.b.", @"dir/foo.txt", "a.b.c.d.", 2)]
102+
[TestCase(@"dir/unrelated.txt", "x.x.x.x.", @"dir/foo.txt", "a.b.c.d.", 4)]
103+
public async Task Can_Handle_Renames(string oldPath, string oldContent, string newPath, string newContent, int expectLinesAdded)
104104
{
105105
using (var temp = new TempRepository())
106106
{
@@ -114,6 +114,24 @@ public async Task Rename(string oldPath, string oldContent, string newPath, stri
114114
Assert.That(changes?.LinesAdded, Is.EqualTo(expectLinesAdded));
115115
}
116116
}
117+
118+
[Test]
119+
public void Path_Must_Not_Use_Windows_Directory_Separator()
120+
{
121+
using (var temp = new TempRepository())
122+
{
123+
var path = @"dir\foo.txt";
124+
var oldContent = "oldContent";
125+
var newContent = "newContent";
126+
var commit1 = AddCommit(temp.Repository, path, oldContent);
127+
var commit2 = AddCommit(temp.Repository, path, newContent);
128+
var contentBytes = new UTF8Encoding(false).GetBytes(newContent);
129+
var target = new GitService(new RepositoryFacade());
130+
131+
Assert.ThrowsAsync<ArgumentException>(() =>
132+
target.CompareWith(temp.Repository, commit1.Sha, commit2.Sha, path, contentBytes));
133+
}
134+
}
117135
}
118136

119137
public class TheCreateLocalRepositoryModelMethod

0 commit comments

Comments
 (0)