From 0da9919408de322fead3cfdc4f55f641878cd4ed Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 8 Oct 2015 19:19:17 +0200 Subject: [PATCH 1/3] ToRepositoryUrl should never throw, it's a conversion helper Fixes #121 --- src/GitHub.Exports/Primitives/UriString.cs | 7 +++- src/GitHub.Exports/Services/GitService.cs | 3 +- .../GitHub.Exports/GitServiceTests.cs | 37 +++++++++++++++++++ .../GitHub.Primitives/UriStringTests.cs | 21 +++++++++++ src/UnitTests/UnitTests.csproj | 1 + 5 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 src/UnitTests/GitHub.Exports/GitServiceTests.cs diff --git a/src/GitHub.Exports/Primitives/UriString.cs b/src/GitHub.Exports/Primitives/UriString.cs index da6139a64d..8125bdaf4f 100644 --- a/src/GitHub.Exports/Primitives/UriString.cs +++ b/src/GitHub.Exports/Primitives/UriString.cs @@ -108,6 +108,7 @@ bool ParseScpSyntax(string scpString) Host = match.Groups["host"].Value.ToNullIfEmpty(); Owner = match.Groups["owner"].Value.ToNullIfEmpty(); RepositoryName = GetRepositoryName(match.Groups["repo"].Value); + IsScpUri = true; return true; } return false; @@ -123,6 +124,8 @@ bool ParseScpSyntax(string scpString) public bool IsFileUri { get; private set; } + public bool IsScpUri { get; private set; } + public bool IsValidUri => url != null; /// @@ -131,7 +134,9 @@ bool ParseScpSyntax(string scpString) /// public Uri ToRepositoryUrl() { - if (url != null && IsFileUri) return url; + // we only want to process urls that represent network resources + if (!IsValidUri && !IsScpUri) return url; + if (IsValidUri && IsFileUri) return url; var scheme = url != null && IsHypertextTransferProtocol ? url.Scheme diff --git a/src/GitHub.Exports/Services/GitService.cs b/src/GitHub.Exports/Services/GitService.cs index 2800aa869a..d08efb0219 100644 --- a/src/GitHub.Exports/Services/GitService.cs +++ b/src/GitHub.Exports/Services/GitService.cs @@ -167,8 +167,7 @@ public static UriString GetOriginUri(IRepository repo) { return repo ?.Network - .Remotes - .FirstOrDefault(x => x.Name.Equals("origin", StringComparison.Ordinal)) + .Remotes["origin"] ?.Url; } diff --git a/src/UnitTests/GitHub.Exports/GitServiceTests.cs b/src/UnitTests/GitHub.Exports/GitServiceTests.cs new file mode 100644 index 0000000000..52e40b92be --- /dev/null +++ b/src/UnitTests/GitHub.Exports/GitServiceTests.cs @@ -0,0 +1,37 @@ +using System; +using GitHub.Services; +using LibGit2Sharp; +using NSubstitute; +using Xunit; +using System.Linq; +using System.Collections; +using System.Collections.Generic; + +public class GitServiceTests : TestBaseClass +{ + [Theory] + [InlineData("asdf", null)] + [InlineData("", null)] + [InlineData(null, null)] + [InlineData("file:///C:/dev/exp/foo", "file:///C:/dev/exp/foo")] + [InlineData("http://example.com/", "http://example.com/")] + [InlineData("http://haacked@example.com/foo/bar", "http://example.com/foo/bar")] + [InlineData("https://github.com/github/Windows", "https://github.com/github/Windows")] + [InlineData("https://github.com/github/Windows.git", "https://github.com/github/Windows")] + [InlineData("https://haacked@github.com/github/Windows.git", "https://github.com/github/Windows")] + [InlineData("http://example.com:4000/github/Windows", "http://example.com:4000/github/Windows")] + [InlineData("git@192.168.1.2:github/Windows.git", "https://192.168.1.2/github/Windows")] + [InlineData("git@example.com:org/repo.git", "https://example.com/org/repo")] + [InlineData("ssh://git@github.com:443/shana/cef", "https://github.com/shana/cef")] + [InlineData("ssh://git@example.com:23/haacked/encourage", "https://example.com:23/haacked/encourage")] + public void GetUriShouldNotThrow(string url, string expected) + { + var origin = Substitute.For(); + origin.Url.Returns(url); + var repository = Substitute.For(); + repository.Network.Remotes["origin"].Returns(origin); + + var gitservice = new GitService(); + Assert.Equal(expected, gitservice.GetUri(repository)?.ToString()); + } +} diff --git a/src/UnitTests/GitHub.Primitives/UriStringTests.cs b/src/UnitTests/GitHub.Primitives/UriStringTests.cs index c2b4783ef5..fa0caf154d 100644 --- a/src/UnitTests/GitHub.Primitives/UriStringTests.cs +++ b/src/UnitTests/GitHub.Primitives/UriStringTests.cs @@ -177,6 +177,27 @@ public void ConvertsToWebUrl(string uriString, string expected) { Assert.Equal(new Uri(expected), new UriString(uriString).ToRepositoryUrl()); } + + [Theory] + [InlineData("asdf", null)] + [InlineData("", null)] + [InlineData("file:///C:/dev/exp/foo", "file:///C:/dev/exp/foo")] + [InlineData("http://example.com/", "http://example.com/")] + [InlineData("http://haacked@example.com/foo/bar", "http://example.com/foo/bar")] + [InlineData("https://github.com/github/Windows", "https://github.com/github/Windows")] + [InlineData("https://github.com/github/Windows.git", "https://github.com/github/Windows")] + [InlineData("https://haacked@github.com/github/Windows.git", "https://github.com/github/Windows")] + [InlineData("http://example.com:4000/github/Windows", "http://example.com:4000/github/Windows")] + [InlineData("git@192.168.1.2:github/Windows.git", "https://192.168.1.2/github/Windows")] + [InlineData("git@example.com:org/repo.git", "https://example.com/org/repo")] + [InlineData("ssh://git@github.com:443/shana/cef", "https://github.com/shana/cef")] + [InlineData("ssh://git@example.com:23/haacked/encourage", "https://example.com:23/haacked/encourage")] + public void ShouldNeverThrow(string url, string expected) + { + Uri uri; + Uri.TryCreate(expected, UriKind.Absolute, out uri); + Assert.Equal(uri, new UriString(url).ToRepositoryUrl()); + } } public class TheAdditionOperator diff --git a/src/UnitTests/UnitTests.csproj b/src/UnitTests/UnitTests.csproj index 660b00715c..d649562b8c 100644 --- a/src/UnitTests/UnitTests.csproj +++ b/src/UnitTests/UnitTests.csproj @@ -155,6 +155,7 @@ + From 9dc2a8edbd61b14301004eb76c83b6b6876230d4 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 8 Oct 2015 20:23:40 +0200 Subject: [PATCH 2/3] Simplify check --- src/GitHub.Exports/Primitives/UriString.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/GitHub.Exports/Primitives/UriString.cs b/src/GitHub.Exports/Primitives/UriString.cs index 8125bdaf4f..aeea7e372c 100644 --- a/src/GitHub.Exports/Primitives/UriString.cs +++ b/src/GitHub.Exports/Primitives/UriString.cs @@ -135,8 +135,7 @@ bool ParseScpSyntax(string scpString) public Uri ToRepositoryUrl() { // we only want to process urls that represent network resources - if (!IsValidUri && !IsScpUri) return url; - if (IsValidUri && IsFileUri) return url; + if (!IsScpUri && (!IsValidUri || IsFileUri)) return url; var scheme = url != null && IsHypertextTransferProtocol ? url.Scheme From f6bd5b6b835f1f2b6f18fba6dcf4146df141883b Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 8 Oct 2015 20:25:23 +0200 Subject: [PATCH 3/3] Add comment clarifying return value --- src/GitHub.Exports/Primitives/UriString.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHub.Exports/Primitives/UriString.cs b/src/GitHub.Exports/Primitives/UriString.cs index aeea7e372c..32628f1a78 100644 --- a/src/GitHub.Exports/Primitives/UriString.cs +++ b/src/GitHub.Exports/Primitives/UriString.cs @@ -131,7 +131,7 @@ bool ParseScpSyntax(string scpString) /// /// Attempts a best-effort to convert the remote origin to a GitHub Repository URL. /// - /// + /// A converted uri, or the existing one if we can't convert it (which might be null) public Uri ToRepositoryUrl() { // we only want to process urls that represent network resources