From: Florian Schmidt <flosch@nutanix.com>
To: git@vger.kernel.org
Cc: Florian Schmidt <flosch@nutanix.com>,
Jonathan Davies <jonathan.davies@nutanix.com>,
Phillip Wood <phillip.wood@dunelm.org.uk>,
Junio C Hamano <gitster@pobox.com>,
Denton Liu <liu.denton@gmail.com>
Subject: [PATCH] wt-status: Don't find scissors line beyond buf len
Date: Thu, 7 Mar 2024 18:37:38 +0000 [thread overview]
Message-ID: <20240307183743.219951-1-flosch@nutanix.com> (raw)
Currently, if
(a) There is a "---" divider in a commit message,
(b) At some point beyond that divider, there is a cut-line (that is,
"# ------------------------ >8 ------------------------") in the
commit message,
(c) the user does not explicitly set the "no-divider" option,
then "git interpret-trailers" will hang indefinitively.
This is because when (a) is true, find_end_of_log_message() will invoke
ignored_log_message_bytes() with a len that is intended to make it
ignore the part of the commit message beyond the divider. However,
ignored_log_message_bytes() calls wt_status_locate_end(), and that
function ignores the length restriction when it tries to locate the cut
line. If it manages to find one, the returned cutoff value is greater
than len. At this point, ignored_log_message_bytes() goes into an
infinite loop, because it won't advance the string parsing beyond len,
but the exit condition expects to reach cutoff.
It seems sensible to expect that wt_status_locate_end() should honour
the length parameter passed in, and doing so fixes this issue.
Signed-off-by: Florian Schmidt <flosch@nutanix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@nutanix.com>
---
Side remark: Since strstr() doesn't consider len, and will always search
up to a null byte, I now wonder whether it would be safer to create a
new strbuf that only contains the len bytes we want to operate on. If
anybody ever thinks they can pass a non-null-terminated string into
wt_status_locate_end() because they already provide a len parameter,
they will not have a good time. So it's that traded off against the
slightly higher overhead of creating yet another buffer and copying a
potentially large-ish commit message around.
t/t7513-interpret-trailers.sh | 14 ++++++++++++++
wt-status.c | 13 +++++++++----
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index ec9c6de114..3d3e13ccf8 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -1935,4 +1935,18 @@ test_expect_success 'suppressing --- does not disable cut-line handling' '
test_cmp expected actual
'
+test_expect_success 'handling of --- lines in conjunction with cut-lines' '
+ echo "my-trailer: here" >expected &&
+
+ git interpret-trailers --parse >actual <<-\EOF &&
+ subject
+
+ my-trailer: here
+ ---
+ # ------------------------ >8 ------------------------
+ EOF
+
+ test_cmp expected actual
+'
+
test_done
diff --git a/wt-status.c b/wt-status.c
index b5a29083df..51a84575ed 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1089,14 +1089,19 @@ size_t wt_status_locate_end(const char *s, size_t len)
{
const char *p;
struct strbuf pattern = STRBUF_INIT;
+ size_t result = len;
strbuf_addf(&pattern, "\n%c %s", comment_line_char, cut_line);
if (starts_with(s, pattern.buf + 1))
- len = 0;
- else if ((p = strstr(s, pattern.buf)))
- len = p - s + 1;
+ result = 0;
+ else if ((p = strstr(s, pattern.buf))) {
+ result = p - s + 1;
+ if (result > len) {
+ result = len;
+ }
+ }
strbuf_release(&pattern);
- return len;
+ return result;
}
void wt_status_append_cut_line(struct strbuf *buf)
--
2.42.0
next reply other threads:[~2024-03-07 18:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-07 18:37 Florian Schmidt [this message]
2024-03-07 19:20 ` [PATCH] wt-status: Don't find scissors line beyond buf len Junio C Hamano
2024-03-07 20:47 ` Junio C Hamano
2024-03-07 21:09 ` Eric Sunshine
2024-03-07 21:15 ` Kristoffer Haugsbakk
2024-03-07 21:24 ` Junio C Hamano
2024-03-07 21:26 ` Eric Sunshine
2024-03-07 21:30 ` Kristoffer Haugsbakk
2024-03-08 9:08 ` Florian Schmidt
2024-03-08 15:26 ` Junio C Hamano
2024-03-08 17:43 ` Florian Schmidt
2024-04-06 1:37 ` Linus Arver
2024-03-07 19:35 ` Junio C Hamano
2024-03-08 9:13 ` Florian Schmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240307183743.219951-1-flosch@nutanix.com \
--to=flosch@nutanix.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jonathan.davies@nutanix.com \
--cc=liu.denton@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).