[PATCH] gpg-interface: trim only CR characters that precede LF

[PATCH] gpg-interface: trim only CR characters that precede LF

[Okhuomon Ajayi]

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 <okhuomonajayi54@gmail.com>
---
 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

Re: [PATCH] gpg-interface: trim only CR characters that precede LF

[Junio C Hamano]

Okhuomon Ajayi <okhuomonajayi54@gmail.com> 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)

Re: [PATCH] gpg-interface: trim only CR characters that precede LF

[Okhuomon Ajayi]

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 <gitster@pobox.com> wrote:
>
> Okhuomon Ajayi <okhuomonajayi54@gmail.com> 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)

Re: [PATCH] gpg-interface: trim only CR characters that precede LF

[Junio C Hamano]

Okhuomon Ajayi <okhuomonajayi54@gmail.com> 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.

Re: [PATCH] gpg-interface: trim only CR characters that precede LF

[Kristoffer Haugsbakk]

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.

Re: [PATCH] gpg-interface: trim only CR characters that precede LF

[Okhuomon Ajayi]

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
<kristofferhaugsbakk@fastmail.com> 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.