c4adea82 (Convert CR/LF to LF in tag signatures, 2008-07-11) introduced CR stripping for GPG output on Windows, but intentionally stripped all CR characters unconditionally to "keep the code simpler", even though only \r\n sequences (Windows line endings) needed to be normalized. 2f47eae2 (Split GPG interface into its own helper library, 2011-09-07) moved the code into gpg-interface.c, and 29b31577 (ssh signing: add ssh key format and signing code, 2021-09-10) extracted it into the remove_cr_after() helper when adding SSH signing support.
The original laziness was safe at the time because lone CR characters are not expected in GPG signature output. However, the NEEDSWORK comment left by a previous reader correctly identified that only \r\n pairs should be stripped, not lone \r characters.
Fix the loop to skip \r only when immediately followed by \n, keeping lone trailing CR characters intact. Rename the function to strip_cr_before_lf to reflect its corrected behavior, and update both call sites and their comments accordingly.
Signed-off-by: Antonio De Stefani <antonio.destefani08@gmail.com> --- gpg-interface.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-)
Show changes to gpg-interface.c +11 −14
diff --git a/gpg-interface.c b/gpg-interface.c index dafd5371fa..95abf1ef4e 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -990,21 +990,18 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, return ret; } -/* - * Strip CR from the line endings, in case we are on Windows. - * NEEDSWORK: make it trim only CRs before LFs and rename - */ -static void remove_cr_after(struct strbuf *buffer, size_t offset) +/* Strip CR before LF from the line endings, in case we are on Windows. */ +static void strip_cr_before_lf(struct strbuf *buffer, size_t offset) { size_t i, j; for (i = j = offset; i < buffer->len; i++) { - if (buffer->buf[i] != '\r') { - if (i != j) - buffer->buf[j] = buffer->buf[i]; - j++; - } + if (buffer->buf[i] == '\r' && + i + 1 < buffer->len && buffer->buf[i + 1] == '\n') + continue; + buffer->buf[j++] = buffer->buf[i]; } + strbuf_setlen(buffer, j); } @@ -1049,8 +1046,8 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature, } strbuf_release(&gpg_status); - /* Strip CR from the line endings, in case we are on Windows. */ - remove_cr_after(signature, bottom); + /* Strip CR before LF from the line endings, in case we are on Windows. */ + strip_cr_before_lf(signature, bottom); return 0; } @@ -1136,8 +1133,8 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, ssh_signature_filename.buf); goto out; } - /* Strip CR from the line endings, in case we are on Windows. */ - remove_cr_after(signature, bottom); + /* Strip CR before LF from the line endings, in case we are on Windows. */ + strip_cr_before_lf(signature, bottom); out: if (key_file)
-- 2.54.0