From dbc77850ff62b134d7fe7ab659ca2d3ef24cf556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 31 Aug 2014 16:48:41 +0200 Subject: [PATCH 1/4] transport: distinguish between unknown and unsupported transports Even when built without a SSH support, we know about this transport. It is implemented, but the current code makes us return an error message saying it's not. This is a leftover from the initial implementation of the transports when there were in fact transports we knew about but were not implemented. Instead, let the SSH transport itself say it cannot run, the same as we do for HTTPS. --- src/transport.c | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/transport.c b/src/transport.c index d390e1422..a5faf3f19 100644 --- a/src/transport.c +++ b/src/transport.c @@ -20,25 +20,16 @@ typedef struct transport_definition { static git_smart_subtransport_definition http_subtransport_definition = { git_smart_subtransport_http, 1 }; static git_smart_subtransport_definition git_subtransport_definition = { git_smart_subtransport_git, 0 }; -#ifdef GIT_SSH static git_smart_subtransport_definition ssh_subtransport_definition = { git_smart_subtransport_ssh, 0 }; -#endif static transport_definition local_transport_definition = { "file://", git_transport_local, NULL }; -#ifdef GIT_SSH -static transport_definition ssh_transport_definition = { "ssh://", git_transport_smart, &ssh_subtransport_definition }; -#else -static transport_definition dummy_transport_definition = { NULL, git_transport_dummy, NULL }; -#endif static transport_definition transports[] = { { "git://", git_transport_smart, &git_subtransport_definition }, { "http://", git_transport_smart, &http_subtransport_definition }, { "https://", git_transport_smart, &http_subtransport_definition }, { "file://", git_transport_local, NULL }, -#ifdef GIT_SSH { "ssh://", git_transport_smart, &ssh_subtransport_definition }, -#endif { NULL, 0, 0 } }; @@ -95,11 +86,6 @@ static int transport_find_fn( if (!definition && strrchr(url, ':')) { // re-search transports again with ssh:// as url so that we can find a third party ssh transport definition = transport_find_by_url("ssh://"); -#ifndef GIT_SSH - if (!definition) { - definition = &dummy_transport_definition; - } -#endif } #ifndef GIT_WIN32 @@ -121,15 +107,6 @@ static int transport_find_fn( * Public API * **************/ -int git_transport_dummy(git_transport **transport, git_remote *owner, void *param) -{ - GIT_UNUSED(transport); - GIT_UNUSED(owner); - GIT_UNUSED(param); - giterr_set(GITERR_NET, "This transport isn't implemented. Sorry"); - return -1; -} - int git_transport_new(git_transport **out, git_remote *owner, const char *url) { git_transport_cb fn; @@ -246,7 +223,7 @@ int git_remote_supported_url(const char* url) if (transport_find_fn(&fn, url, ¶m) < 0) return 0; - return fn != &git_transport_dummy; + return 1; } int git_transport_init(git_transport *opts, unsigned int version) From bd3854a09c9bd4196cf280108d3f6286ee6de258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 31 Aug 2014 17:12:45 +0200 Subject: [PATCH 2/4] transport: return ENOTFOUND for HTTPS and SSH when they're not supported The previous commit makes it harder to figure out if the library was built with support for a particular transport. Roll back some of the changes and remove ssh:// and https:// from the list if we're being built without support for them. --- src/transport.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/transport.c b/src/transport.c index a5faf3f19..3295b196c 100644 --- a/src/transport.c +++ b/src/transport.c @@ -20,16 +20,22 @@ typedef struct transport_definition { static git_smart_subtransport_definition http_subtransport_definition = { git_smart_subtransport_http, 1 }; static git_smart_subtransport_definition git_subtransport_definition = { git_smart_subtransport_git, 0 }; +#ifdef GIT_SSH static git_smart_subtransport_definition ssh_subtransport_definition = { git_smart_subtransport_ssh, 0 }; +#endif static transport_definition local_transport_definition = { "file://", git_transport_local, NULL }; static transport_definition transports[] = { { "git://", git_transport_smart, &git_subtransport_definition }, { "http://", git_transport_smart, &http_subtransport_definition }, +#if defined(GIT_SSL) || defined(GIT_WINHTTP) { "https://", git_transport_smart, &http_subtransport_definition }, +#endif { "file://", git_transport_local, NULL }, +#ifdef GIT_SSH { "ssh://", git_transport_smart, &ssh_subtransport_definition }, +#endif { NULL, 0, 0 } }; From ba67c0752269ba06523fe7f5fa350e6328b20241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 31 Aug 2014 17:16:40 +0200 Subject: [PATCH 3/4] remote: get rid of git_remote_valid_url() It does the same as git_remote_supported_url() but has a name which implies we'd check the URL for correctness while we're simply looking at the scheme and looking it up in our lists. While here, fix up the tests so we check all the combination of what's supported. --- include/git2/remote.h | 13 +++++-------- src/transport.c | 15 ++------------- tests/network/remote/remotes.c | 29 +++++++++++++++++------------ 3 files changed, 24 insertions(+), 33 deletions(-) diff --git a/include/git2/remote.h b/include/git2/remote.h index c0717fa31..de5823e6d 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -384,15 +384,12 @@ GIT_EXTERN(int) git_remote_fetch( const char *reflog_message); /** - * Return whether a string is a valid remote URL * - * @param url the url to check - * @return 1 if the url is valid, 0 otherwise - */ -GIT_EXTERN(int) git_remote_valid_url(const char *url); - -/** - * Return whether the passed URL is supported by this version of the library. + * Return whether the library supports a particular URL scheme + * + * Both the built-in and externally-registered transport lists are + * searched for a transport which supports the scheme of the given + * URL. * * @param url the url to check * @return 1 if the url is supported, 0 otherwise diff --git a/src/transport.c b/src/transport.c index 3295b196c..d42c92684 100644 --- a/src/transport.c +++ b/src/transport.c @@ -212,24 +212,13 @@ done: return error; } -/* from remote.h */ -int git_remote_valid_url(const char *url) -{ - git_transport_cb fn; - void *param; - - return !transport_find_fn(&fn, url, ¶m); -} - int git_remote_supported_url(const char* url) { git_transport_cb fn; void *param; - if (transport_find_fn(&fn, url, ¶m) < 0) - return 0; - - return 1; + /* The only error we expect is ENOTFOUND */ + return !transport_find_fn(&fn, url, ¶m); } int git_transport_init(git_transport *opts, unsigned int version) diff --git a/tests/network/remote/remotes.c b/tests/network/remote/remotes.c index 21c57119a..1d5d318c5 100644 --- a/tests/network/remote/remotes.c +++ b/tests/network/remote/remotes.c @@ -91,25 +91,30 @@ void test_network_remote_remotes__error_when_no_push_available(void) git_remote_free(r); } -void test_network_remote_remotes__parsing_ssh_remote(void) -{ - cl_assert( git_remote_valid_url("git@github.com:libgit2/libgit2.git") ); -} - -void test_network_remote_remotes__parsing_local_path_fails_if_path_not_found(void) -{ - cl_assert( !git_remote_valid_url("/home/git/repos/libgit2.git") ); -} - void test_network_remote_remotes__supported_transport_methods_are_supported(void) { - cl_assert( git_remote_supported_url("git://github.com/libgit2/libgit2") ); + cl_assert(git_remote_supported_url("git://github.com/libgit2/libgit2")); + cl_assert(git_remote_supported_url("http://github.com/libgit2/libgit2")); + +#ifdef GIT_SSH + cl_assert(git_remote_supported_url("git@github.com:libgit2/libgit2.git")); + cl_assert(git_remote_supported_url("ssh://git@github.com/libgit2/libgit2.git")); +#endif + +#if defined(GIT_SSL) || defined(GIT_WINHTTP) + cl_assert(git_remote_supported_url("https://git@github.com/libgit2/libgit2.git")); +#endif } void test_network_remote_remotes__unsupported_transport_methods_are_unsupported(void) { #ifndef GIT_SSH - cl_assert( !git_remote_supported_url("git@github.com:libgit2/libgit2.git") ); + cl_assert(!git_remote_supported_url("git@github.com:libgit2/libgit2.git")); + cl_assert(!git_remote_supported_url("ssh://git@github.com/libgit2/libgit2.git")); +#endif + +#if !defined(GIT_SSL) && !defined(GIT_WINHTTP) + cl_assert(!git_remote_supported_url("https://git@github.com/libgit2/libgit2.git")); #endif } From 05ac70514fed0ae75ba2844f22b233da1c25b8cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 31 Aug 2014 21:53:42 +0200 Subject: [PATCH 4/4] remote: test for supported URLs in a single place Instead of using ifdefs to run the tests, use them to set when we expect to support a particular scheme and always have the tests in the code. --- tests/network/remote/remotes.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/tests/network/remote/remotes.c b/tests/network/remote/remotes.c index 1d5d318c5..45f2a795f 100644 --- a/tests/network/remote/remotes.c +++ b/tests/network/remote/remotes.c @@ -91,31 +91,24 @@ void test_network_remote_remotes__error_when_no_push_available(void) git_remote_free(r); } -void test_network_remote_remotes__supported_transport_methods_are_supported(void) +void test_network_remote_remotes__supported_urls(void) { - cl_assert(git_remote_supported_url("git://github.com/libgit2/libgit2")); - cl_assert(git_remote_supported_url("http://github.com/libgit2/libgit2")); + int ssh_supported = 0, https_supported = 0; #ifdef GIT_SSH - cl_assert(git_remote_supported_url("git@github.com:libgit2/libgit2.git")); - cl_assert(git_remote_supported_url("ssh://git@github.com/libgit2/libgit2.git")); + ssh_supported = 1; #endif #if defined(GIT_SSL) || defined(GIT_WINHTTP) - cl_assert(git_remote_supported_url("https://git@github.com/libgit2/libgit2.git")); -#endif -} - -void test_network_remote_remotes__unsupported_transport_methods_are_unsupported(void) -{ -#ifndef GIT_SSH - cl_assert(!git_remote_supported_url("git@github.com:libgit2/libgit2.git")); - cl_assert(!git_remote_supported_url("ssh://git@github.com/libgit2/libgit2.git")); + https_supported = 1; #endif -#if !defined(GIT_SSL) && !defined(GIT_WINHTTP) - cl_assert(!git_remote_supported_url("https://git@github.com/libgit2/libgit2.git")); -#endif + cl_assert(git_remote_supported_url("git://github.com/libgit2/libgit2")); + cl_assert(git_remote_supported_url("http://github.com/libgit2/libgit2")); + + cl_assert_equal_i(ssh_supported, git_remote_supported_url("git@github.com:libgit2/libgit2.git")); + cl_assert_equal_i(ssh_supported, git_remote_supported_url("ssh://git@github.com/libgit2/libgit2.git")); + cl_assert_equal_i(https_supported, git_remote_supported_url("https://github.com/libgit2/libgit2.git")); } void test_network_remote_remotes__refspec_parsing(void)