{"thread":{"id":"64336","subject":"[PATCH] gpg-interface: trim only CR characters that precede LF","startedAt":"2025-10-16T18:44:42Z","lastAt":"2025-10-16T21:01:37Z","messageCount":6,"participants":["Okhuomon Ajayi","Junio C Hamano","Kristoffer Haugsbakk"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"528982","messageId":"20251016184420.78268-1-okhuomonajayi54@gmail.com","threadId":"64336","inReplyTo":null,"subject":"[PATCH] gpg-interface: trim only CR characters that precede LF","fromName":"Okhuomon Ajayi","fromEmail":"okhuomonajayi54@gmail.com","sentAt":"2025-10-16T18:44:20Z","receivedAt":"2025-10-16T18:44:42Z","isPatch":true,"sender":{"key":"okhuomonajayi54@gmail.com","avatar":null},"body":"The current implementation of remove_cr_after() drops every carriage\nreturn (CR) it finds, even when the CR is not part of a CRLF sequence.\nThis can damage data that legitimately contains standalone CR bytes,\nsuch as binary payloads or text formatted for older systems.\n\nUpdate remove_cr_after() to remove a CR only when it is immediately\nfollowed by an LF. This keeps Windows-style CRLF normalization intact\nwhile preserving lone CR characters that are part of the data itself.\n\nSigned-off-by: Okhuomon Ajayi \n---\n gpg-interface.c | 25 ++++++++++++++++---------\n 1 file changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 2f4f0e32cb..c961607444 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -965,19 +965,22 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n }\n \n /*\n- * Strip CR from the line endings, in case we are on Windows.\n- * NEEDSWORK: make it trim only CRs before LFs and rename\n+ * Trim CR characters only when they appear before LF (\\r\\n) line endings.\n+ * This avoids removing legitimate lone CRs from teh content.\n */\n-static void remove_cr_after(struct strbuf *buffer, size_t offset)\n+static void trim_cr_before_lf(struct strbuf *buffer, size_t offset)\n {\n \tsize_t i, j;\n \n \tfor (i = j = offset; i < buffer->len; i++) {\n-\t\tif (buffer->buf[i] != '\\r') {\n+\t /* skip CR only if it comes right before LF */\n+\t\tif (buffer->buf[i] == '\\r' && i + 1 < buffer->len && buffer->buf[i+1] == '\\n')\n+\t\t continue;\n+ \n \t\t\tif (i != j)\n \t\t\t\tbuffer->buf[j] = buffer->buf[i];\n \t\t\tj++;\n-\t\t}\n+\t\t\n \t}\n \tstrbuf_setlen(buffer, j);\n }\n@@ -1023,8 +1026,10 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n \t}\n \tstrbuf_release(&gpg_status);\n \n-\t/* Strip CR from the line endings, in case we are on Windows. */\n-\tremove_cr_after(signature, bottom);\n+\t/* Trim carriage returns (CR) only when they appear before line feeds (LF),.\n+\t* mainly for handling Windows-style line endings\n+ \t*/\n+\ttrim_cr_before_lf(signature, bottom);\n \n \treturn 0;\n }\n@@ -1110,8 +1115,10 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,\n \t\t\tssh_signature_filename.buf);\n \t\tgoto out;\n \t}\n-\t/* Strip CR from the line endings, in case we are on Windows. */\n-\tremove_cr_after(signature, bottom);\n+\t/* Trim carriage returns (CR) only when they appear before line feeds (LF),\n+\t* mainly for handling Windows-style line endings.\n+\t*/\n+\ttrim_cr_before_lf(signature, bottom);\n \n out:\n \tif (key_file)\n-- \n2.43.0\n\n"},{"id":"528984","messageId":"xmqq4iry4r3e.fsf@gitster.g","threadId":"64336","inReplyTo":"20251016184420.78268-1-okhuomonajayi54@gmail.com","subject":"Re: [PATCH] gpg-interface: trim only CR characters that precede LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-16T18:52:05Z","receivedAt":"2025-10-16T18:52:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Okhuomon Ajayi writes:\n\n> /*\n> - * Strip CR from the line endings, in case we are on Windows.\n> - * NEEDSWORK: make it trim only CRs before LFs and rename\n> + * Trim CR characters only when they appear before LF (\\r\\n) line endings.\n> + * This avoids removing legitimate lone CRs from teh content.\n\n\"teh\" -> \"the\". I know, I myself often make teh same typo.\n\n> */\n> -static void remove_cr_after(struct strbuf *buffer, size_t offset)\n> +static void trim_cr_before_lf(struct strbuf *buffer, size_t offset)\n\nIn other words, this normalizes crlf to lf line ending.\n\n> {\n> \tsize_t i, j;\n> \n> \tfor (i = j = offset; i < buffer->len; i++) {\n> -\t\tif (buffer->buf[i] != '\\r') {\n> +\t /* skip CR only if it comes right before LF */\n> +\t\tif (buffer->buf[i] == '\\r' && i + 1 < buffer->len && buffer->buf[i+1] == '\\n')\n\nAre two different mixture of tabs and spaces used in the above two\nlines? I think they wanted to begin at the same column.\n\nAlso, the second line is overly long that it does not even fit on my\n92-column wide terminal (yes, 80 is the limit, but this will let a\nline in the patches quoted a few times to still fit, as long as the\npatch honors the 80-column limit).\n\n> +\t\t continue;\n\n> \t\t\tif (i != j)\n> \t\t\t\tbuffer->buf[j] = buffer->buf[i];\n> \t\t\tj++;\n> -\t\t}\n> +\t\t\n\nDo we need a blank line here? I dunno.\n\n> \t}\n> \tstrbuf_setlen(buffer, j);\n> }\n> @@ -1023,8 +1026,10 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n> \t}\n> \tstrbuf_release(&gpg_status);\n> \n> -\t/* Strip CR from the line endings, in case we are on Windows. */\n> -\tremove_cr_after(signature, bottom);\n> +\t/* Trim carriage returns (CR) only when they appear before line feeds (LF),.\n> +\t* mainly for handling Windows-style line endings\n> + \t*/\n\n\t/* Convert CRLF to LF, in case we are on Windows */\n\n> +\ttrim_cr_before_lf(signature, bottom);\n> \n> \treturn 0;\n> }\n> @@ -1110,8 +1115,10 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,\n> \t\t\tssh_signature_filename.buf);\n> \t\tgoto out;\n> \t}\n> -\t/* Strip CR from the line endings, in case we are on Windows. */\n> -\tremove_cr_after(signature, bottom);\n> +\t/* Trim carriage returns (CR) only when they appear before line feeds (LF),\n> +\t* mainly for handling Windows-style line endings.\n> +\t*/\n> +\ttrim_cr_before_lf(signature, bottom);\n\nDitto.\n\n> \n> out:\n> \tif (key_file)\n"},{"id":"528993","messageId":"CAFpMFfBe7+pMUL8aaDkGkPUaE9RhCW25OJhJy69EcukgSFn9+A@mail.gmail.com","threadId":"64336","inReplyTo":"xmqq4iry4r3e.fsf@gitster.g","subject":"Re: [PATCH] gpg-interface: trim only CR characters that precede LF","fromName":"Okhuomon Ajayi","fromEmail":"okhuomonajayi54@gmail.com","sentAt":"2025-10-16T19:38:11Z","receivedAt":"2025-10-16T19:38:23Z","isPatch":true,"sender":{"key":"okhuomonajayi54@gmail.com","avatar":null},"body":"Hi Junio,\nHaha, I smiled at your “teh” comment — I myself often make teh same typo\nThanks a lot for catching the typo and for the detailed feedback on\nstyle and indentation.\nI’ll fix the tab/space mix, shorten the long line, and use your\nsuggested comment wording in the next revision\n\nOn Thu, Oct 16, 2025 at 7:52 PM Junio C Hamano wrote:\n>\n> Okhuomon Ajayi writes:\n>\n> > /*\n> > - * Strip CR from the line endings, in case we are on Windows.\n> > - * NEEDSWORK: make it trim only CRs before LFs and rename\n> > + * Trim CR characters only when they appear before LF (\\r\\n) line endings.\n> > + * This avoids removing legitimate lone CRs from teh content.\n>\n> \"teh\" -> \"the\". I know, I myself often make teh same typo.\n>\n> > */\n> > -static void remove_cr_after(struct strbuf *buffer, size_t offset)\n> > +static void trim_cr_before_lf(struct strbuf *buffer, size_t offset)\n>\n> In other words, this normalizes crlf to lf line ending.\n>\n> > {\n> > size_t i, j;\n> >\n> > for (i = j = offset; i < buffer->len; i++) {\n> > - if (buffer->buf[i] != '\\r') {\n> > + /* skip CR only if it comes right before LF */\n> > + if (buffer->buf[i] == '\\r' && i + 1 < buffer->len && buffer->buf[i+1] == '\\n')\n>\n> Are two different mixture of tabs and spaces used in the above two\n> lines? I think they wanted to begin at the same column.\n>\n> Also, the second line is overly long that it does not even fit on my\n> 92-column wide terminal (yes, 80 is the limit, but this will let a\n> line in the patches quoted a few times to still fit, as long as the\n> patch honors the 80-column limit).\n>\n> > + continue;\n>\n> > if (i != j)\n> > buffer->buf[j] = buffer->buf[i];\n> > j++;\n> > - }\n> > +\n>\n> Do we need a blank line here? I dunno.\n>\n> > }\n> > strbuf_setlen(buffer, j);\n> > }\n> > @@ -1023,8 +1026,10 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,\n> > }\n> > strbuf_release(&gpg_status);\n> >\n> > - /* Strip CR from the line endings, in case we are on Windows. */\n> > - remove_cr_after(signature, bottom);\n> > + /* Trim carriage returns (CR) only when they appear before line feeds (LF),.\n> > + * mainly for handling Windows-style line endings\n> > + */\n>\n> /* Convert CRLF to LF, in case we are on Windows */\n>\n> > + trim_cr_before_lf(signature, bottom);\n> >\n> > return 0;\n> > }\n> > @@ -1110,8 +1115,10 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,\n> > ssh_signature_filename.buf);\n> > goto out;\n> > }\n> > - /* Strip CR from the line endings, in case we are on Windows. */\n> > - remove_cr_after(signature, bottom);\n> > + /* Trim carriage returns (CR) only when they appear before line feeds (LF),\n> > + * mainly for handling Windows-style line endings.\n> > + */\n> > + trim_cr_before_lf(signature, bottom);\n>\n> Ditto.\n>\n> >\n> > out:\n> > if (key_file)\n"},{"id":"529011","messageId":"xmqq7bwu3716.fsf@gitster.g","threadId":"64336","inReplyTo":"CAFpMFfBe7+pMUL8aaDkGkPUaE9RhCW25OJhJy69EcukgSFn9+A@mail.gmail.com","subject":"Re: [PATCH] gpg-interface: trim only CR characters that precede LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-16T20:50:45Z","receivedAt":"2025-10-16T20:50:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Okhuomon Ajayi writes:\n\n> Hi Junio,\n> Haha, I smiled at your “teh” comment — I myself often make teh same typo\n> Thanks a lot for catching the typo and for the detailed feedback on\n> style and indentation.\n> I’ll fix the tab/space mix, shorten the long line, and use your\n> suggested comment wording in the next revision\n\nI was hinting that the new function name is less than optimal, which\nmay not have been conveyed very well X-<.\n\n>> > -static void remove_cr_after(struct strbuf *buffer, size_t offset)\n>> > +static void trim_cr_before_lf(struct strbuf *buffer, size_t offset)\n>>\n>> In other words, this normalizes crlf to lf line ending.\n"},{"id":"529013","messageId":"5b52ee84-8889-4357-ac46-93ce5b6b100e@app.fastmail.com","threadId":"64336","inReplyTo":"CAFpMFfBe7+pMUL8aaDkGkPUaE9RhCW25OJhJy69EcukgSFn9+A@mail.gmail.com","subject":"Re: [PATCH] gpg-interface: trim only CR characters that precede LF","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-10-16T20:53:49Z","receivedAt":"2025-10-16T20:54:12Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Thu, Oct 16, 2025, at 21:38, Okhuomon Ajayi wrote:\n> Hi Junio,\n> Haha, I smiled at your “teh” comment — I myself often make teh same typo\n\nBut on the other hand I think the the easiest mistake to overlook is\nwhen the article is doubled.\n"},{"id":"529017","messageId":"CAFpMFfC1cut5=qwoRfvv+zCgqvN6z2WS=R7ynjwSd6LB0aJD0g@mail.gmail.com","threadId":"64336","inReplyTo":"5b52ee84-8889-4357-ac46-93ce5b6b100e@app.fastmail.com","subject":"Re: [PATCH] gpg-interface: trim only CR characters that precede LF","fromName":"Okhuomon Ajayi","fromEmail":"okhuomonajayi54@gmail.com","sentAt":"2025-10-16T21:01:24Z","receivedAt":"2025-10-16T21:01:37Z","isPatch":true,"sender":{"key":"okhuomonajayi54@gmail.com","avatar":null},"body":"Haha, Yeah Kristoffer! , I see how “the the” can sneak in I’ll watch\nout for that too 😄\n\nOn Thu, Oct 16, 2025 at 9:54 PM Kristoffer Haugsbakk\n wrote:\n>\n> On Thu, Oct 16, 2025, at 21:38, Okhuomon Ajayi wrote:\n> > Hi Junio,\n> > Haha, I smiled at your “teh” comment — I myself often make teh same typo\n>\n> But on the other hand I think the the easiest mistake to overlook is\n> when the article is doubled.\n"}]}