diff options
33 files changed, 487 insertions, 214 deletions
diff --git a/include/git2/branch.h b/include/git2/branch.h index c9ae9cc5d..f55903cd6 100644 --- a/include/git2/branch.h +++ b/include/git2/branch.h @@ -29,6 +29,9 @@ GIT_BEGIN_DECL * * The returned reference must be freed by the user. * + * The branch name will be checked for validity. + * See `git_tag_create()` for rules about valid names. + * * @param out Pointer where to store the underlying reference. * * @param branch_name Name for the branch; this name is @@ -42,7 +45,7 @@ GIT_BEGIN_DECL * * @param force Overwrite existing branch. * - * @return 0 or an error code. + * @return 0, GIT_EINVALIDSPEC or an error code. * A proper reference is written in the refs/heads namespace * pointing to the provided target commit. */ @@ -94,6 +97,9 @@ GIT_EXTERN(int) git_branch_foreach( /** * Move/rename an existing local branch reference. * + * The new branch name will be checked for validity. + * See `git_tag_create()` for rules about valid names. + * * @param branch Current underlying reference of the branch. * * @param new_branch_name Target name of the branch once the move @@ -101,7 +107,7 @@ GIT_EXTERN(int) git_branch_foreach( * * @param force Overwrite existing branch. * - * @return 0 on success, or an error code. + * @return 0 on success, GIT_EINVALIDSPEC or an error code. */ GIT_EXTERN(int) git_branch_move( git_reference *branch, @@ -113,6 +119,9 @@ GIT_EXTERN(int) git_branch_move( * * The generated reference must be freed by the user. * + * The branch name will be checked for validity. + * See `git_tag_create()` for rules about valid names. + * * @param out pointer to the looked-up branch reference * * @param repo the repository to look up the branch @@ -124,7 +133,7 @@ GIT_EXTERN(int) git_branch_move( * be valued with either GIT_BRANCH_LOCAL or GIT_BRANCH_REMOTE. * * @return 0 on success; GIT_ENOTFOUND when no matching branch - * exists, otherwise an error code. + * exists, GIT_EINVALIDSPEC, otherwise an error code. */ GIT_EXTERN(int) git_branch_lookup( git_reference **out, diff --git a/include/git2/errors.h b/include/git2/errors.h index 9dd42f0c4..63b6bc8ee 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -30,6 +30,7 @@ enum { GIT_EORPHANEDHEAD = -9, GIT_EUNMERGED = -10, GIT_ENONFASTFORWARD = -11, + GIT_EINVALIDSPEC = -12, GIT_PASSTHROUGH = -30, GIT_ITEROVER = -31, diff --git a/include/git2/object.h b/include/git2/object.h index fcc56cb27..e5ca17e16 100644 --- a/include/git2/object.h +++ b/include/git2/object.h @@ -179,8 +179,9 @@ GIT_EXTERN(size_t) git_object__size(git_otype type); * * @param peeled Pointer to the peeled git_object * @param object The object to be processed - * @param target_type The type of the requested object - * @return 0 or an error code + * @param target_type The type of the requested object (GIT_OBJ_COMMIT, + * GIT_OBJ_TAG, GIT_OBJ_TREE, GIT_OBJ_BLOB or GIT_OBJ_ANY). + * @return 0 on success, GIT_EAMBIGUOUS, GIT_ENOTFOUND or an error code */ GIT_EXTERN(int) git_object_peel( git_object **peeled, diff --git a/include/git2/reflog.h b/include/git2/reflog.h index 45dff2165..418826d1d 100644 --- a/include/git2/reflog.h +++ b/include/git2/reflog.h @@ -63,9 +63,12 @@ GIT_EXTERN(int) git_reflog_append(git_reflog *reflog, const git_oid *id, const g * * The reflog to be renamed is expected to already exist * + * The new name will be checked for validity. + * See `git_reference_create_symbolic()` for rules about valid names. + * * @param ref the reference * @param name the new name of the reference - * @return 0 or an error code + * @return 0 on success, GIT_EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_reflog_rename(git_reference *ref, const char *name); diff --git a/include/git2/refs.h b/include/git2/refs.h index c92646115..f3082102f 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, @@ -471,8 +479,9 @@ GIT_EXTERN(int) git_reference_normalize_name( * * @param peeled Pointer to the peeled git_object * @param ref The reference to be processed - * @param target_type The type of the requested object - * @return 0 or an error code + * @param target_type The type of the requested object (GIT_OBJ_COMMIT, + * GIT_OBJ_TAG, GIT_OBJ_TREE, GIT_OBJ_BLOB or GIT_OBJ_ANY). + * @return 0 on success, GIT_EAMBIGUOUS, GIT_ENOTFOUND or an error code */ GIT_EXTERN(int) git_reference_peel( git_object **out, 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/include/git2/revparse.h b/include/git2/revparse.h index 4567027e5..3df6fef7f 100644 --- a/include/git2/revparse.h +++ b/include/git2/revparse.h @@ -27,7 +27,8 @@ GIT_BEGIN_DECL * @param out pointer to output object * @param repo the repository to search in * @param spec the textual specification for an object - * @return on success, GIT_ERROR otherwise (use git_error_last for information about the error) + * @return 0 on success, GIT_ENOTFOUND, GIT_EAMBIGUOUS, + * GIT_EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_revparse_single(git_object **out, git_repository *repo, const char *spec); diff --git a/include/git2/tag.h b/include/git2/tag.h index f82a6c9f9..a9773be56 100644 --- a/include/git2/tag.h +++ b/include/git2/tag.h @@ -144,6 +144,10 @@ GIT_EXTERN(const char *) git_tag_message(const git_tag *tag); * The message will not be cleaned up. This can be achieved * through `git_message_prettify()`. * + * The tag name will be checked for validity. You must avoid + * the characters '~', '^', ':', '\\', '?', '[', and '*', and the + * sequences ".." and "@{" which have special meaning to revparse. + * * @param oid Pointer where to store the OID of the * newly created tag. If the tag already exists, this parameter * will be the oid of the existing tag, and the function will @@ -165,7 +169,7 @@ GIT_EXTERN(const char *) git_tag_message(const git_tag *tag); * * @param force Overwrite existing references * - * @return 0 or an error code + * @return 0 on success, GIT_EINVALIDSPEC or an error code * A tag object is written to the ODB, and a proper reference * is written in the /refs/tags folder, pointing to it */ @@ -200,6 +204,9 @@ GIT_EXTERN(int) git_tag_create_frombuffer( * this target object. If `force` is true and a reference * already exists with the given name, it'll be replaced. * + * The tag name will be checked for validity. + * See `git_tag_create()` for rules about valid names. + * * @param oid Pointer where to store the OID of the provided * target object. If the tag already exists, this parameter * will be filled with the oid of the existing pointed object @@ -216,7 +223,7 @@ GIT_EXTERN(int) git_tag_create_frombuffer( * * @param force Overwrite existing references * - * @return 0 or an error code + * @return 0 on success, GIT_EINVALIDSPEC or an error code * A proper reference is written in the /refs/tags folder, * pointing to the provided target object */ @@ -230,12 +237,15 @@ GIT_EXTERN(int) git_tag_create_lightweight( /** * Delete an existing tag reference. * + * The tag name will be checked for validity. + * See `git_tag_create()` for rules about valid names. + * * @param repo Repository where lives the tag * * @param tag_name Name of the tag to be deleted; * this name is validated for consistency. * - * @return 0 or an error code + * @return 0 on success, GIT_EINVALIDSPEC or an error code */ GIT_EXTERN(int) git_tag_delete( git_repository *repo, diff --git a/src/common.h b/src/common.h index a35239e3d..b3b551508 100644 --- a/src/common.h +++ b/src/common.h @@ -61,9 +61,9 @@ void giterr_set(int error_class, const char *string, ...); /** * Set the error message for a regex failure, using the internal regex - * error code lookup. + * error code lookup and return a libgit error code. */ -void giterr_set_regex(const regex_t *regex, int error_code); +int giterr_set_regex(const regex_t *regex, int error_code); /* NOTE: other giterr functions are in the public errors.h header file */ diff --git a/src/errors.c b/src/errors.c index ac7fa934d..e62507216 100644 --- a/src/errors.c +++ b/src/errors.c @@ -93,11 +93,18 @@ void giterr_set_str(int error_class, const char *string) set_error(error_class, message); } -void giterr_set_regex(const regex_t *regex, int error_code) +int giterr_set_regex(const regex_t *regex, int error_code) { char error_buf[1024]; regerror(error_code, regex, error_buf, sizeof(error_buf)); giterr_set_str(GITERR_REGEX, error_buf); + + if (error_code == REG_NOMATCH) + return GIT_ENOTFOUND; + else if (error_code > REG_BADPAT) + return GIT_EINVALIDSPEC; + else + return -1; } void giterr_clear(void) diff --git a/src/object.c b/src/object.c index f88c2ba50..d57b6468c 100644 --- a/src/object.c +++ b/src/object.c @@ -304,12 +304,6 @@ size_t git_object__size(git_otype type) return git_objects_table[type].size; } -static int peel_error(int error, const char* msg) -{ - giterr_set(GITERR_INVALID, "The given object cannot be peeled - %s", msg); - return error; -} - static int dereference_object(git_object **dereferenced, git_object *obj) { git_otype type = git_object_type(obj); @@ -322,22 +316,46 @@ static int dereference_object(git_object **dereferenced, git_object *obj) return git_tag_target(dereferenced, (git_tag*)obj); case GIT_OBJ_BLOB: - return peel_error(GIT_ERROR, "cannot dereference blob"); + return GIT_ENOTFOUND; case GIT_OBJ_TREE: - return peel_error(GIT_ERROR, "cannot dereference tree"); + return GIT_EAMBIGUOUS; default: - return peel_error(GIT_ENOTFOUND, "unexpected object type encountered"); + return GIT_EINVALIDSPEC; } } +static int peel_error(int error, const git_oid *oid, git_otype type) +{ + const char *type_name; + char hex_oid[GIT_OID_HEXSZ + 1]; + + type_name = git_object_type2string(type); + + git_oid_fmt(hex_oid, oid); + hex_oid[GIT_OID_HEXSZ] = '\0'; + + giterr_set(GITERR_OBJECT, "The git_object of id '%s' can not be " + "successfully peeled into a %s (git_otype=%i).", hex_oid, type_name, type); + + return error; +} + int git_object_peel( git_object **peeled, const git_object *object, git_otype target_type) { git_object *source, *deref = NULL; + int error; + + if (target_type != GIT_OBJ_TAG && + target_type != GIT_OBJ_COMMIT && + target_type != GIT_OBJ_TREE && + target_type != GIT_OBJ_BLOB && + target_type != GIT_OBJ_ANY) + return GIT_EINVALIDSPEC; assert(object && peeled); @@ -346,7 +364,7 @@ int git_object_peel( source = (git_object *)object; - while (!dereference_object(&deref, source)) { + while (!(error = dereference_object(&deref, source))) { if (source != object) git_object_free(source); @@ -371,6 +389,10 @@ int git_object_peel( git_object_free(source); git_object_free(deref); - return -1; + + if (error) + error = peel_error(error, git_object_id(object), target_type); + + return error; } diff --git a/src/reflog.c b/src/reflog.c index ac481fb81..df799b113 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -340,21 +340,29 @@ cleanup: int git_reflog_rename(git_reference *ref, const char *new_name) { - int error = -1, fd; + int error, fd; git_buf old_path = GIT_BUF_INIT; git_buf new_path = GIT_BUF_INIT; git_buf temp_path = GIT_BUF_INIT; + git_buf normalized = GIT_BUF_INIT; assert(ref && new_name); + if ((error = git_reference__normalize_name( + &normalized, new_name, GIT_REF_FORMAT_ALLOW_ONELEVEL)) < 0) + goto cleanup; + + error = -1; + if (git_buf_joinpath(&temp_path, git_reference_owner(ref)->path_repository, GIT_REFLOG_DIR) < 0) return -1; if (git_buf_joinpath(&old_path, git_buf_cstr(&temp_path), ref->name) < 0) goto cleanup; - if (git_buf_joinpath(&new_path, git_buf_cstr(&temp_path), new_name) < 0) - goto cleanup; + if (git_buf_joinpath(&new_path, + git_buf_cstr(&temp_path), git_buf_cstr(&normalized)) < 0) + goto cleanup; /* * Move the reflog to a temporary place. This two-phase renaming is required @@ -386,6 +394,7 @@ cleanup: git_buf_free(&temp_path); git_buf_free(&old_path); git_buf_free(&new_path); + git_buf_free(&normalized); return error; } diff --git a/src/refs.c b/src/refs.c index 76c9f42ba..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); @@ -1962,8 +1965,12 @@ int git_reference__is_valid_name( const char *refname, unsigned int flags) { + int error; + + error = git_reference__normalize_name(NULL, refname, flags) == 0; giterr_clear(); - return git_reference__normalize_name(NULL, refname, flags) == 0; + + return error; } int git_reference_is_valid_name( 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/src/revparse.c b/src/revparse.c index 308b92923..ade03d0e4 100644 --- a/src/revparse.c +++ b/src/revparse.c @@ -13,12 +13,6 @@ #include "git2.h" -static int revspec_error(const char *revspec) -{ - giterr_set(GITERR_INVALID, "Failed to parse revision specifier - Invalid pattern '%s'", revspec); - return -1; -} - static int disambiguate_refname(git_reference **out, git_repository *repo, const char *refname) { int error, i; @@ -51,7 +45,7 @@ static int disambiguate_refname(git_reference **out, git_repository *repo, const goto cleanup; if (!git_reference_is_valid_name(git_buf_cstr(&refnamebuf))) { - error = GIT_ENOTFOUND; + error = GIT_EINVALIDSPEC; continue; } @@ -90,17 +84,18 @@ static int build_regex(regex_t *regex, const char *pattern) if (*pattern == '\0') { giterr_set(GITERR_REGEX, "Empty pattern"); - return -1; + return GIT_EINVALIDSPEC; } error = regcomp(regex, pattern, REG_EXTENDED); if (!error) return 0; - giterr_set_regex(regex, error); + error = giterr_set_regex(regex, error); + regfree(regex); - return -1; + return error; } static int maybe_describe(git_object**out, git_repository *repo, const char *spec) @@ -174,7 +169,7 @@ static int try_parse_numeric(int *n, const char *curly_braces_content) return 0; } -static int retrieve_previously_checked_out_branch_or_revision(git_object **out, git_reference **base_ref, git_repository *repo, const char *spec, const char *identifier, size_t position) +static int retrieve_previously_checked_out_branch_or_revision(git_object **out, git_reference **base_ref, git_repository *repo, const char *identifier, size_t position) { git_reference *ref = NULL; git_reflog *reflog = NULL; @@ -189,7 +184,7 @@ static int retrieve_previously_checked_out_branch_or_revision(git_object **out, cur = position; if (*identifier != '\0' || *base_ref != NULL) - return revspec_error(spec); + return GIT_EINVALIDSPEC; if (build_regex(&preg, "checkout: moving from (.*) to .*") < 0) return -1; @@ -332,6 +327,11 @@ static int retrieve_remote_tracking_reference(git_reference **base_ref, const ch *base_ref = NULL; } + if (!git_reference_is_branch(ref)) { + error = GIT_EINVALIDSPEC; + goto cleanup; + } + if ((error = git_branch_tracking(&tracking, ref)) < 0) goto cleanup; @@ -357,13 +357,13 @@ static int handle_at_syntax(git_object **out, git_reference **ref, const char *s is_numeric = !try_parse_numeric(&parsed, curly_braces_content); if (*curly_braces_content == '-' && (!is_numeric || parsed == 0)) { - error = revspec_error(spec); + error = GIT_EINVALIDSPEC; goto cleanup; } if (is_numeric) { if (parsed < 0) - error = retrieve_previously_checked_out_branch_or_revision(out, ref, repo, spec, git_buf_cstr(&identifier), -parsed); + error = retrieve_previously_checked_out_branch_or_revision(out, ref, repo, git_buf_cstr(&identifier), -parsed); else error = retrieve_revobject_from_reflog(out, ref, repo, git_buf_cstr(&identifier), parsed); @@ -416,8 +416,9 @@ static int handle_caret_parent_syntax(git_object **out, git_object *obj, int n) git_object *temp_commit = NULL; int error; - if (git_object_peel(&temp_commit, obj, GIT_OBJ_COMMIT) < 0) - return -1; + if ((error = git_object_peel(&temp_commit, obj, GIT_OBJ_COMMIT)) < 0) + return (error == GIT_EAMBIGUOUS || error == GIT_ENOTFOUND) ? + GIT_EINVALIDSPEC : error; if (n == 0) { *out = temp_commit; @@ -435,8 +436,9 @@ static int handle_linear_syntax(git_object **out, git_object *obj, int n) git_object *temp_commit = NULL; int error; - if (git_object_peel(&temp_commit, obj, GIT_OBJ_COMMIT) < 0) - return -1; + if ((error = git_object_peel(&temp_commit, obj, GIT_OBJ_COMMIT)) < 0) + return (error == GIT_EAMBIGUOUS || error == GIT_ENOTFOUND) ? + GIT_EINVALIDSPEC : error; error = git_commit_nth_gen_ancestor((git_commit **)out, (git_commit*)temp_commit, n); @@ -453,8 +455,8 @@ static int handle_colon_syntax( int error = -1; git_tree_entry *entry = NULL; - if (git_object_peel(&tree, obj, GIT_OBJ_TREE) < 0) - return -1; + if ((error = git_object_peel(&tree, obj, GIT_OBJ_TREE)) < 0) + return error == GIT_ENOTFOUND ? GIT_EINVALIDSPEC : error; if (*path == '\0') { *out = tree; @@ -507,21 +509,21 @@ static int handle_grep_syntax(git_object **out, git_repository *repo, const git_ { regex_t preg; git_revwalk *walk = NULL; - int error = -1; + int error; - if (build_regex(&preg, pattern) < 0) - return -1; + if ((error = build_regex(&preg, pattern)) < 0) + return error; - if (git_revwalk_new(&walk, repo) < 0) + if ((error = git_revwalk_new(&walk, repo)) < 0) goto cleanup; git_revwalk_sorting(walk, GIT_SORT_TIME); if (spec_oid == NULL) { // TODO: @carlosmn: The glob should be refs/* but this makes git_revwalk_next() fails - if (git_revwalk_push_glob(walk, GIT_REFS_HEADS_DIR "*") < 0) + if ((error = git_revwalk_push_glob(walk, GIT_REFS_HEADS_DIR "*")) < 0) goto cleanup; - } else if (git_revwalk_push(walk, spec_oid) < 0) + } else if ((error = git_revwalk_push(walk, spec_oid)) < 0) goto cleanup; error = walk_and_search(out, walk, &preg); @@ -546,7 +548,7 @@ static int handle_caret_curly_syntax(git_object **out, git_object *obj, const ch expected_type = parse_obj_type(curly_braces_content); if (expected_type == GIT_OBJ_BAD) - return -1; + return GIT_EINVALIDSPEC; return git_object_peel(out, obj, expected_type); } @@ -560,13 +562,13 @@ static int extract_curly_braces_content(git_buf *buf, const char *spec, size_t * (*pos)++; if (spec[*pos] == '\0' || spec[*pos] != '{') - return revspec_error(spec); + return GIT_EINVALIDSPEC; (*pos)++; while (spec[*pos] != '}') { if (spec[*pos] == '\0') - return revspec_error(spec); + return GIT_EINVALIDSPEC; git_buf_putc(buf, spec[(*pos)++]); } @@ -610,7 +612,7 @@ static int extract_how_many(int *n, const char *spec, size_t *pos) if (git__isdigit(spec[*pos])) { if ((git__strtol32(&parsed, spec + *pos, &end_ptr, 10) < 0) < 0) - return revspec_error(spec); + return GIT_EINVALIDSPEC; accumulated += (parsed - 1); *pos = end_ptr - spec; @@ -655,7 +657,7 @@ static int ensure_base_rev_loaded(git_object **object, git_reference **reference } if (!allow_empty_identifier && identifier_len == 0) - return revspec_error(spec); + return GIT_EINVALIDSPEC; if (git_buf_put(&identifier, spec, identifier_len) < 0) return -1; @@ -666,12 +668,12 @@ static int ensure_base_rev_loaded(git_object **object, git_reference **reference return error; } -static int ensure_base_rev_is_not_known_yet(git_object *object, const char *spec) +static int ensure_base_rev_is_not_known_yet(git_object *object) { if (object == NULL) return 0; - return revspec_error(spec); + return GIT_EINVALIDSPEC; } static bool any_left_hand_identifier(git_object *object, git_reference *reference, size_t identifier_len) @@ -688,12 +690,12 @@ static bool any_left_hand_identifier(git_object *object, git_reference *referenc return false; } -static int ensure_left_hand_identifier_is_not_known_yet(git_object *object, git_reference *reference, const char *spec) +static int ensure_left_hand_identifier_is_not_known_yet(git_object *object, git_reference *reference) { - if (!ensure_base_rev_is_not_known_yet(object, spec) && reference == NULL) + if (!ensure_base_rev_is_not_known_yet(object) && reference == NULL) return 0; - return revspec_error(spec); + return GIT_EINVALIDSPEC; } int git_revparse_single(git_object **out, git_repository *repo, const char *spec) @@ -800,7 +802,7 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec if ((error = extract_curly_braces_content(&buf, spec, &pos)) < 0) goto cleanup; - if ((error = ensure_base_rev_is_not_known_yet(base_rev, spec)) < 0) + if ((error = ensure_base_rev_is_not_known_yet(base_rev)) < 0) goto cleanup; if ((error = handle_at_syntax(&temp_object, &reference, spec, identifier_len, repo, git_buf_cstr(&buf))) < 0) @@ -815,7 +817,7 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec } default: - if ((error = ensure_left_hand_identifier_is_not_known_yet(base_rev, reference, spec)) < 0) + if ((error = ensure_left_hand_identifier_is_not_known_yet(base_rev, reference)) < 0) goto cleanup; pos++; @@ -830,8 +832,13 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec error = 0; cleanup: - if (error) + if (error) { + if (error == GIT_EINVALIDSPEC) + giterr_set(GITERR_INVALID, + "Failed to parse revision specifier - Invalid pattern '%s'", spec); + git_object_free(base_rev); + } git_reference_free(reference); git_buf_free(&buf); return error; @@ -251,7 +251,7 @@ static int git_tag_create__internal( error = retrieve_tag_reference_oid(oid, &ref_name, repo, tag_name); if (error < 0 && error != GIT_ENOTFOUND) - return -1; + goto cleanup; /** Ensure the tag name doesn't conflict with an already existing * reference unless overwriting has explictly been requested **/ @@ -269,6 +269,7 @@ static int git_tag_create__internal( error = git_reference_create(&new_ref, repo, ref_name.ptr, oid, allow_ref_overwrite); +cleanup: git_reference_free(new_ref); git_buf_free(&ref_name); return error; @@ -384,7 +385,7 @@ int git_tag_delete(git_repository *repo, const char *tag_name) git_buf_free(&ref_name); if (error < 0) - return -1; + return error; return git_reference_delete(tag_ref); } diff --git a/tests-clar/network/fetch.c b/tests-clar/network/fetch.c index 84c947291..623352fa5 100644 --- a/tests-clar/network/fetch.c +++ b/tests-clar/network/fetch.c @@ -79,6 +79,9 @@ void test_network_fetch__no_tags_http(void) static void transferProgressCallback(const git_transfer_progress *stats, void *payload) { bool *invoked = (bool *)payload; + + GIT_UNUSED(stats); + *invoked = true; } 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) diff --git a/tests-clar/object/peel.c b/tests-clar/object/peel.c index a19772858..bb0bbd096 100644 --- a/tests-clar/object/peel.c +++ b/tests-clar/object/peel.c @@ -79,12 +79,12 @@ void test_object_peel__can_peel_a_commit(void) void test_object_peel__cannot_peel_a_tree(void) { - assert_peel_error(GIT_ERROR, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_BLOB); + assert_peel_error(GIT_EAMBIGUOUS, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_BLOB); } void test_object_peel__cannot_peel_a_blob(void) { - assert_peel_error(GIT_ERROR, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_COMMIT); + assert_peel_error(GIT_ENOTFOUND, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_COMMIT); } void test_object_peel__target_any_object_for_type_change(void) @@ -98,8 +98,13 @@ void test_object_peel__target_any_object_for_type_change(void) "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_TREE); /* fail to peel tree */ - assert_peel_error(GIT_ERROR, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_ANY); + assert_peel_error(GIT_EAMBIGUOUS, "53fc32d17276939fc79ed05badaef2db09990016", GIT_OBJ_ANY); /* fail to peel blob */ - assert_peel_error(GIT_ERROR, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_ANY); + assert_peel_error(GIT_ENOTFOUND, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_ANY); +} + +void test_object_peel__should_use_a_well_known_type(void) +{ + assert_peel_error(GIT_EINVALIDSPEC, "7b4384978d2493e851f9cca7858815fac9b10980", GIT_OBJ__EXT2); } diff --git a/tests-clar/object/tag/write.c b/tests-clar/object/tag/write.c index ad6ca76b2..eb0ac2897 100644 --- a/tests-clar/object/tag/write.c +++ b/tests-clar/object/tag/write.c @@ -88,7 +88,6 @@ void test_object_tag_write__overwrite(void) git_object_free(target); git_signature_free(tagger); - } void test_object_tag_write__replace(void) @@ -190,3 +189,33 @@ void test_object_tag_write__delete(void) git_reference_free(ref_tag); } + +void test_object_tag_write__creating_with_an_invalid_name_returns_EINVALIDSPEC(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_OBJ_COMMIT)); + + cl_git_pass(git_signature_new(&tagger, tagger_name, tagger_email, 123456789, 60)); + + cl_assert_equal_i(GIT_EINVALIDSPEC, + git_tag_create(&tag_id, g_repo, + "Inv@{id", target, tagger, tagger_message, 0) + ); + + cl_assert_equal_i(GIT_EINVALIDSPEC, + git_tag_create_lightweight(&tag_id, g_repo, + "Inv@{id", target, 0) + ); + + git_object_free(target); + git_signature_free(tagger); +} + +void test_object_tag_write__deleting_with_an_invalid_name_returns_EINVALIDSPEC(void) +{ + cl_assert_equal_i(GIT_EINVALIDSPEC, git_tag_delete(g_repo, "Inv@{id")); +} diff --git a/tests-clar/refs/branches/create.c b/tests-clar/refs/branches/create.c index a8c4d4f51..693a592a3 100644 --- a/tests-clar/refs/branches/create.c +++ b/tests-clar/refs/branches/create.c @@ -65,3 +65,12 @@ void test_refs_branches_create__can_force_create_over_an_existing_branch(void) cl_git_pass(git_oid_cmp(git_reference_target(branch), git_commit_id(target))); cl_assert_equal_s("refs/heads/br2", git_reference_name(branch)); } + + +void test_refs_branches_create__creating_a_branch_with_an_invalid_name_returns_EINVALIDSPEC(void) +{ + retrieve_known_commit(&target, repo); + + cl_assert_equal_i(GIT_EINVALIDSPEC, + git_branch_create(&branch, repo, "inv@{id", target, 0)); +}
\ No newline at end of file diff --git a/tests-clar/refs/branches/lookup.c b/tests-clar/refs/branches/lookup.c index d07ed0ed8..95d49a4b3 100644 --- a/tests-clar/refs/branches/lookup.c +++ b/tests-clar/refs/branches/lookup.c @@ -35,3 +35,11 @@ void test_refs_branches_lookup__trying_to_retrieve_an_unknown_branch_returns_ENO cl_assert_equal_i(GIT_ENOTFOUND, git_branch_lookup(&branch, repo, "where/are/you", GIT_BRANCH_LOCAL)); cl_assert_equal_i(GIT_ENOTFOUND, git_branch_lookup(&branch, repo, "over/here", GIT_BRANCH_REMOTE)); } + +void test_refs_branches_lookup__trying_to_retrieve_a_branch_with_an_invalid_name_returns_EINVALIDSPEC(void) +{ + cl_assert_equal_i(GIT_EINVALIDSPEC, + git_branch_lookup(&branch, repo, "are/you/inv@{id", GIT_BRANCH_LOCAL)); + cl_assert_equal_i(GIT_EINVALIDSPEC, + git_branch_lookup(&branch, repo, "yes/i am", GIT_BRANCH_REMOTE)); +} diff --git a/tests-clar/refs/branches/move.c b/tests-clar/refs/branches/move.c index 4bf1d69d0..17fb6dfe6 100644 --- a/tests-clar/refs/branches/move.c +++ b/tests-clar/refs/branches/move.c @@ -51,6 +51,11 @@ void test_refs_branches_move__can_not_move_a_branch_if_its_destination_name_coll cl_assert_equal_i(GIT_EEXISTS, git_branch_move(ref, "master", 0)); } +void test_refs_branches_move__moving_a_branch_with_an_invalid_name_returns_EINVALIDSPEC(void) +{ + cl_assert_equal_i(GIT_EINVALIDSPEC, git_branch_move(ref, "Inv@{id", 0)); +} + void test_refs_branches_move__can_not_move_a_non_branch(void) { git_reference *tag; diff --git a/tests-clar/refs/branches/tracking.c b/tests-clar/refs/branches/tracking.c index e8b2f24d7..7af5f974f 100644 --- a/tests-clar/refs/branches/tracking.c +++ b/tests-clar/refs/branches/tracking.c @@ -61,7 +61,7 @@ void test_refs_branches_tracking__trying_to_retrieve_a_remote_tracking_reference cl_assert_equal_i(GIT_ENOTFOUND, git_branch_tracking(&tracking, branch)); } -static void assert_merge_and_or_remote_key_missing(git_repository *repository, git_object *target, const char *entry_name) +static void assert_merge_and_or_remote_key_missing(git_repository *repository, const git_commit *target, const char *entry_name) { git_reference *branch; @@ -76,19 +76,19 @@ void test_refs_branches_tracking__retrieve_a_remote_tracking_reference_from_a_br { git_reference *head; git_repository *repository; - git_object *target; + git_commit *target; repository = cl_git_sandbox_init("testrepo.git"); cl_git_pass(git_repository_head(&head, repository)); - cl_git_pass(git_reference_peel(&target, head, GIT_OBJ_COMMIT)); + cl_git_pass(git_reference_peel(((git_object **)&target), head, GIT_OBJ_COMMIT)); git_reference_free(head); assert_merge_and_or_remote_key_missing(repository, target, "remoteless"); assert_merge_and_or_remote_key_missing(repository, target, "mergeless"); assert_merge_and_or_remote_key_missing(repository, target, "mergeandremoteless"); - git_object_free(target); + git_commit_free(target); cl_git_sandbox_cleanup(); } diff --git a/tests-clar/refs/create.c b/tests-clar/refs/create.c index bef9bfd24..56c323d8a 100644 --- a/tests-clar/refs/create.c +++ b/tests-clar/refs/create.c @@ -25,16 +25,11 @@ void test_refs_create__symbolic(void) git_reference *new_reference, *looked_up_ref, *resolved_ref; git_repository *repo2; git_oid id; - git_buf ref_path = GIT_BUF_INIT; const char *new_head_tracker = "ANOTHER_HEAD_TRACKER"; git_oid_fromstr(&id, current_master_tip); - /* Retrieve the physical path to the symbolic ref for further cleaning */ - cl_git_pass(git_buf_joinpath(&ref_path, g_repo->path_repository, new_head_tracker)); - git_buf_free(&ref_path); - /* Create and write the new symbolic reference */ cl_git_pass(git_reference_symbolic_create(&new_reference, g_repo, new_head_tracker, current_head_target, 0)); @@ -72,13 +67,11 @@ void test_refs_create__deep_symbolic(void) // create a deep symbolic reference git_reference *new_reference, *looked_up_ref, *resolved_ref; git_oid id; - git_buf ref_path = GIT_BUF_INIT; const char *new_head_tracker = "deep/rooted/tracker"; git_oid_fromstr(&id, current_master_tip); - cl_git_pass(git_buf_joinpath(&ref_path, g_repo->path_repository, new_head_tracker)); cl_git_pass(git_reference_symbolic_create(&new_reference, g_repo, new_head_tracker, current_head_target, 0)); cl_git_pass(git_reference_lookup(&looked_up_ref, g_repo, new_head_tracker)); cl_git_pass(git_reference_resolve(&resolved_ref, looked_up_ref)); @@ -87,7 +80,6 @@ void test_refs_create__deep_symbolic(void) git_reference_free(new_reference); git_reference_free(looked_up_ref); git_reference_free(resolved_ref); - git_buf_free(&ref_path); } void test_refs_create__oid(void) @@ -96,15 +88,11 @@ void test_refs_create__oid(void) git_reference *new_reference, *looked_up_ref; git_repository *repo2; git_oid id; - git_buf ref_path = GIT_BUF_INIT; const char *new_head = "refs/heads/new-head"; git_oid_fromstr(&id, current_master_tip); - /* Retrieve the physical path to the symbolic ref for further cleaning */ - cl_git_pass(git_buf_joinpath(&ref_path, g_repo->path_repository, new_head)); - /* Create and write the new object id reference */ cl_git_pass(git_reference_create(&new_reference, g_repo, new_head, &id, 0)); @@ -128,7 +116,6 @@ void test_refs_create__oid(void) git_reference_free(new_reference); git_reference_free(looked_up_ref); - git_buf_free(&ref_path); } void test_refs_create__oid_unknown(void) @@ -162,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/peel.c b/tests-clar/refs/peel.c index 6fa6009d5..34bd02ce0 100644 --- a/tests-clar/refs/peel.c +++ b/tests-clar/refs/peel.c @@ -78,7 +78,7 @@ void test_refs_peel__can_peel_a_symbolic_reference(void) void test_refs_peel__cannot_peel_into_a_non_existing_target(void) { - assert_peel_error(GIT_ERROR, "refs/tags/point_to_blob", GIT_OBJ_TAG); + assert_peel_error(GIT_ENOTFOUND, "refs/tags/point_to_blob", GIT_OBJ_TAG); } void test_refs_peel__can_peel_into_any_non_tag_object(void) diff --git a/tests-clar/refs/read.c b/tests-clar/refs/read.c index c10a540c0..3e2a59afd 100644 --- a/tests-clar/refs/read.c +++ b/tests-clar/refs/read.c @@ -223,11 +223,19 @@ void test_refs_read__trailing(void) void test_refs_read__unfound_return_ENOTFOUND(void) { git_reference *reference; + git_oid id; - 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")); + + cl_assert_equal_i(GIT_ENOTFOUND, + git_reference_name_to_id(&id, g_repo, "refs/tags/test/farther/master")); } static void assert_is_branch(const char *name, bool expected_branchness) @@ -245,3 +253,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/reflog/reflog.c b/tests-clar/refs/reflog/reflog.c index 8743c8a76..19ee53567 100644 --- a/tests-clar/refs/reflog/reflog.c +++ b/tests-clar/refs/reflog/reflog.c @@ -170,3 +170,15 @@ void test_refs_reflog_reflog__cannot_write_a_moved_reflog(void) git_buf_free(&moved_log_path); git_buf_free(&master_log_path); } + +void test_refs_reflog_reflog__renaming_with_an_invalid_name_returns_EINVALIDSPEC(void) +{ + git_reference *master; + + cl_git_pass(git_reference_lookup(&master, g_repo, "refs/heads/master")); + + cl_assert_equal_i(GIT_EINVALIDSPEC, + git_reflog_rename(master, "refs/heads/Inv@{id")); + + git_reference_free(master); +} 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)); - - /* Can not be renamed outside of the refs hierarchy. */ - cl_git_fail(git_reference_rename(looked_up_ref, "i-will-sudo-you", 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 + * 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/revparse.c b/tests-clar/refs/revparse.c index 3698b5197..81a6bc469 100644 --- a/tests-clar/refs/revparse.c +++ b/tests-clar/refs/revparse.c @@ -7,9 +7,6 @@ static git_repository *g_repo; static git_object *g_obj; -static char g_orig_tz[16] = {0}; - - /* Helpers */ static void test_object_inrepo(const char *spec, const char *expected_oid, git_repository *repo) @@ -37,19 +34,12 @@ static void test_object(const char *spec, const char *expected_oid) void test_refs_revparse__initialize(void) { - char *tz = cl_getenv("TZ"); - if (tz) - strcpy(g_orig_tz, tz); - cl_setenv("TZ", "UTC"); - cl_git_pass(git_repository_open(&g_repo, cl_fixture("testrepo.git"))); } void test_refs_revparse__cleanup(void) { git_repository_free(g_repo); - g_obj = NULL; - cl_setenv("TZ", g_orig_tz); } void test_refs_revparse__nonexistant_object(void) @@ -59,13 +49,17 @@ void test_refs_revparse__nonexistant_object(void) test_object("this-does-not-exist~2", NULL); } -void test_refs_revparse__invalid_reference_name(void) +static void assert_invalid_spec(const char *invalid_spec) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense^1")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "this doesn't make sense~2")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "")); + cl_assert_equal_i( + GIT_EINVALIDSPEC, git_revparse_single(&g_obj, g_repo, invalid_spec)); +} +void test_refs_revparse__invalid_reference_name(void) +{ + assert_invalid_spec("this doesn't make sense"); + assert_invalid_spec("Inv@{id"); + assert_invalid_spec(""); } void test_refs_revparse__shas(void) @@ -104,9 +98,11 @@ void test_refs_revparse__describe_output(void) void test_refs_revparse__nth_parent(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "be3563a^-1")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "^")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "be3563a^{tree}^")); + assert_invalid_spec("be3563a^-1"); + assert_invalid_spec("^"); + assert_invalid_spec("be3563a^{tree}^"); + assert_invalid_spec("point_to_blob^{blob}^"); + assert_invalid_spec("this doesn't make sense^1"); test_object("be3563a^1", "9fd738e8f7967c078dceed8190330fc8648ee56a"); test_object("be3563a^", "9fd738e8f7967c078dceed8190330fc8648ee56a"); @@ -133,8 +129,10 @@ void test_refs_revparse__not_tag(void) void test_refs_revparse__to_type(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "wrapped_tag^{blob}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "wrapped_tag^{trip}")); + assert_invalid_spec("wrapped_tag^{trip}"); + test_object("point_to_blob^{commit}", NULL); + cl_assert_equal_i( + GIT_EAMBIGUOUS, git_revparse_single(&g_obj, g_repo, "wrapped_tag^{blob}")); test_object("wrapped_tag^{commit}", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("wrapped_tag^{tree}", "944c0f6e4dfa41595e6eb3ceecdb14f50fe18162"); @@ -144,11 +142,15 @@ void test_refs_revparse__to_type(void) void test_refs_revparse__linear_history(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "~")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "foo~bar")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master~bar")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master~-1")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master~0bar")); + assert_invalid_spec("~"); + test_object("foo~bar", NULL); + + assert_invalid_spec("master~bar"); + assert_invalid_spec("master~-1"); + assert_invalid_spec("master~0bar"); + assert_invalid_spec("this doesn't make sense~2"); + assert_invalid_spec("be3563a^{tree}~"); + assert_invalid_spec("point_to_blob^{blob}~"); test_object("master~0", "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"); test_object("master~1", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); @@ -159,10 +161,10 @@ void test_refs_revparse__linear_history(void) void test_refs_revparse__chaining(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master@{0}@{0}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{u}@{-1}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-1}@{-1}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-3}@{0}")); + assert_invalid_spec("master@{0}@{0}"); + assert_invalid_spec("@{u}@{-1}"); + assert_invalid_spec("@{-1}@{-1}"); + assert_invalid_spec("@{-3}@{0}"); test_object("master@{0}~1^1", "9fd738e8f7967c078dceed8190330fc8648ee56a"); test_object("@{u}@{0}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); @@ -178,8 +180,9 @@ void test_refs_revparse__chaining(void) void test_refs_revparse__upstream(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "e90810b@{u}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "refs/tags/e90810b@{u}")); + assert_invalid_spec("e90810b@{u}"); + assert_invalid_spec("refs/tags/e90810b@{u}"); + test_object("refs/heads/e90810b@{u}", NULL); test_object("master@{upstream}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); test_object("@{u}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); @@ -190,7 +193,7 @@ void test_refs_revparse__upstream(void) void test_refs_revparse__ordinal(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master@{-2}")); + assert_invalid_spec("master@{-2}"); /* TODO: make the test below actually fail * cl_git_fail(git_revparse_single(&g_obj, g_repo, "master@{1a}")); @@ -212,9 +215,9 @@ void test_refs_revparse__ordinal(void) void test_refs_revparse__previous_head(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-xyz}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-0}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "@{-1b}")); + assert_invalid_spec("@{-xyz}"); + assert_invalid_spec("@{-0}"); + assert_invalid_spec("@{-1b}"); test_object("@{-42}", NULL); @@ -271,9 +274,9 @@ void test_refs_revparse__reflog_of_a_ref_under_refs(void) void test_refs_revparse__revwalk(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master^{/not found in any commit}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master^{/merge}")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, "master^{/((}")); + test_object("master^{/not found in any commit}", NULL); + test_object("master^{/merge}", NULL); + assert_invalid_spec("master^{/((}"); test_object("master^{/anoth}", "5b5b025afb0b4c913b4c338a42934a3863bf3644"); test_object("master^{/Merge}", "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); @@ -354,8 +357,9 @@ void test_refs_revparse__date(void) void test_refs_revparse__colon(void) { - cl_git_fail(git_revparse_single(&g_obj, g_repo, ":/")); - cl_git_fail(git_revparse_single(&g_obj, g_repo, ":2:README")); + assert_invalid_spec(":/"); + assert_invalid_spec("point_to_blob:readme.txt"); + cl_git_fail(git_revparse_single(&g_obj, g_repo, ":2:README")); /* Not implemented */ test_object(":/not found in any commit", NULL); test_object("subtrees:ab/42.txt", NULL); @@ -445,11 +449,8 @@ void test_refs_revparse__disambiguation(void) void test_refs_revparse__a_too_short_objectid_returns_EAMBIGUOUS(void) { - int result; - - result = git_revparse_single(&g_obj, g_repo, "e90"); - - cl_assert_equal_i(GIT_EAMBIGUOUS, result); + cl_assert_equal_i( + GIT_EAMBIGUOUS, git_revparse_single(&g_obj, g_repo, "e90")); } void test_refs_revparse__issue_994(void) 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); +} |