@matheusbsbs / matheus-bagliano / commits / 523fa69c36

Fork 0

reflog: cleanse messages in the refs.c layer

Regarding reflog messages: - We expect that a reflog message consists of a single line. The file format used by the files backend may add a LF after the message as a delimiter, and output by commands like "git log -g" may complete such an incomplete line by adding a LF at the end, but philosophically, the terminating LF is not a part of the message. - We however allow callers of refs API to supply a random sequence of NUL terminated bytes. We cleanse caller-supplied message by squashing a run of whitespaces into a SP, and by trimming trailing whitespace, before storing the message. This is how we tolerate, instead of erring out, a message with LF in it (be it at the end, in the middle, or both). Currently, the cleansing of the reflog message is done by the files backend, before the log is written out. This is sufficient with the current code, as that is the only backend that writes reflogs. But new backends can be added that write reflogs, and we'd want the resulting log message we would read out of "log -g" the same no matter what backend is used, and moving the code to do so to the generic layer is a way to do so. An added benefit is that the "cleansing" function could be updated later, independent from individual backends, to e.g. allow multi-line log messages if we wanted to, and when that happens, it would help a lot to ensure we covered all bases if the cleansing function (which would be updated) is called from the generic layer. Side note: I am not interested in supporting multi-line reflog messages right at the moment (nobody is asking for it), but I envision that instead of the "squash a run of whitespaces into a SP and rtrim" cleansing, we can %urlencode problematic bytes in the message *AND* append a SP at the end, when a new version of Git that supports multi-line and/or verbatim reflog messages writes a reflog record. The reading side can detect the presense of SP at the end (which should have been rtrimmed out if it were written by existing versions of Git) as a signal that decoding %urlencode recovers the original reflog message. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Jul 10, 2020 at 17:19 UTC 523fa69c36744ae6779e38614cb9bfb2be552923

3 files changed +42 -16

refs.c

+41 -9

@@ -870,7 +870,7 @@ int delete_ref(const char *msg, const char *refname,

870 old_oid, flags);

871 }

872

873 -void copy_reflog_msg(struct strbuf *sb, const char *msg)

873 +static void copy_reflog_msg(struct strbuf *sb, const char *msg)

874 {

875 char c;

876 int wasspace = 1;

@@ -887,6 +887,15 @@ void copy_reflog_msg(struct strbuf *sb, const char *msg)

887 strbuf_rtrim(sb);

888 }

889

890 +static char *normalize_reflog_message(const char *msg)

891 +{

892 + struct strbuf sb = STRBUF_INIT;

893 +

894 + if (msg && *msg)

895 + copy_reflog_msg(&sb, msg);

896 + return strbuf_detach(&sb, NULL);

897 +}

898 +

899 int should_autocreate_reflog(const char *refname)

900 {

901 switch (log_all_ref_updates) {

@@ -1092,7 +1101,7 @@ struct ref_update *ref_transaction_add_update(

1101 oidcpy(&update->new_oid, new_oid);

1102 if (flags & REF_HAVE_OLD)

1103 oidcpy(&update->old_oid, old_oid);

1095 - update->msg = xstrdup_or_null(msg);

1104 + update->msg = normalize_reflog_message(msg);

1105 return update;

1106 }

1107

@@ -1951,9 +1960,14 @@ int refs_create_symref(struct ref_store *refs,

1960 const char *refs_heads_master,

1961 const char *logmsg)

1962 {

1954 - return refs->be->create_symref(refs, ref_target,

1955 - refs_heads_master,

1956 - logmsg);

1963 + char *msg;

1964 + int retval;

1965 +

1966 + msg = normalize_reflog_message(logmsg);

1967 + retval = refs->be->create_symref(refs, ref_target, refs_heads_master,

1968 + msg);

1969 + free(msg);

1970 + return retval;

1971 }

1972

1973 int create_symref(const char *ref_target, const char *refs_heads_master,

@@ -2268,10 +2282,16 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,

2282 return refs->be->initial_transaction_commit(refs, transaction, err);

2283 }

2284

2271 -int refs_delete_refs(struct ref_store *refs, const char *msg,

2285 +int refs_delete_refs(struct ref_store *refs, const char *logmsg,

2286 struct string_list *refnames, unsigned int flags)

2287 {

2274 - return refs->be->delete_refs(refs, msg, refnames, flags);

2288 + char *msg;

2289 + int retval;

2290 +

2291 + msg = normalize_reflog_message(logmsg);

2292 + retval = refs->be->delete_refs(refs, msg, refnames, flags);

2293 + free(msg);

2294 + return retval;

2295 }

2296

2297 int delete_refs(const char *msg, struct string_list *refnames,

@@ -2283,7 +2303,13 @@ int delete_refs(const char *msg, struct string_list *refnames,

2303 int refs_rename_ref(struct ref_store *refs, const char *oldref,

2304 const char *newref, const char *logmsg)

2305 {

2286 - return refs->be->rename_ref(refs, oldref, newref, logmsg);

2306 + char *msg;

2307 + int retval;

2308 +

2309 + msg = normalize_reflog_message(logmsg);

2310 + retval = refs->be->rename_ref(refs, oldref, newref, msg);

2311 + free(msg);

2312 + return retval;

2313 }

2314

2315 int rename_ref(const char *oldref, const char *newref, const char *logmsg)

@@ -2294,7 +2320,13 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)

2320 int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,

2321 const char *newref, const char *logmsg)

2322 {

2297 - return refs->be->copy_ref(refs, oldref, newref, logmsg);

2323 + char *msg;

2324 + int retval;

2325 +

2326 + msg = normalize_reflog_message(logmsg);

2327 + retval = refs->be->copy_ref(refs, oldref, newref, msg);

2328 + free(msg);

2329 + return retval;

2330 }

2331

2332 int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)

refs/files-backend.c

+1 -1

@@ -1629,7 +1629,7 @@ static int log_ref_write_fd(int fd, const struct object_id *old_oid,

1629

1630 strbuf_addf(&sb, "%s %s %s", oid_to_hex(old_oid), oid_to_hex(new_oid), committer);

1631 if (msg && *msg)

1632 - copy_reflog_msg(&sb, msg);

1632 + strbuf_addstr(&sb, msg);

1633 strbuf_addch(&sb, '\n');

1634 if (write_in_full(fd, sb.buf, sb.len) < 0)

1635 ret = -1;

refs/refs-internal.h

-6

@@ -96,12 +96,6 @@ enum peel_status {

96 */

97 enum peel_status peel_object(const struct object_id *name, struct object_id *oid);

99 -/*

100 - * Copy the reflog message msg to sb while cleaning up the whitespaces.

101 - * Especially, convert LF to space, because reflog file is one line per entry.

102 - */

103 -void copy_reflog_msg(struct strbuf *sb, const char *msg);

104 -

99 /**

100 * Information needed for a single ref update. Set new_oid to the new

101 * value or to null_oid to delete the ref. To check the old value