From 2bca5b679b9e1f7f7e5cfafa75a6a94549875197 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Thu, 7 Feb 2013 23:44:18 +0100 Subject: [PATCH] remote: Introduce git_remote_is_valid_name() Fix libgit2/libgit2sharp#318 --- include/git2/remote.h | 8 ++++++ src/refs.c | 9 +++---- src/remote.c | 36 +++++++++++++++---------- tests-clar/network/remote/isvalidname.c | 17 ++++++++++++ tests-clar/network/remote/remotes.c | 22 ++++++++++++--- tests-clar/refs/isvalidname.c | 1 + tests-clar/refs/normalize.c | 2 ++ 7 files changed, 72 insertions(+), 23 deletions(-) create mode 100644 tests-clar/network/remote/isvalidname.c diff --git a/include/git2/remote.h b/include/git2/remote.h index b92a0cd04..6f36a3999 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -450,6 +450,14 @@ GIT_EXTERN(int) git_remote_update_fetchhead(git_remote *remote); */ GIT_EXTERN(void) git_remote_set_update_fetchhead(git_remote *remote, int value); +/** + * Ensure the remote name is well-formed. + * + * @param remote_name name to be checked. + * @return 1 if the reference name is acceptable; 0 if it isn't + */ +GIT_EXTERN(int) git_remote_is_valid_name(const char *remote_name); + /** @} */ GIT_END_DECL #endif diff --git a/src/refs.c b/src/refs.c index fd57ce8f7..7dabfefae 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1622,7 +1622,7 @@ static int ensure_segment_validity(const char *name) /* A refname component can not end with ".lock" */ if (current - name >= lock_len && - !git__strncmp(current - lock_len, GIT_FILELOCK_EXTENSION, lock_len)) + !memcmp(current - lock_len, GIT_FILELOCK_EXTENSION, lock_len)) return -1; return (int)(current - name); @@ -1697,11 +1697,10 @@ int git_reference__normalize_name( segments_count++; } - /* This means that there's a leading slash in the refname */ - if (segment_len == 0 && segments_count == 0) { + /* No empty segment is allowed when not normalizing */ + if (segment_len == 0 && !normalize) goto cleanup; - } - + if (current[segment_len] == '\0') break; diff --git a/src/remote.c b/src/remote.c index 920ca7a18..0a1f2b856 100644 --- a/src/remote.c +++ b/src/remote.c @@ -59,21 +59,9 @@ static int download_tags_value(git_remote *remote, git_config *cfg) static int ensure_remote_name_is_valid(const char *name) { - git_buf buf = GIT_BUF_INIT; - git_refspec refspec; - int error = -1; + int error = 0; - if (!name || *name == '\0') - goto cleanup; - - git_buf_printf(&buf, "refs/heads/test:refs/remotes/%s/test", name); - error = git_refspec__parse(&refspec, git_buf_cstr(&buf), true); - - git_buf_free(&buf); - git_refspec__free(&refspec); - -cleanup: - if (error) { + if (!git_remote_is_valid_name(name)) { giterr_set( GITERR_CONFIG, "'%s' is not a valid remote name.", name); @@ -1380,3 +1368,23 @@ void git_remote_set_update_fetchhead(git_remote *remote, int value) { remote->update_fetchhead = value; } + +int git_remote_is_valid_name( + const char *remote_name) +{ + git_buf buf = GIT_BUF_INIT; + git_refspec refspec; + int error = -1; + + if (!remote_name || *remote_name == '\0') + return 0; + + git_buf_printf(&buf, "refs/heads/test:refs/remotes/%s/test", remote_name); + error = git_refspec__parse(&refspec, git_buf_cstr(&buf), true); + + git_buf_free(&buf); + git_refspec__free(&refspec); + + giterr_clear(); + return error == 0; +} diff --git a/tests-clar/network/remote/isvalidname.c b/tests-clar/network/remote/isvalidname.c new file mode 100644 index 000000000..c26fbd0a5 --- /dev/null +++ b/tests-clar/network/remote/isvalidname.c @@ -0,0 +1,17 @@ +#include "clar_libgit2.h" + +void test_network_remote_isvalidname__can_detect_invalid_formats(void) +{ + cl_assert_equal_i(false, git_remote_is_valid_name("/")); + cl_assert_equal_i(false, git_remote_is_valid_name("//")); + cl_assert_equal_i(false, git_remote_is_valid_name(".lock")); + cl_assert_equal_i(false, git_remote_is_valid_name("a.lock")); + cl_assert_equal_i(false, git_remote_is_valid_name("/no/leading/slash")); + cl_assert_equal_i(false, git_remote_is_valid_name("no/trailing/slash/")); +} + +void test_network_remote_isvalidname__wont_hopefully_choke_on_valid_formats(void) +{ + cl_assert_equal_i(true, git_remote_is_valid_name("webmatrix")); + cl_assert_equal_i(true, git_remote_is_valid_name("yishaigalatzer/rules")); +} diff --git a/tests-clar/network/remote/remotes.c b/tests-clar/network/remote/remotes.c index e68de7f55..42f090b42 100644 --- a/tests-clar/network/remote/remotes.c +++ b/tests-clar/network/remote/remotes.c @@ -361,13 +361,27 @@ void test_network_remote_remotes__check_structure_version(void) cl_assert_equal_i(GITERR_INVALID, err->klass); } -void test_network_remote_remotes__cannot_create_a_remote_which_name_conflicts_with_an_existing_remote(void) +void assert_cannot_create_remote(const char *name, int expected_error) { git_remote *remote = NULL; - cl_assert_equal_i( - GIT_EEXISTS, - git_remote_create(&remote, _repo, "test", "git://github.com/libgit2/libgit2")); + cl_git_fail_with( + git_remote_create(&remote, _repo, name, "git://github.com/libgit2/libgit2"), + expected_error); cl_assert_equal_p(remote, NULL); } + +void test_network_remote_remotes__cannot_create_a_remote_which_name_conflicts_with_an_existing_remote(void) +{ + assert_cannot_create_remote("test", GIT_EEXISTS); +} + + +void test_network_remote_remotes__cannot_create_a_remote_which_name_is_invalid(void) +{ + assert_cannot_create_remote("/", GIT_EINVALIDSPEC); + assert_cannot_create_remote("//", GIT_EINVALIDSPEC); + assert_cannot_create_remote(".lock", GIT_EINVALIDSPEC); + assert_cannot_create_remote("a.lock", GIT_EINVALIDSPEC); +} diff --git a/tests-clar/refs/isvalidname.c b/tests-clar/refs/isvalidname.c index e9fbdbcd1..65c70ba4d 100644 --- a/tests-clar/refs/isvalidname.c +++ b/tests-clar/refs/isvalidname.c @@ -12,6 +12,7 @@ void test_refs_isvalidname__can_detect_invalid_formats(void) cl_assert_equal_i(false, git_reference_is_valid_name("lower_case")); cl_assert_equal_i(false, git_reference_is_valid_name("/stupid/name/master")); cl_assert_equal_i(false, git_reference_is_valid_name("/")); + cl_assert_equal_i(false, git_reference_is_valid_name("//")); cl_assert_equal_i(false, git_reference_is_valid_name("")); cl_assert_equal_i(false, git_reference_is_valid_name("refs/heads/sub.lock/webmatrix")); } diff --git a/tests-clar/refs/normalize.c b/tests-clar/refs/normalize.c index 562c34f06..7f313ef38 100644 --- a/tests-clar/refs/normalize.c +++ b/tests-clar/refs/normalize.c @@ -88,6 +88,8 @@ void test_refs_normalize__symbolic(void) GIT_REF_FORMAT_ALLOW_ONELEVEL, ""); ensure_refname_invalid( GIT_REF_FORMAT_ALLOW_ONELEVEL, "heads\foo"); + ensure_refname_invalid( + GIT_REF_FORMAT_ALLOW_ONELEVEL, "/"); ensure_refname_invalid( GIT_REF_FORMAT_ALLOW_ONELEVEL, "///");