diff --git a/include/git2/refs.h b/include/git2/refs.h index c92646115..7d047ca49 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -26,13 +26,13 @@ GIT_BEGIN_DECL * * The returned reference must be freed by the user. * - * See `git_reference_create_symbolic()` for documentation about valid - * reference names. + * The name will be checked for validity. + * See `git_reference_create_symbolic()` for rules about valid names. * * @param out pointer to the looked-up reference * @param repo the repository to look up the reference * @param name the long name for the reference (e.g. HEAD, refs/heads/master, refs/tags/v0.1.0, ...) - * @return 0 or an error code (ENOTFOUND, EINVALIDSPEC) + * @return 0 on success, ENOTFOUND, EINVALIDSPEC or an error code. */ GIT_EXTERN(int) git_reference_lookup(git_reference **out, git_repository *repo, const char *name); @@ -43,11 +43,13 @@ GIT_EXTERN(int) git_reference_lookup(git_reference **out, git_repository *repo, * through to the object id that it refers to. This avoids having to * allocate or free any `git_reference` objects for simple situations. * + * The name will be checked for validity. + * See `git_reference_create_symbolic()` for rules about valid names. + * * @param out Pointer to oid to be filled in * @param repo The repository in which to look up the reference * @param name The long name for the reference - * @return 0 on success, -1 if name could not be resolved (EINVALIDSPEC, - * ENOTFOUND, etc) + * @return 0 on success, ENOTFOUND, EINVALIDSPEC or an error code. */ GIT_EXTERN(int) git_reference_name_to_id( git_oid *out, git_repository *repo, const char *name); @@ -79,7 +81,7 @@ GIT_EXTERN(int) git_reference_name_to_id( * @param name The name of the reference * @param target The target of the reference * @param force Overwrite existing references - * @return 0 or an error code (EEXISTS, EINVALIDSPEC) + * @return 0 on success, EEXISTS, EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_reference_symbolic_create(git_reference **out, git_repository *repo, const char *name, const char *target, int force); @@ -111,7 +113,7 @@ GIT_EXTERN(int) git_reference_symbolic_create(git_reference **out, git_repositor * @param name The name of the reference * @param id The object id pointed to by the reference. * @param force Overwrite existing references - * @return 0 or an error code (EINVALIDSPEC, EEXISTS) + * @return 0 on success, EEXISTS, EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_reference_create(git_reference **out, git_repository *repo, const char *name, const git_oid *id, int force); @@ -193,9 +195,12 @@ GIT_EXTERN(git_repository *) git_reference_owner(const git_reference *ref); * * The reference will be automatically updated in memory and on disk. * + * The target name will be checked for validity. + * See `git_reference_create_symbolic()` for rules about valid names. + * * @param ref The reference * @param target The new target for the reference - * @return 0 or an error code (EINVALIDSPEC) + * @return 0 on success, EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_reference_symbolic_set_target(git_reference *ref, const char *target); @@ -216,8 +221,9 @@ GIT_EXTERN(int) git_reference_set_target(git_reference *ref, const git_oid *id); * Rename an existing reference. * * This method works for both direct and symbolic references. - * The new name will be checked for validity and may be - * modified into a normalized form. + * + * The new name will be checked for validity. + * See `git_reference_create_symbolic()` for rules about valid names. * * The given git_reference will be updated in place. * @@ -234,7 +240,7 @@ GIT_EXTERN(int) git_reference_set_target(git_reference *ref, const git_oid *id); * @param ref The reference to rename * @param name The new name for the reference * @param force Overwrite an existing reference - * @return 0 or an error code (EINVALIDSPEC, EEXISTS) + * @return 0 on success, EINVALIDSPEC, EEXISTS or an error code * */ GIT_EXTERN(int) git_reference_rename(git_reference *ref, const char *name, int force); @@ -446,13 +452,15 @@ typedef enum { * Once normalized, if the reference name is valid, it will be returned in * the user allocated buffer. * + * See `git_reference_create_symbolic()` for rules about valid names. + * * @param buffer_out User allocated buffer to store normalized name * @param buffer_size Size of buffer_out * @param name Reference name to be checked. * @param flags Flags to constrain name validation rules - see the * GIT_REF_FORMAT constants above. - * @return 0 on success or error code (GIT_EBUFS if buffer is too small, -1 - * if reference is invalid) + * @return 0 on success, GIT_EBUFS if buffer is too small, EINVALIDSPEC + * or an error code. */ GIT_EXTERN(int) git_reference_normalize_name( char *buffer_out, diff --git a/src/refs.c b/src/refs.c index 61a515c94..85813096b 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1215,11 +1215,11 @@ int git_reference_symbolic_create( git_reference *ref = NULL; int error; - if (git_reference__normalize_name_lax( + if ((error = git_reference__normalize_name_lax( normalized, sizeof(normalized), - name) < 0) - return -1; + name)) < 0) + return error; if ((error = reference_can_write(repo, normalized, NULL, force)) < 0) return error; @@ -1255,11 +1255,11 @@ int git_reference_create( git_reference *ref = NULL; char normalized[GIT_REFNAME_MAX]; - if (git_reference__normalize_name_lax( + if ((error = git_reference__normalize_name_lax( normalized, sizeof(normalized), - name) < 0) - return -1; + name)) < 0) + return error; if ((error = reference_can_write(repo, normalized, NULL, force)) < 0) return error; @@ -1330,6 +1330,7 @@ int git_reference_set_target(git_reference *ref, const git_oid *id) */ int git_reference_symbolic_set_target(git_reference *ref, const char *target) { + int error; char normalized[GIT_REFNAME_MAX]; if ((ref->flags & GIT_REF_SYMBOLIC) == 0) { @@ -1338,11 +1339,11 @@ int git_reference_symbolic_set_target(git_reference *ref, const char *target) return -1; } - if (git_reference__normalize_name_lax( + if ((error = git_reference__normalize_name_lax( normalized, sizeof(normalized), - target)) - return -1; + target)) < 0) + return error; git__free(ref->target.symbolic); ref->target.symbolic = git__strdup(normalized); @@ -1363,12 +1364,12 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force) GIT_REF_FORMAT_ALLOW_ONELEVEL : GIT_REF_FORMAT_NORMAL; - if (git_reference_normalize_name( + if ((result = git_reference_normalize_name( normalized, sizeof(normalized), new_name, - normalization_flags) < 0) - return -1; + normalization_flags)) < 0) + return result; if ((result = reference_can_write(ref->owner, normalized, ref->name, force)) < 0) return result; @@ -1645,7 +1646,7 @@ int git_reference__normalize_name( // Inspired from https://github.com/git/git/blob/f06d47e7e0d9db709ee204ed13a8a7486149f494/refs.c#L36-100 char *current; - int segment_len, segments_count = 0, error = -1; + int segment_len, segments_count = 0, error = GIT_EINVALIDSPEC; unsigned int process_flags; bool normalize = (buf != NULL); assert(name); @@ -1677,8 +1678,10 @@ int git_reference__normalize_name( git_buf_truncate(buf, cur_len + segment_len + (segments_count ? 1 : 0)); - if (git_buf_oom(buf)) + if (git_buf_oom(buf)) { + error = -1; goto cleanup; + } } segments_count++; @@ -1721,7 +1724,7 @@ int git_reference__normalize_name( error = 0; cleanup: - if (error) + if (error == GIT_EINVALIDSPEC) giterr_set( GITERR_REFERENCE, "The given reference name '%s' is not valid", name); diff --git a/tests-clar/refs/create.c b/tests-clar/refs/create.c index d22f04939..56c323d8a 100644 --- a/tests-clar/refs/create.c +++ b/tests-clar/refs/create.c @@ -149,3 +149,19 @@ void test_refs_create__propagate_eexists(void) error = git_reference_symbolic_create(&ref, g_repo, "HEAD", current_head_target, false); cl_assert(error == GIT_EEXISTS); } + +void test_refs_create__creating_a_reference_with_an_invalid_name_returns_EINVALIDSPEC(void) +{ + git_reference *new_reference; + git_oid id; + + const char *name = "refs/heads/inv@{id"; + + git_oid_fromstr(&id, current_master_tip); + + cl_assert_equal_i(GIT_EINVALIDSPEC, git_reference_create( + &new_reference, g_repo, name, &id, 0)); + + cl_assert_equal_i(GIT_EINVALIDSPEC, git_reference_symbolic_create( + &new_reference, g_repo, name, current_head_target, 0)); +} diff --git a/tests-clar/refs/normalize.c b/tests-clar/refs/normalize.c index a144ef5c0..870a533ca 100644 --- a/tests-clar/refs/normalize.c +++ b/tests-clar/refs/normalize.c @@ -21,7 +21,9 @@ static void ensure_refname_invalid(unsigned int flags, const char *input_refname { char buffer_out[GIT_REFNAME_MAX]; - cl_git_fail(git_reference_normalize_name(buffer_out, sizeof(buffer_out), input_refname, flags)); + cl_assert_equal_i( + GIT_EINVALIDSPEC, + git_reference_normalize_name(buffer_out, sizeof(buffer_out), input_refname, flags)); } void test_refs_normalize__can_normalize_a_direct_reference_name(void) diff --git a/tests-clar/refs/read.c b/tests-clar/refs/read.c index c10a540c0..aa7f01d57 100644 --- a/tests-clar/refs/read.c +++ b/tests-clar/refs/read.c @@ -224,10 +224,14 @@ void test_refs_read__unfound_return_ENOTFOUND(void) { git_reference *reference; - cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&reference, g_repo, "TEST_MASTER")); - cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&reference, g_repo, "refs/test/master")); - cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&reference, g_repo, "refs/tags/test/master")); - cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&reference, g_repo, "refs/tags/test/farther/master")); + cl_assert_equal_i(GIT_ENOTFOUND, + git_reference_lookup(&reference, g_repo, "TEST_MASTER")); + cl_assert_equal_i(GIT_ENOTFOUND, + git_reference_lookup(&reference, g_repo, "refs/test/master")); + cl_assert_equal_i(GIT_ENOTFOUND, + git_reference_lookup(&reference, g_repo, "refs/tags/test/master")); + cl_assert_equal_i(GIT_ENOTFOUND, + git_reference_lookup(&reference, g_repo, "refs/tags/test/farther/master")); } static void assert_is_branch(const char *name, bool expected_branchness) @@ -245,3 +249,15 @@ void test_refs_read__can_determine_if_a_reference_is_a_local_branch(void) assert_is_branch("refs/remotes/test/master", false); assert_is_branch("refs/tags/e90810b", false); } + +void test_refs_read__invalid_name_returns_EINVALIDSPEC(void) +{ + git_reference *reference; + git_oid id; + + cl_assert_equal_i(GIT_EINVALIDSPEC, + git_reference_lookup(&reference, g_repo, "refs/heads/Inv@{id")); + + cl_assert_equal_i(GIT_EINVALIDSPEC, + git_reference_name_to_id(&id, g_repo, "refs/heads/Inv@{id")); +} diff --git a/tests-clar/refs/rename.c b/tests-clar/refs/rename.c index ec5c12507..bfdef15fa 100644 --- a/tests-clar/refs/rename.c +++ b/tests-clar/refs/rename.c @@ -180,10 +180,14 @@ void test_refs_rename__invalid_name(void) cl_git_pass(git_reference_lookup(&looked_up_ref, g_repo, packed_test_head_name)); /* Can not be renamed with an invalid name. */ - cl_git_fail(git_reference_rename(looked_up_ref, "Hello! I'm a very invalid name.", 0)); + cl_assert_equal_i( + GIT_EINVALIDSPEC, + git_reference_rename(looked_up_ref, "Hello! I'm a very invalid name.", 0)); - /* Can not be renamed outside of the refs hierarchy. */ - cl_git_fail(git_reference_rename(looked_up_ref, "i-will-sudo-you", 0)); + /* Can not be renamed outside of the refs hierarchy + * unless it's ALL_CAPS_AND_UNDERSCORES. + */ + cl_assert_equal_i(GIT_EINVALIDSPEC, git_reference_rename(looked_up_ref, "i-will-sudo-you", 0)); /* Failure to rename it hasn't corrupted its state */ git_reference_free(looked_up_ref); diff --git a/tests-clar/refs/update.c b/tests-clar/refs/update.c new file mode 100644 index 000000000..6c2107ee2 --- /dev/null +++ b/tests-clar/refs/update.c @@ -0,0 +1,29 @@ +#include "clar_libgit2.h" + +#include "refs.h" + +static git_repository *g_repo; + +void test_refs_update__initialize(void) +{ + g_repo = cl_git_sandbox_init("testrepo.git"); +} + +void test_refs_update__cleanup(void) +{ + cl_git_sandbox_cleanup(); +} + +void test_refs_update__updating_the_target_of_a_symref_with_an_invalid_name_returns_EINVALIDSPEC(void) +{ + git_reference *head; + + cl_git_pass(git_reference_lookup(&head, g_repo, GIT_HEAD_FILE)); + + cl_assert_equal_i(GIT_REF_SYMBOLIC, git_reference_type(head)); + + cl_assert_equal_i(GIT_EINVALIDSPEC, git_reference_symbolic_set_target( + head, "refs/heads/inv@{id")); + + git_reference_free(head); +}