From: Nitzan Gur-Furman Date: Wed, 6 Dec 2023 13:47:27 +0000 (+0100) Subject: FooterLines: handle extraction from messages without headers X-Git-Tag: v6.9.0.202402211805-m3~31^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fchanges%2F44%2F1173244%2F8;p=jgit.git FooterLines: handle extraction from messages without headers Prior to this change, long subjects of messages with no headers were treated as headers, and therefore were skipped. In a message of the form `\n\n`, the footers would then get parsed as a message, meaning no footers were returned. After this change, the first lines are skipped only if they match any of the known headers. The first line ofter the optional headers is then assumed to be the subject line. `FooterLineTest` had a few test cases for extracting footers from messages with no headers. However, there were all with short messages, so the "skip this line" logic in `RawParseUtils` was never triggered. Added test case to catch this issue. Change-Id: I971a1dddf1a9aea094360c3c8fc3b9a8b011bbf9 Issue: Google b/287891316 --- diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java index a6808f6351..cac0743d68 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java @@ -97,6 +97,15 @@ public class FooterLineTest extends RepositoryTestCase { assertEquals(1, footers.size()); } + @Test + public void testOneFooter_longSubject_NoHeaders() { + String noRawMsg = "50+ chars loooooooooooooong custom commit message.\n\n" + + "Footer: value\n"; + List footers = FooterLine.fromMessage(noRawMsg); + assertNotNull(footers); + assertEquals(1, footers.size()); + } + @Test public void testSignedOffBy_OneUserNoLF() { String msg = buildMessage("subject\n\nbody of commit\n" + "\n" diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java index d651b63afb..227ea0fd65 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java @@ -81,7 +81,11 @@ public final class FooterLine { // empty } - int msgB = RawParseUtils.commitMessage(raw, 0); + // The first non-header line is never a footer. + int msgB = RawParseUtils.nextLfSkippingSplitLines(raw, + RawParseUtils.hasAnyKnownHeaders(raw) + ? RawParseUtils.commitMessage(raw, 0) + : 0); ArrayList r = new ArrayList<>(4); Charset enc = RawParseUtils.guessEncoding(raw); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java index 71fae815fe..fccdca8b9d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java @@ -16,7 +16,12 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jgit.lib.ObjectChecker.author; import static org.eclipse.jgit.lib.ObjectChecker.committer; import static org.eclipse.jgit.lib.ObjectChecker.encoding; +import static org.eclipse.jgit.lib.ObjectChecker.object; +import static org.eclipse.jgit.lib.ObjectChecker.parent; +import static org.eclipse.jgit.lib.ObjectChecker.tag; import static org.eclipse.jgit.lib.ObjectChecker.tagger; +import static org.eclipse.jgit.lib.ObjectChecker.tree; +import static org.eclipse.jgit.lib.ObjectChecker.type; import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; @@ -520,17 +525,24 @@ public final class RawParseUtils { } /** - * Locate the end of the header. Note that headers may be - * more than one line long. + * Locate the first end of line after the given position, while treating + * following lines which are starting with spaces as part of the current + * line. + *

+ * For example, {@code nextLfSkippingSplitLines( + * "row \n with space at beginning of a following line\nThe actual next line", + * 0)} will return the position of {@code "\nThe actual next line"}. + * * @param b * buffer to scan. * @param ptr - * position within buffer to start looking for the end-of-header. - * @return new position just after the header. This is either - * b.length, or the index of the header's terminating newline. - * @since 5.1 + * position within buffer to start looking for the next line. + * @return new position just after the line end of the last line-split. This + * is either b.length, or the index of the current split-line's + * terminating newline. + * @since 6.9 */ - public static final int headerEnd(final byte[] b, int ptr) { + public static final int nextLfSkippingSplitLines(final byte[] b, int ptr) { final int sz = b.length; while (ptr < sz) { final byte c = b[ptr++]; @@ -541,6 +553,28 @@ public final class RawParseUtils { return ptr - 1; } + /** + * Locate the first end of header after the given position. Note that + * headers may be more than one line long. + *

+ * Also note that there might be multiple headers. If you wish to find the + * last header's end - call this in a loop. + * + * @param b + * buffer to scan. + * @param ptr + * position within buffer to start looking for the header + * (normally a new-line). + * @return new position just after the line end. This is either b.length, or + * the index of the header's terminating newline. + * @since 5.1 + * @deprecated use {{@link #nextLfSkippingSplitLines}} directly instead + */ + @Deprecated + public static final int headerEnd(final byte[] b, int ptr) { + return nextLfSkippingSplitLines(b, ptr); + } + /** * Find the start of the contents of a given header. * @@ -575,6 +609,21 @@ public final class RawParseUtils { return -1; } + /** + * Returns whether the message starts with any known headers. + * + * @param b + * buffer to scan. + * @return whether the message starts with any known headers + */ + public static final boolean hasAnyKnownHeaders(byte[] b) { + return match(b, 0, tree) != -1 || match(b, 0, parent) != -1 + || match(b, 0, author) != -1 || match(b, 0, committer) != -1 + || match(b, 0, encoding) != -1 || match(b, 0, object) != -1 + || match(b, 0, type) != -1 || match(b, 0, tag) != -1 + || match(b, 0, tagger) != -1; + } + /** * Locate the first position before a given character. * @@ -1258,6 +1307,7 @@ public final class RawParseUtils { final int sz = b.length; if (ptr == 0) ptr += 48; // skip the "object ..." line. + // Assume the rest of the current paragraph is all headers. while (ptr < sz && b[ptr] != '\n') ptr = nextLF(b, ptr); if (ptr < sz && b[ptr] == '\n')