# [PATCH] gpg-interface: trim only CR characters that precede LF 6 messages from 2025-10-16 to 2025-10-16. Participants: Okhuomon Ajayi, Junio C Hamano, Kristoffer Haugsbakk. Thread: https://gitlist.dev/t/64336 ## Okhuomon Ajayi, 2025-10-16 18:44 Subject: [PATCH] gpg-interface: trim only CR characters that precede LF Message-ID: <20251016184420.78268-1-okhuomonajayi54@gmail.com> URL: https://gitlist.dev/e/20251016184420.78268-1-okhuomonajayi54%40gmail.com ``` The current implementation of remove_cr_after() drops every carriage return (CR) it finds, even when the CR is not part of a CRLF sequence. This can damage data that legitimately contains standalone CR bytes, such as binary payloads or text formatted for older systems. Update remove_cr_after() to remove a CR only when it is immediately followed by an LF. This keeps Windows-style CRLF normalization intact while preserving lone CR characters that are part of the data itself. Signed-off-by: Okhuomon Ajayi --- gpg-interface.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/gpg-interface.c b/gpg-interface.c index 2f4f0e32cb..c961607444 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -965,19 +965,22 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig } /* - * Strip CR from the line endings, in case we are on Windows. - * NEEDSWORK: make it trim only CRs before LFs and rename + * Trim CR characters only when they appear before LF (\r\n) line endings. + * This avoids removing legitimate lone CRs from teh content. */ -static void remove_cr_after(struct strbuf *buffer, size_t offset) +static void trim_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') { + /* skip CR only if it comes right before LF */ + if (buffer->buf[i] == '\r' && i + 1 < buffer->len && buffer->buf[i+1] == '\n') + continue; + if (i != j) buffer->buf[j] = buffer->buf[i]; j++; - } + } strbuf_setlen(buffer, j); } @@ -1023,8 +1026,10 @@ 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); + /* Trim carriage returns (CR) only when they appear before line feeds (LF),. + * mainly for handling Windows-style line endings + */ + trim_cr_before_lf(signature, bottom); return 0; } @@ -1110,8 +1115,10 @@ 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); + /* Trim carriage returns (CR) only when they appear before line feeds (LF), + * mainly for handling Windows-style line endings. + */ + trim_cr_before_lf(signature, bottom); out: if (key_file) -- 2.43.0 ``` ## Junio C Hamano, 2025-10-16 18:52 Subject: Re: [PATCH] gpg-interface: trim only CR characters that precede LF Message-ID: URL: https://gitlist.dev/e/xmqq4iry4r3e.fsf%40gitster.g In-Reply-To: <20251016184420.78268-1-okhuomonajayi54@gmail.com> ``` Okhuomon Ajayi writes: > /* > - * Strip CR from the line endings, in case we are on Windows. > - * NEEDSWORK: make it trim only CRs before LFs and rename > + * Trim CR characters only when they appear before LF (\r\n) line endings. > + * This avoids removing legitimate lone CRs from teh content. "teh" -> "the". I know, I myself often make teh same typo. > */ > -static void remove_cr_after(struct strbuf *buffer, size_t offset) > +static void trim_cr_before_lf(struct strbuf *buffer, size_t offset) In other words, this normalizes crlf to lf line ending. > { > size_t i, j; > > for (i = j = offset; i < buffer->len; i++) { > - if (buffer->buf[i] != '\r') { > + /* skip CR only if it comes right before LF */ > + if (buffer->buf[i] == '\r' && i + 1 < buffer->len && buffer->buf[i+1] == '\n') Are two different mixture of tabs and spaces used in the above two lines? I think they wanted to begin at the same column. Also, the second line is overly long that it does not even fit on my 92-column wide terminal (yes, 80 is the limit, but this will let a line in the patches quoted a few times to still fit, as long as the patch honors the 80-column limit). > + continue; > if (i != j) > buffer->buf[j] = buffer->buf[i]; > j++; > - } > + Do we need a blank line here? I dunno. > } > strbuf_setlen(buffer, j); > } > @@ -1023,8 +1026,10 @@ 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); > + /* Trim carriage returns (CR) only when they appear before line feeds (LF),. > + * mainly for handling Windows-style line endings > + */ /* Convert CRLF to LF, in case we are on Windows */ > + trim_cr_before_lf(signature, bottom); > > return 0; > } > @@ -1110,8 +1115,10 @@ 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); > + /* Trim carriage returns (CR) only when they appear before line feeds (LF), > + * mainly for handling Windows-style line endings. > + */ > + trim_cr_before_lf(signature, bottom); Ditto. > > out: > if (key_file) ``` ## Okhuomon Ajayi, 2025-10-16 19:38 Subject: Re: [PATCH] gpg-interface: trim only CR characters that precede LF Message-ID: URL: https://gitlist.dev/e/CAFpMFfBe7%2BpMUL8aaDkGkPUaE9RhCW25OJhJy69EcukgSFn9%2BA%40mail.gmail.com In-Reply-To: ``` Hi Junio, Haha, I smiled at your “teh” comment — I myself often make teh same typo Thanks a lot for catching the typo and for the detailed feedback on style and indentation. I’ll fix the tab/space mix, shorten the long line, and use your suggested comment wording in the next revision On Thu, Oct 16, 2025 at 7:52 PM Junio C Hamano wrote: > > Okhuomon Ajayi writes: > > > /* > > - * Strip CR from the line endings, in case we are on Windows. > > - * NEEDSWORK: make it trim only CRs before LFs and rename > > + * Trim CR characters only when they appear before LF (\r\n) line endings. > > + * This avoids removing legitimate lone CRs from teh content. > > "teh" -> "the". I know, I myself often make teh same typo. > > > */ > > -static void remove_cr_after(struct strbuf *buffer, size_t offset) > > +static void trim_cr_before_lf(struct strbuf *buffer, size_t offset) > > In other words, this normalizes crlf to lf line ending. > > > { > > size_t i, j; > > > > for (i = j = offset; i < buffer->len; i++) { > > - if (buffer->buf[i] != '\r') { > > + /* skip CR only if it comes right before LF */ > > + if (buffer->buf[i] == '\r' && i + 1 < buffer->len && buffer->buf[i+1] == '\n') > > Are two different mixture of tabs and spaces used in the above two > lines? I think they wanted to begin at the same column. > > Also, the second line is overly long that it does not even fit on my > 92-column wide terminal (yes, 80 is the limit, but this will let a > line in the patches quoted a few times to still fit, as long as the > patch honors the 80-column limit). > > > + continue; > > > if (i != j) > > buffer->buf[j] = buffer->buf[i]; > > j++; > > - } > > + > > Do we need a blank line here? I dunno. > > > } > > strbuf_setlen(buffer, j); > > } > > @@ -1023,8 +1026,10 @@ 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); > > + /* Trim carriage returns (CR) only when they appear before line feeds (LF),. > > + * mainly for handling Windows-style line endings > > + */ > > /* Convert CRLF to LF, in case we are on Windows */ > > > + trim_cr_before_lf(signature, bottom); > > > > return 0; > > } > > @@ -1110,8 +1115,10 @@ 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); > > + /* Trim carriage returns (CR) only when they appear before line feeds (LF), > > + * mainly for handling Windows-style line endings. > > + */ > > + trim_cr_before_lf(signature, bottom); > > Ditto. > > > > > out: > > if (key_file) ``` ## Junio C Hamano, 2025-10-16 20:50 Subject: Re: [PATCH] gpg-interface: trim only CR characters that precede LF Message-ID: URL: https://gitlist.dev/e/xmqq7bwu3716.fsf%40gitster.g In-Reply-To: ``` Okhuomon Ajayi writes: > Hi Junio, > Haha, I smiled at your “teh” comment — I myself often make teh same typo > Thanks a lot for catching the typo and for the detailed feedback on > style and indentation. > I’ll fix the tab/space mix, shorten the long line, and use your > suggested comment wording in the next revision I was hinting that the new function name is less than optimal, which may not have been conveyed very well X-<. >> > -static void remove_cr_after(struct strbuf *buffer, size_t offset) >> > +static void trim_cr_before_lf(struct strbuf *buffer, size_t offset) >> >> In other words, this normalizes crlf to lf line ending. ``` ## Kristoffer Haugsbakk, 2025-10-16 20:53 Subject: Re: [PATCH] gpg-interface: trim only CR characters that precede LF Message-ID: <5b52ee84-8889-4357-ac46-93ce5b6b100e@app.fastmail.com> URL: https://gitlist.dev/e/5b52ee84-8889-4357-ac46-93ce5b6b100e%40app.fastmail.com In-Reply-To: ``` On Thu, Oct 16, 2025, at 21:38, Okhuomon Ajayi wrote: > Hi Junio, > Haha, I smiled at your “teh” comment — I myself often make teh same typo But on the other hand I think the the easiest mistake to overlook is when the article is doubled. ``` ## Okhuomon Ajayi, 2025-10-16 21:01 Subject: Re: [PATCH] gpg-interface: trim only CR characters that precede LF Message-ID: URL: https://gitlist.dev/e/CAFpMFfC1cut5%3DqwoRfvv%2BzCgqvN6z2WS%3DR7ynjwSd6LB0aJD0g%40mail.gmail.com In-Reply-To: <5b52ee84-8889-4357-ac46-93ce5b6b100e@app.fastmail.com> ``` Haha, Yeah Kristoffer! , I see how “the the” can sneak in I’ll watch out for that too 😄 On Thu, Oct 16, 2025 at 9:54 PM Kristoffer Haugsbakk wrote: > > On Thu, Oct 16, 2025, at 21:38, Okhuomon Ajayi wrote: > > Hi Junio, > > Haha, I smiled at your “teh” comment — I myself often make teh same typo > > But on the other hand I think the the easiest mistake to overlook is > when the article is doubled. ```