diff options
| -rw-r--r-- | include/git2/remote.h | 22 | ||||
| -rw-r--r-- | src/remote.c | 67 | ||||
| -rw-r--r-- | tests-clar/network/remoterename.c | 4 | ||||
| -rw-r--r-- | tests-clar/network/remotes.c | 39 |
4 files changed, 94 insertions, 38 deletions
diff --git a/include/git2/remote.h b/include/git2/remote.h index 6c70d7fbc..0483cfc4b 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -39,30 +39,39 @@ typedef int (*git_remote_rename_problem_cb)(const char *problematic_refspec, voi * Create a remote with the default refspecs in memory. You can use * this when you have a URL instead of a remote's name. * + * The name, when provided, will be checked for validity. + * See `git_tag_create()` for rules about valid names. + * * @param out pointer to the new remote object * @param repo the associated repository - * @param name the remote's name + * @param name the optional remote's name * @param url the remote repository's URL * @param fetch the fetch refspec to use for this remote - * @return 0 or an error code + * @return 0, GIT_EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_remote_new(git_remote **out, git_repository *repo, const char *name, const char *url, const char *fetch); /** * Get the information for a particular remote * + * The name will be checked for validity. + * See `git_tag_create()` for rules about valid names. + * * @param out pointer to the new remote object * @param repo the associated repository * @param name the remote's name - * @return 0 or an error code + * @return 0, GIT_ENOTFOUND, GIT_EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_remote_load(git_remote **out, git_repository *repo, const char *name); /** * Save a remote to its repository's configuration * + * One can't save a nameless inmemory remote. Doing so will + * result in a GIT_EINVALIDSPEC being returned. + * * @param remote the remote to save to config - * @return 0 or an error code + * @return 0, GIT_EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_remote_save(const git_remote *remote); @@ -391,12 +400,15 @@ GIT_EXTERN(void) git_remote_set_autotag( * All remote-tracking branches and configuration settings * for the remote are updated. * + * The new name will be checked for validity. + * See `git_tag_create()` for rules about valid names. + * * @param remote the remote to rename * @param new_name the new name the remote should bear * @param callback Optional callback to notify the consumer of fetch refspecs * that haven't been automatically updated and need potential manual tweaking. * @param payload Additional data to pass to the callback - * @return 0 or an error code + * @return 0, GIT_EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_remote_rename( git_remote *remote, diff --git a/src/remote.c b/src/remote.c index c84911aa1..dc8d9681c 100644 --- a/src/remote.c +++ b/src/remote.c @@ -57,6 +57,32 @@ static int download_tags_value(git_remote *remote, git_config *cfg) return error; } +static int ensure_remote_name_is_valid(const char *name) +{ + git_buf buf = GIT_BUF_INIT; + git_refspec refspec; + int error = -1; + + 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) { + giterr_set( + GITERR_CONFIG, + "'%s' is not a valid remote name.", name); + error = GIT_EINVALIDSPEC; + } + + return error; +} + int git_remote_new(git_remote **out, git_repository *repo, const char *name, const char *url, const char *fetch) { git_remote *remote; @@ -79,6 +105,12 @@ int git_remote_new(git_remote **out, git_repository *repo, const char *name, con GITERR_CHECK_ALLOC(remote->url); if (name != NULL) { + int error; + if ((error = ensure_remote_name_is_valid(name)) < 0) { + git_remote_free(remote); + return error; + } + remote->name = git__strdup(name); GITERR_CHECK_ALLOC(remote->name); } @@ -111,6 +143,9 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name) assert(out && repo && name); + if ((error = ensure_remote_name_is_valid(name)) < 0) + return error; + if (git_repository_config__weakptr(&config, repo) < 0) return -1; @@ -212,30 +247,6 @@ cleanup: return error; } -static int ensure_remote_name_is_valid(const char *name) -{ - git_buf buf = GIT_BUF_INIT; - git_refspec refspec; - int error = -1; - - 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) - giterr_set( - GITERR_CONFIG, - "'%s' is not a valid remote name.", name); - - return error; -} - static int update_config_refspec( git_config *config, const char *remote_name, @@ -279,8 +290,8 @@ int git_remote_save(const git_remote *remote) assert(remote); - if (ensure_remote_name_is_valid(remote->name) < 0) - return -1; + if ((error = ensure_remote_name_is_valid(remote->name)) < 0) + return error; if (git_repository_config__weakptr(&config, remote->repo) < 0) return -1; @@ -958,6 +969,10 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo) int git_remote_add(git_remote **out, git_repository *repo, const char *name, const char *url) { git_buf buf = GIT_BUF_INIT; + int error; + + if ((error = ensure_remote_name_is_valid(name)) < 0) + return error; if (git_buf_printf(&buf, "+refs/heads/*:refs/remotes/%s/*", name) < 0) return -1; diff --git a/tests-clar/network/remoterename.c b/tests-clar/network/remoterename.c index b14554572..bd582314d 100644 --- a/tests-clar/network/remoterename.c +++ b/tests-clar/network/remoterename.c @@ -121,7 +121,9 @@ void test_network_remoterename__new_name_can_contain_dots(void) void test_network_remoterename__new_name_must_conform_to_reference_naming_conventions(void) { - cl_git_fail(git_remote_rename(_remote, "new@{name", dont_call_me_cb, NULL)); + cl_assert_equal_i( + GIT_EINVALIDSPEC, + git_remote_rename(_remote, "new@{name", dont_call_me_cb, NULL)); } void test_network_remoterename__renamed_name_is_persisted(void) diff --git a/tests-clar/network/remotes.c b/tests-clar/network/remotes.c index 14fda1670..b4a3ad99d 100644 --- a/tests-clar/network/remotes.c +++ b/tests-clar/network/remotes.c @@ -202,6 +202,11 @@ void test_network_remotes__loading_a_missing_remote_returns_ENOTFOUND(void) cl_assert_equal_i(GIT_ENOTFOUND, git_remote_load(&_remote, _repo, "just-left-few-minutes-ago")); } +void test_network_remotes__loading_with_an_invalid_name_returns_EINVALIDSPEC(void) +{ + cl_assert_equal_i(GIT_EINVALIDSPEC, git_remote_load(&_remote, _repo, "Inv@{id")); +} + /* * $ git remote add addtest http://github.com/libgit2/libgit2 * @@ -229,8 +234,9 @@ void test_network_remotes__cannot_add_a_nameless_remote(void) { git_remote *remote; - cl_git_fail(git_remote_add(&remote, _repo, NULL, "git://github.com/libgit2/libgit2")); - cl_git_fail(git_remote_add(&remote, _repo, "", "git://github.com/libgit2/libgit2")); + cl_assert_equal_i( + GIT_EINVALIDSPEC, + git_remote_add(&remote, _repo, NULL, "git://github.com/libgit2/libgit2")); } void test_network_remotes__cannot_save_a_nameless_remote(void) @@ -239,13 +245,34 @@ void test_network_remotes__cannot_save_a_nameless_remote(void) cl_git_pass(git_remote_new(&remote, _repo, NULL, "git://github.com/libgit2/libgit2", NULL)); - cl_git_fail(git_remote_save(remote)); + cl_assert_equal_i(GIT_EINVALIDSPEC, git_remote_save(remote)); git_remote_free(remote); +} - cl_git_pass(git_remote_new(&remote, _repo, "", "git://github.com/libgit2/libgit2", NULL)); +void test_network_remotes__cannot_add_a_remote_with_an_invalid_name(void) +{ + git_remote *remote; - cl_git_fail(git_remote_save(remote)); - git_remote_free(remote); + cl_assert_equal_i( + GIT_EINVALIDSPEC, + git_remote_add(&remote, _repo, "Inv@{id", "git://github.com/libgit2/libgit2")); + + cl_assert_equal_i( + GIT_EINVALIDSPEC, + git_remote_add(&remote, _repo, "", "git://github.com/libgit2/libgit2")); +} + +void test_network_remotes__cannot_initialize_a_remote_with_an_invalid_name(void) +{ + git_remote *remote; + + cl_assert_equal_i( + GIT_EINVALIDSPEC, + git_remote_new(&remote, _repo, "Inv@{id", "git://github.com/libgit2/libgit2", NULL)); + + cl_assert_equal_i( + GIT_EINVALIDSPEC, + git_remote_new(&remote, _repo, "", "git://github.com/libgit2/libgit2", NULL)); } void test_network_remotes__tagopt(void) |