diff options
author | Edward Thomson <ethomson@edwardthomson.com> | 2019-11-02 07:30:32 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-11-02 07:30:32 -0500 |
commit | bf2911d7d0e4fd0f8a76d5de658af0929fa32ca8 (patch) | |
tree | 6ad4b08f0f5b62281cdcdd5a1b9921c1e62f6b72 | |
parent | d5017a14284447c289dd04cae1a310dc34aa562c (diff) | |
parent | 7968e90f79d4c209f26d6a6f48db2137b168906e (diff) | |
download | libgit2-bf2911d7d0e4fd0f8a76d5de658af0929fa32ca8.tar.gz |
Merge pull request #5275 from pks-t/pks/reflogs-with-newlines
reflogs: fix behaviour around reflogs with newlines
-rw-r--r-- | src/parse.c | 10 | ||||
-rw-r--r-- | src/parse.h | 1 | ||||
-rw-r--r-- | src/refdb_fs.c | 93 | ||||
-rw-r--r-- | src/reflog.c | 24 | ||||
-rw-r--r-- | src/stash.c | 80 | ||||
-rw-r--r-- | tests/refs/reflog/messages.c | 21 | ||||
-rw-r--r-- | tests/refs/reflog/reflog.c | 38 | ||||
-rw-r--r-- | tests/stash/save.c | 20 |
8 files changed, 155 insertions, 132 deletions
diff --git a/src/parse.c b/src/parse.c index b04fda36b..0a10758bf 100644 --- a/src/parse.c +++ b/src/parse.c @@ -101,6 +101,16 @@ int git_parse_advance_digit(int64_t *out, git_parse_ctx *ctx, int base) return 0; } +int git_parse_advance_oid(git_oid *out, git_parse_ctx *ctx) +{ + if (ctx->line_len < GIT_OID_HEXSZ) + return -1; + if ((git_oid_fromstrn(out, ctx->line, GIT_OID_HEXSZ)) < 0) + return -1; + git_parse_advance_chars(ctx, GIT_OID_HEXSZ); + return 0; +} + int git_parse_peek(char *out, git_parse_ctx *ctx, int flags) { size_t remain = ctx->line_len; diff --git a/src/parse.h b/src/parse.h index 188ac281c..0ecb7c103 100644 --- a/src/parse.h +++ b/src/parse.h @@ -50,6 +50,7 @@ int git_parse_advance_expected( int git_parse_advance_ws(git_parse_ctx *ctx); int git_parse_advance_nl(git_parse_ctx *ctx); int git_parse_advance_digit(int64_t *out, git_parse_ctx *ctx, int base); +int git_parse_advance_oid(git_oid *out, git_parse_ctx *ctx); enum GIT_PARSE_PEEK_FLAGS { GIT_PARSE_PEEK_SKIP_WHITESPACE = (1 << 0) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index c48afb94f..77b72dc2a 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -13,6 +13,7 @@ #include "futils.h" #include "filebuf.h" #include "pack.h" +#include "parse.h" #include "reflog.h" #include "refdb.h" #include "iterator.h" @@ -1651,70 +1652,57 @@ static int reflog_alloc(git_reflog **reflog, const char *name) static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size) { - const char *ptr; - git_reflog_entry *entry; - -#define seek_forward(_increase) do { \ - if (_increase >= buf_size) { \ - git_error_set(GIT_ERROR_INVALID, "ran out of data while parsing reflog"); \ - goto fail; \ - } \ - buf += _increase; \ - buf_size -= _increase; \ - } while (0) - - while (buf_size > GIT_REFLOG_SIZE_MIN) { - entry = git__calloc(1, sizeof(git_reflog_entry)); - GIT_ERROR_CHECK_ALLOC(entry); + git_parse_ctx parser = GIT_PARSE_CTX_INIT; - entry->committer = git__calloc(1, sizeof(git_signature)); - GIT_ERROR_CHECK_ALLOC(entry->committer); + if ((git_parse_ctx_init(&parser, buf, buf_size)) < 0) + return -1; - if (git_oid_fromstrn(&entry->oid_old, buf, GIT_OID_HEXSZ) < 0) - goto fail; - seek_forward(GIT_OID_HEXSZ + 1); + for (; parser.remain_len; git_parse_advance_line(&parser)) { + git_reflog_entry *entry; + const char *sig; + char c; - if (git_oid_fromstrn(&entry->oid_cur, buf, GIT_OID_HEXSZ) < 0) - goto fail; - seek_forward(GIT_OID_HEXSZ + 1); + entry = git__calloc(1, sizeof(*entry)); + GIT_ERROR_CHECK_ALLOC(entry); + entry->committer = git__calloc(1, sizeof(*entry->committer)); + GIT_ERROR_CHECK_ALLOC(entry->committer); - ptr = buf; + if (git_parse_advance_oid(&entry->oid_old, &parser) < 0 || + git_parse_advance_expected(&parser, " ", 1) < 0 || + git_parse_advance_oid(&entry->oid_cur, &parser) < 0) + goto next; - /* Seek forward to the end of the signature. */ - while (*buf && *buf != '\t' && *buf != '\n') - seek_forward(1); + sig = parser.line; + while (git_parse_peek(&c, &parser, 0) == 0 && c != '\t' && c != '\n') + git_parse_advance_chars(&parser, 1); - if (git_signature__parse(entry->committer, &ptr, buf + 1, NULL, *buf) < 0) - goto fail; + if (git_signature__parse(entry->committer, &sig, parser.line, NULL, 0) < 0) + goto next; - if (*buf == '\t') { - /* We got a message. Read everything till we reach LF. */ - seek_forward(1); - ptr = buf; + if (c == '\t') { + size_t len; + git_parse_advance_chars(&parser, 1); - while (*buf && *buf != '\n') - seek_forward(1); + len = parser.line_len; + if (parser.line[len - 1] == '\n') + len--; - entry->msg = git__strndup(ptr, buf - ptr); + entry->msg = git__strndup(parser.line, len); GIT_ERROR_CHECK_ALLOC(entry->msg); - } else - entry->msg = NULL; + } - while (*buf && *buf == '\n' && buf_size > 1) - seek_forward(1); + if ((git_vector_insert(&log->entries, entry)) < 0) { + git_reflog_entry__free(entry); + return -1; + } - if (git_vector_insert(&log->entries, entry) < 0) - goto fail; + continue; + +next: + git_reflog_entry__free(entry); } return 0; - -#undef seek_forward - -fail: - git_reflog_entry__free(entry); - - return -1; } static int create_new_reflog_file(const char *filepath) @@ -1856,8 +1844,15 @@ static int serialize_reflog_entry( git_buf_rtrim(buf); if (msg) { + size_t i; + git_buf_putc(buf, '\t'); git_buf_puts(buf, msg); + + for (i = 0; i < buf->size - 2; i++) + if (buf->ptr[i] == '\n') + buf->ptr[i] = ' '; + git_buf_rtrim(buf); } git_buf_putc(buf, '\n'); diff --git a/src/reflog.c b/src/reflog.c index 1834a2736..24dada047 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -74,9 +74,8 @@ int git_reflog_write(git_reflog *reflog) int git_reflog_append(git_reflog *reflog, const git_oid *new_oid, const git_signature *committer, const char *msg) { - git_reflog_entry *entry; const git_reflog_entry *previous; - const char *newline; + git_reflog_entry *entry; assert(reflog && new_oid && committer); @@ -87,19 +86,18 @@ int git_reflog_append(git_reflog *reflog, const git_oid *new_oid, const git_sign goto cleanup; if (msg != NULL) { - if ((entry->msg = git__strdup(msg)) == NULL) - goto cleanup; + size_t i, msglen = strlen(msg); - newline = strchr(msg, '\n'); - - if (newline) { - if (newline[1] != '\0') { - git_error_set(GIT_ERROR_INVALID, "reflog message cannot contain newline"); - goto cleanup; - } + if ((entry->msg = git__strndup(msg, msglen)) == NULL) + goto cleanup; - entry->msg[newline - msg] = '\0'; - } + /* + * Replace all newlines with spaces, except for + * the final trailing newline. + */ + for (i = 0; i < msglen; i++) + if (entry->msg[i] == '\n') + entry->msg[i] = ' '; } previous = git_reflog_entry_byindex(reflog, 0); diff --git a/src/stash.c b/src/stash.c index aa3cecf6e..4a13d0530 100644 --- a/src/stash.c +++ b/src/stash.c @@ -437,36 +437,33 @@ cleanup: return error; } -static int prepare_worktree_commit_message( - git_buf* msg, - const char *user_message) +static int prepare_worktree_commit_message(git_buf *out, const char *user_message) { git_buf buf = GIT_BUF_INIT; - int error; - - if ((error = git_buf_set(&buf, git_buf_cstr(msg), git_buf_len(msg))) < 0) - return error; - - git_buf_clear(msg); + int error = 0; - if (!user_message) - git_buf_printf(msg, "WIP on %s", git_buf_cstr(&buf)); - else { + if (!user_message) { + git_buf_printf(&buf, "WIP on %s", git_buf_cstr(out)); + } else { const char *colon; - if ((colon = strchr(git_buf_cstr(&buf), ':')) == NULL) + if ((colon = strchr(git_buf_cstr(out), ':')) == NULL) goto cleanup; - git_buf_puts(msg, "On "); - git_buf_put(msg, git_buf_cstr(&buf), colon - buf.ptr); - git_buf_printf(msg, ": %s\n", user_message); + git_buf_puts(&buf, "On "); + git_buf_put(&buf, git_buf_cstr(out), colon - out->ptr); + git_buf_printf(&buf, ": %s\n", user_message); + } + + if (git_buf_oom(&buf)) { + error = -1; + goto cleanup; } - error = (git_buf_oom(msg) || git_buf_oom(&buf)) ? -1 : 0; + git_buf_swap(out, &buf); cleanup: git_buf_dispose(&buf); - return error; } @@ -497,10 +494,7 @@ static int is_dirty_cb(const char *path, unsigned int status, void *payload) return GIT_PASSTHROUGH; } -static int ensure_there_are_changes_to_stash( - git_repository *repo, - bool include_untracked_files, - bool include_ignored_files) +static int ensure_there_are_changes_to_stash(git_repository *repo, uint32_t flags) { int error; git_status_options opts = GIT_STATUS_OPTIONS_INIT; @@ -508,11 +502,11 @@ static int ensure_there_are_changes_to_stash( opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR; opts.flags = GIT_STATUS_OPT_EXCLUDE_SUBMODULES; - if (include_untracked_files) + if (flags & GIT_STASH_INCLUDE_UNTRACKED) opts.flags |= GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS; - if (include_ignored_files) + if (flags & GIT_STASH_INCLUDE_IGNORED) opts.flags |= GIT_STATUS_OPT_INCLUDE_IGNORED | GIT_STATUS_OPT_RECURSE_IGNORED_DIRS; @@ -527,20 +521,14 @@ static int ensure_there_are_changes_to_stash( return error; } -static int reset_index_and_workdir( - git_repository *repo, - git_commit *commit, - bool remove_untracked, - bool remove_ignored) +static int reset_index_and_workdir(git_repository *repo, git_commit *commit, uint32_t flags) { git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT; opts.checkout_strategy = GIT_CHECKOUT_FORCE; - - if (remove_untracked) + if (flags & GIT_STASH_INCLUDE_UNTRACKED) opts.checkout_strategy |= GIT_CHECKOUT_REMOVE_UNTRACKED; - - if (remove_ignored) + if (flags & GIT_STASH_INCLUDE_IGNORED) opts.checkout_strategy |= GIT_CHECKOUT_REMOVE_IGNORED; return git_checkout_tree(repo, (git_object *)commit, &opts); @@ -566,31 +554,26 @@ int git_stash_save( if ((error = retrieve_base_commit_and_message(&b_commit, &msg, repo)) < 0) goto cleanup; - if ((error = ensure_there_are_changes_to_stash( - repo, - (flags & GIT_STASH_INCLUDE_UNTRACKED) != 0, - (flags & GIT_STASH_INCLUDE_IGNORED) != 0)) < 0) + if ((error = ensure_there_are_changes_to_stash(repo, flags)) < 0) goto cleanup; if ((error = git_repository_index(&index, repo)) < 0) goto cleanup; - if ((error = commit_index( - &i_commit, repo, index, stasher, git_buf_cstr(&msg), b_commit)) < 0) + if ((error = commit_index(&i_commit, repo, index, stasher, + git_buf_cstr(&msg), b_commit)) < 0) goto cleanup; if ((flags & (GIT_STASH_INCLUDE_UNTRACKED | GIT_STASH_INCLUDE_IGNORED)) && - (error = commit_untracked( - &u_commit, repo, stasher, git_buf_cstr(&msg), - i_commit, flags)) < 0) + (error = commit_untracked(&u_commit, repo, stasher, + git_buf_cstr(&msg), i_commit, flags)) < 0) goto cleanup; if ((error = prepare_worktree_commit_message(&msg, message)) < 0) goto cleanup; - if ((error = commit_worktree( - out, repo, stasher, git_buf_cstr(&msg), - i_commit, b_commit, u_commit)) < 0) + if ((error = commit_worktree(out, repo, stasher, git_buf_cstr(&msg), + i_commit, b_commit, u_commit)) < 0) goto cleanup; git_buf_rtrim(&msg); @@ -598,11 +581,8 @@ int git_stash_save( if ((error = update_reflog(out, repo, git_buf_cstr(&msg))) < 0) goto cleanup; - if ((error = reset_index_and_workdir( - repo, - ((flags & GIT_STASH_KEEP_INDEX) != 0) ? i_commit : b_commit, - (flags & GIT_STASH_INCLUDE_UNTRACKED) != 0, - (flags & GIT_STASH_INCLUDE_IGNORED) != 0)) < 0) + if ((error = reset_index_and_workdir(repo, (flags & GIT_STASH_KEEP_INDEX) ? i_commit : b_commit, + flags)) < 0) goto cleanup; cleanup: diff --git a/tests/refs/reflog/messages.c b/tests/refs/reflog/messages.c index f8acd23d2..43f59a84b 100644 --- a/tests/refs/reflog/messages.c +++ b/tests/refs/reflog/messages.c @@ -281,6 +281,27 @@ void test_refs_reflog_messages__creating_a_direct_reference(void) git_reference_free(reference); } +void test_refs_reflog_messages__newline_gets_replaced(void) +{ + const git_reflog_entry *entry; + git_signature *signature; + git_reflog *reflog; + git_oid oid; + + cl_git_pass(git_signature_now(&signature, "me", "foo@example.com")); + cl_git_pass(git_oid_fromstr(&oid, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750")); + + cl_git_pass(git_reflog_read(&reflog, g_repo, "HEAD")); + cl_assert_equal_sz(7, git_reflog_entrycount(reflog)); + cl_git_pass(git_reflog_append(reflog, &oid, signature, "inner\nnewline")); + cl_assert_equal_sz(8, git_reflog_entrycount(reflog)); + + cl_assert(entry = git_reflog_entry_byindex(reflog, 0)); + cl_assert_equal_s(git_reflog_entry_message(entry), "inner newline"); + + git_signature_free(signature); + git_reflog_free(reflog); +} void test_refs_reflog_messages__renaming_ref(void) { diff --git a/tests/refs/reflog/reflog.c b/tests/refs/reflog/reflog.c index 7e4b1ef4a..5cefc3227 100644 --- a/tests/refs/reflog/reflog.c +++ b/tests/refs/reflog/reflog.c @@ -87,15 +87,13 @@ void test_refs_reflog_reflog__append_then_read(void) cl_git_pass(git_signature_now(&committer, "foo", "foo@bar")); cl_git_pass(git_reflog_read(&reflog, g_repo, new_ref)); - - cl_git_fail(git_reflog_append(reflog, &oid, committer, "no inner\nnewline")); cl_git_pass(git_reflog_append(reflog, &oid, committer, NULL)); cl_git_pass(git_reflog_append(reflog, &oid, committer, commit_msg "\n")); cl_git_pass(git_reflog_write(reflog)); - git_reflog_free(reflog); assert_appends(committer, &oid); + git_reflog_free(reflog); git_signature_free(committer); } @@ -224,45 +222,45 @@ void test_refs_reflog_reflog__reading_the_reflog_from_a_reference_with_no_log_re git_buf_dispose(&subtrees_log_path); } -void test_refs_reflog_reflog__reading_a_reflog_with_invalid_format_returns_error(void) +void test_refs_reflog_reflog__reading_a_reflog_with_invalid_format_succeeds(void) { git_reflog *reflog; - const git_error *error; const char *refname = "refs/heads/newline"; const char *refmessage = "Reflog*message with a newline and enough content after it to pass the GIT_REFLOG_SIZE_MIN check inside reflog_parse."; + const git_reflog_entry *entry; git_reference *ref; git_oid id; git_buf logpath = GIT_BUF_INIT, logcontents = GIT_BUF_INIT; char *star; - git_oid_fromstr(&id, current_master_tip); - - /* create a new branch */ + /* Create a new branch. */ + cl_git_pass(git_oid_fromstr(&id, current_master_tip)); cl_git_pass(git_reference_create(&ref, g_repo, refname, &id, 1, refmessage)); - /* corrupt the branch reflog by introducing a newline inside the reflog message (we replace '*' with '\n') */ - git_buf_join_n(&logpath, '/', 3, git_repository_path(g_repo), GIT_REFLOG_DIR, refname); + /* + * Corrupt the branch reflog by introducing a newline inside the reflog message. + * We do this by replacing '*' with '\n' + */ + cl_git_pass(git_buf_join_n(&logpath, '/', 3, git_repository_path(g_repo), GIT_REFLOG_DIR, refname)); cl_git_pass(git_futils_readbuffer(&logcontents, git_buf_cstr(&logpath))); cl_assert((star = strchr(git_buf_cstr(&logcontents), '*')) != NULL); *star = '\n'; cl_git_rewritefile(git_buf_cstr(&logpath), git_buf_cstr(&logcontents)); - /* confirm that the file was rewritten successfully and now contains a '\n' in the expected location */ + /* + * Confirm that the file was rewritten successfully + * and now contains a '\n' in the expected location + */ cl_git_pass(git_futils_readbuffer(&logcontents, git_buf_cstr(&logpath))); cl_assert(strstr(git_buf_cstr(&logcontents), "Reflog\nmessage") != NULL); - /* clear the error state so we can capture the error generated by git_reflog_read */ - git_error_clear(); - - cl_git_fail(git_reflog_read(&reflog, g_repo, refname)); - - error = git_error_last(); - - cl_assert(error != NULL); - cl_assert_equal_s("unable to parse OID - contains invalid characters", error->message); + cl_git_pass(git_reflog_read(&reflog, g_repo, refname)); + cl_assert(entry = git_reflog_entry_byindex(reflog, 0)); + cl_assert_equal_s(git_reflog_entry_message(entry), "Reflog"); git_reference_free(ref); + git_reflog_free(reflog); git_buf_dispose(&logpath); git_buf_dispose(&logcontents); } diff --git a/tests/stash/save.c b/tests/stash/save.c index 362c704ea..d568567d5 100644 --- a/tests/stash/save.c +++ b/tests/stash/save.c @@ -283,6 +283,26 @@ void test_stash_save__stashing_updates_the_reflog(void) assert_object_oid("refs/stash@{1}", NULL, GIT_OBJECT_COMMIT); } +void test_stash_save__multiline_message(void) +{ + const char *msg = "This\n\nis a multiline message\n"; + const git_reflog_entry *entry; + git_reflog *reflog; + + assert_object_oid("refs/stash@{0}", NULL, GIT_OBJECT_COMMIT); + + cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, msg, GIT_STASH_DEFAULT)); + + cl_git_pass(git_reflog_read(&reflog, repo, "refs/stash")); + cl_assert(entry = git_reflog_entry_byindex(reflog, 0)); + cl_assert_equal_s(git_reflog_entry_message(entry), "On master: This is a multiline message"); + + assert_object_oid("refs/stash@{0}", git_oid_tostr_s(&stash_tip_oid), GIT_OBJECT_COMMIT); + assert_commit_message_contains("refs/stash@{0}", msg); + + git_reflog_free(reflog); +} + void test_stash_save__cannot_stash_when_there_are_no_local_change(void) { git_index *index; |