From e4f98ac4b81102cf16b2029bc753e9f924fdbff0 Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 12:00:23 +0100 Subject: tests: follow clar naming convention --- tests/refs/branches/name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/refs/branches/name.c b/tests/refs/branches/name.c index 290916eec..efa68e32b 100644 --- a/tests/refs/branches/name.c +++ b/tests/refs/branches/name.c @@ -51,7 +51,7 @@ static int name_is_valid(const char *name) return valid; } -void test_refs_branches_is_name_valid(void) +void test_refs_branches_name__is_name_valid(void) { cl_assert_equal_i(true, name_is_valid("master")); cl_assert_equal_i(true, name_is_valid("test/master")); -- cgit v1.2.1 From 71fafae17a9a82e444c318db9346a6fb9a3d042c Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 12:03:37 +0100 Subject: tests: error when create branch with invalid name --- tests/refs/branches/create.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/refs/branches/create.c b/tests/refs/branches/create.c index 70ffad88d..c5b45f7e8 100644 --- a/tests/refs/branches/create.c +++ b/tests/refs/branches/create.c @@ -277,3 +277,11 @@ void test_refs_branches_create__name_vs_namespace_fail(void) branch = NULL; } } + +void test_refs_branches_create__error_when_create_branch_with_invalid_name(void) +{ + retrieve_known_commit(&target, repo); + + cl_git_fail(git_branch_create(&branch, repo, "HEAD", target, 0)); + cl_git_fail(git_branch_create(&branch, repo, "-dash", target, 0)); +} -- cgit v1.2.1 From 391afec497364e5c945cd1d78aa29e9b8803d9e1 Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 13:54:38 +0100 Subject: branch: refactor branch name validity checks --- src/branch.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/branch.c b/src/branch.c index e6818a86d..69be8c796 100644 --- a/src/branch.c +++ b/src/branch.c @@ -52,6 +52,11 @@ static int not_a_local_branch(const char *reference_name) return -1; } +static bool branch_name_follows_pattern(const char *branch_name) +{ + return branch_name[0] != '-' && git__strcmp(branch_name, "HEAD"); +} + static int create_branch( git_reference **ref_out, git_repository *repository, @@ -72,7 +77,7 @@ static int create_branch( GIT_ASSERT_ARG(ref_out); GIT_ASSERT_ARG(git_commit_owner(commit) == repository); - if (!git__strcmp(branch_name, "HEAD")) { + if (!branch_name_follows_pattern(branch_name)) { git_error_set(GIT_ERROR_REFERENCE, "'HEAD' is not a valid branch name"); error = -1; goto cleanup; @@ -762,7 +767,7 @@ int git_branch_name_is_valid(int *valid, const char *name) * and discourage HEAD as branch name, * https://github.com/git/git/commit/a625b092cc5994 */ - if (!name || name[0] == '-' || !git__strcmp(name, "HEAD")) + if (!name || !branch_name_follows_pattern(name)) goto done; if ((error = git_buf_puts(&ref_name, GIT_REFS_HEADS_DIR)) < 0 || -- cgit v1.2.1 From 2e9228e85a49569f294915c648817d9e4190a55c Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 15:23:41 +0100 Subject: tests: rename test for consistency --- tests/refs/branches/name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/refs/branches/name.c b/tests/refs/branches/name.c index efa68e32b..ea1cf30bb 100644 --- a/tests/refs/branches/name.c +++ b/tests/refs/branches/name.c @@ -51,7 +51,7 @@ static int name_is_valid(const char *name) return valid; } -void test_refs_branches_name__is_name_valid(void) +void test_refs_branches_name__name_is_valid(void) { cl_assert_equal_i(true, name_is_valid("master")); cl_assert_equal_i(true, name_is_valid("test/master")); -- cgit v1.2.1 From 724b5a0e97e54a0cab5422caf4d4244af27bdca7 Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 15:24:34 +0100 Subject: tests: rename to follow clar naming convention --- tests/refs/tags/name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/refs/tags/name.c b/tests/refs/tags/name.c index 0ca5df7d6..809a9dbbb 100644 --- a/tests/refs/tags/name.c +++ b/tests/refs/tags/name.c @@ -7,7 +7,7 @@ static int name_is_valid(const char *name) return valid; } -void test_refs_tags_is_name_valid(void) +void test_refs_tags__name_is_valid(void) { cl_assert_equal_i(true, name_is_valid("sometag")); cl_assert_equal_i(true, name_is_valid("test/sometag")); -- cgit v1.2.1 From 7560ac4d2f19906729963cbdf7c8c7fb675b8f8a Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 15:25:51 +0100 Subject: branches: fix error message for invalid name --- src/branch.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/branch.c b/src/branch.c index 69be8c796..22e7ba82b 100644 --- a/src/branch.c +++ b/src/branch.c @@ -54,6 +54,12 @@ static int not_a_local_branch(const char *reference_name) static bool branch_name_follows_pattern(const char *branch_name) { + /* + * Discourage branch name starting with dash, + * https://github.com/git/git/commit/6348624010888b + * and discourage HEAD as branch name, + * https://github.com/git/git/commit/a625b092cc5994 + */ return branch_name[0] != '-' && git__strcmp(branch_name, "HEAD"); } @@ -78,7 +84,7 @@ static int create_branch( GIT_ASSERT_ARG(git_commit_owner(commit) == repository); if (!branch_name_follows_pattern(branch_name)) { - git_error_set(GIT_ERROR_REFERENCE, "'HEAD' is not a valid branch name"); + git_error_set(GIT_ERROR_REFERENCE, "'%s' is not a valid branch name", branch_name); error = -1; goto cleanup; } @@ -761,12 +767,6 @@ int git_branch_name_is_valid(int *valid, const char *name) *valid = 0; - /* - * Discourage branch name starting with dash, - * https://github.com/git/git/commit/6348624010888b - * and discourage HEAD as branch name, - * https://github.com/git/git/commit/a625b092cc5994 - */ if (!name || !branch_name_follows_pattern(name)) goto done; -- cgit v1.2.1 From fe9bfec46b90443d8c64990858e5b31fbd4a7a2f Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 15:35:15 +0100 Subject: tag: refactor tag name validity checks --- src/tag.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/tag.c b/src/tag.c index ee91e5091..f9b647439 100644 --- a/src/tag.c +++ b/src/tag.c @@ -244,6 +244,15 @@ on_error: return -1; } +static bool tag_name_follows_pattern(const char *tag_name) +{ + /* + * Discourage tag name starting with dash, + * https://github.com/git/git/commit/4f0accd638b8d2 + */ + return tag_name[0] != '-'; +} + static int git_tag_create__internal( git_oid *oid, git_repository *repo, @@ -269,6 +278,11 @@ static int git_tag_create__internal( return -1; } + if (!tag_name_follows_pattern(tag_name)) { + git_error_set(GIT_ERROR_TAG, "'%s' is not a valid tag name", tag_name); + return -1; + } + error = retrieve_tag_reference_oid(oid, &ref_name, repo, tag_name); if (error < 0 && error != GIT_ENOTFOUND) goto cleanup; @@ -540,11 +554,7 @@ int git_tag_name_is_valid(int *valid, const char *name) GIT_ASSERT(valid); - /* - * Discourage tag name starting with dash, - * https://github.com/git/git/commit/4f0accd638b8d2 - */ - if (!name || name[0] == '-') + if (!name || !tag_name_follows_pattern(name)) goto done; if ((error = git_buf_puts(&ref_name, GIT_REFS_TAGS_DIR)) < 0 || -- cgit v1.2.1 From 7456e819341c56f301852ede0770c203e1dfee1f Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 15:42:16 +0100 Subject: tests: error when create tag with invalid name --- tests/object/tag/write.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/object/tag/write.c b/tests/object/tag/write.c index 48c8bfd36..118f39894 100644 --- a/tests/object/tag/write.c +++ b/tests/object/tag/write.c @@ -258,3 +258,19 @@ void test_object_tag_write__creating_an_annotation_does_not_create_a_reference(v create_annotation(&tag_id, "new_tag"); cl_git_fail_with(git_reference_lookup(&tag_ref, g_repo, "refs/tags/new_tag"), GIT_ENOTFOUND); } + +void test_object_tag_write__error_when_create_tag_with_invalid_name(void) +{ + git_oid target_id, tag_id; + git_signature *tagger; + git_object *target; + + git_oid_fromstr(&target_id, tagged_commit); + cl_git_pass(git_object_lookup(&target, g_repo, &target_id, GIT_OBJECT_COMMIT)); + cl_git_pass(git_signature_new(&tagger, tagger_name, tagger_email, 123456789, 60)); + + cl_git_fail( + git_tag_create(&tag_id, g_repo, + "-dash", target, tagger, tagger_message, 0) + ); +} -- cgit v1.2.1 From 1912f145e1e183f850d7611c0bc7e609cbc0a87a Mon Sep 17 00:00:00 2001 From: yuangli Date: Mon, 11 Jul 2022 15:53:35 +0100 Subject: tests: free resources in invalid tag name test --- tests/object/tag/write.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/object/tag/write.c b/tests/object/tag/write.c index 118f39894..4690fd83e 100644 --- a/tests/object/tag/write.c +++ b/tests/object/tag/write.c @@ -273,4 +273,7 @@ void test_object_tag_write__error_when_create_tag_with_invalid_name(void) git_tag_create(&tag_id, g_repo, "-dash", target, tagger, tagger_message, 0) ); + + git_object_free(target); + git_signature_free(tagger); } -- cgit v1.2.1 From f6be8c2697724688eae780eb63ac4dc97423858b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 Jul 2022 22:09:25 -0400 Subject: Apply suggestions from code review --- src/libgit2/branch.c | 6 +++--- src/libgit2/tag.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libgit2/branch.c b/src/libgit2/branch.c index 5660a1dc4..2dd7d2bb4 100644 --- a/src/libgit2/branch.c +++ b/src/libgit2/branch.c @@ -53,7 +53,7 @@ static int not_a_local_branch(const char *reference_name) return -1; } -static bool branch_name_follows_pattern(const char *branch_name) +static bool branch_name_is_valid(const char *branch_name) { /* * Discourage branch name starting with dash, @@ -84,7 +84,7 @@ static int create_branch( GIT_ASSERT_ARG(ref_out); GIT_ASSERT_ARG(git_commit_owner(commit) == repository); - if (!branch_name_follows_pattern(branch_name)) { + if (!branch_name_is_valid(branch_name)) { git_error_set(GIT_ERROR_REFERENCE, "'%s' is not a valid branch name", branch_name); error = -1; goto cleanup; @@ -808,7 +808,7 @@ int git_branch_name_is_valid(int *valid, const char *name) *valid = 0; - if (!name || !branch_name_follows_pattern(name)) + if (!name || !branch_name_is_valid(name)) goto done; if ((error = git_str_puts(&ref_name, GIT_REFS_HEADS_DIR)) < 0 || diff --git a/src/libgit2/tag.c b/src/libgit2/tag.c index b3503368e..3d8f8bf0a 100644 --- a/src/libgit2/tag.c +++ b/src/libgit2/tag.c @@ -244,7 +244,7 @@ on_error: return -1; } -static bool tag_name_follows_pattern(const char *tag_name) +static bool tag_name_is_valid(const char *tag_name) { /* * Discourage tag name starting with dash, @@ -278,7 +278,7 @@ static int git_tag_create__internal( return -1; } - if (!tag_name_follows_pattern(tag_name)) { + if (!tag_name_is_valid(tag_name)) { git_error_set(GIT_ERROR_TAG, "'%s' is not a valid tag name", tag_name); return -1; } @@ -554,7 +554,7 @@ int git_tag_name_is_valid(int *valid, const char *name) GIT_ASSERT(valid); - if (!name || !tag_name_follows_pattern(name)) + if (!name || !tag_name_is_valid(name)) goto done; if ((error = git_str_puts(&ref_name, GIT_REFS_TAGS_DIR)) < 0 || -- cgit v1.2.1 From be08ef7fd7d045a4bd0e3dfc059b32c1590b5016 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 12 Jul 2022 22:39:25 -0400 Subject: Update src/libgit2/tag.c --- src/libgit2/tag.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libgit2/tag.c b/src/libgit2/tag.c index 3d8f8bf0a..792155a4b 100644 --- a/src/libgit2/tag.c +++ b/src/libgit2/tag.c @@ -554,6 +554,8 @@ int git_tag_name_is_valid(int *valid, const char *name) GIT_ASSERT(valid); + *valid = 0; + if (!name || !tag_name_is_valid(name)) goto done; -- cgit v1.2.1