Revert "Improve footer parsing to allow multiline footers."

This reverts commit 340cc787a0d14a0698d757a919ccd77e0a129fb0.

This breaks a test in the git_numberer gerrit plugin used by chromium
[1].  The test checks that first line is never a footer, which sounds
right. That test should be included in FooterLineTest.

[1]
https://chromium.googlesource.com/infra/gerrit-plugins/git-numberer/+/refs/heads/main/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GetFooterLinesTest.java

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 303aedcd0087e0c7b55a5d8e0d222139b118e534..01f6a3a0a012c9960b395d1b01dd6b1ab701db64 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java
@@ -318,57 +318,6 @@ public class FooterLineTest extends RepositoryTestCase {
                assertFalse("not CC", line.matches(FooterKey.CC));
        }
 
-       @Test
-       public void testMultilineFooters() {
-               String msg = buildMessage("subject\n\nbody of commit\n"
-                               + "Not-A-Footer-Line: this line must not be read as a footer\n"
-                               + "\n" // paragraph break, now footers appear in final block
-                               + "Notes: The change must not be merged until dependency ABC is\n"
-                               + " updated.\n"
-                               + "CC:            <some.mailing.list@example.com>\n"
-                               + "not really a footer line but we'll skip it anyway\n"
-                               + "Acked-by: Some Reviewer <sr@example.com>\n");
-               List<FooterLine> footers = FooterLine.fromMessage(msg);
-               FooterLine f;
-
-               assertNotNull(footers);
-               assertEquals(3, footers.size());
-
-               f = footers.get(0);
-               assertEquals("Notes", f.getKey());
-               assertEquals(
-                               "The change must not be merged until dependency ABC is updated.",
-                               f.getValue());
-
-               f = footers.get(1);
-               assertEquals("CC", f.getKey());
-               assertEquals("<some.mailing.list@example.com>", f.getValue());
-
-               f = footers.get(2);
-               assertEquals("Acked-by", f.getKey());
-               assertEquals("Some Reviewer <sr@example.com>", f.getValue());
-       }
-
-       @Test
-       public void testMultilineFooters_multipleWhitespaceAreAllowed() {
-               String msg = buildMessage("subject\n\nbody of commit\n"
-                               + "Not-A-Footer-Line: this line must not be read as a footer\n"
-                               + "\n" // paragraph break, now footers appear in final block
-                               + "Notes: The change must not be merged until dependency ABC is\n"
-                               + "    updated.\n");
-               List<FooterLine> footers = FooterLine.fromMessage(msg);
-               FooterLine f;
-
-               assertNotNull(footers);
-               assertEquals(1, footers.size());
-
-               f = footers.get(0);
-               assertEquals("Notes", f.getKey());
-               assertEquals(
-                               "The change must not be merged until dependency ABC is updated.",
-                               f.getValue());
-       }
-
        private String buildMessage(String msg) {
                StringBuilder buf = new StringBuilder();
                buf.append("tree " + ObjectId.zeroId().name() + "\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 cadeaec6e0a77180abf4c044aea42032b424d10f..2d396d57c50ff5d2660ae04ef0fafa9aa2df5557 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FooterLine.java
@@ -74,42 +74,41 @@ public final class FooterLine {
         */
        public static List<FooterLine> fromMessage(
                        byte[] raw) {
-               // Find the end of the last paragraph.
-               int parEnd = raw.length;
-               for (; parEnd > 0 && (raw[parEnd - 1] == '\n' || raw[parEnd - 1] == ' '); --parEnd);
+               int ptr = raw.length - 1;
+               while (raw[ptr] == '\n') // trim any trailing LFs, not interesting
+                       ptr--;
 
                int msgB = RawParseUtils.commitMessage(raw, 0);
                ArrayList<FooterLine> r = new ArrayList<>(4);
                Charset enc = RawParseUtils.guessEncoding(raw);
+               for (;;) {
+                       ptr = RawParseUtils.prevLF(raw, ptr);
+                       if (ptr <= msgB)
+                               break; // Don't parse commit headers as footer lines.
 
-               // Search for the beginning of last paragraph
-               int parStart = parEnd;
-               for (; parStart > msgB && (raw[parStart - 1] != '\n' || raw[parStart - 2] != '\n'); --parStart);
+                       int keyStart = ptr + 2;
+                       if (raw[keyStart] == '\n')
+                               break; // Stop at first paragraph break, no footers above it.
 
-               for (int ptr = parStart; ptr < parEnd;) {
-                       int keyStart = ptr;
-                       int keyEnd = RawParseUtils.endOfFooterLineKey(raw, ptr);
-                       if (keyEnd < 0) {
-                               // Not a well-formed footer line, skip it.
-                               ptr = RawParseUtils.nextLF(raw, ptr);
-                               continue;
-                       }
+                       int keyEnd = RawParseUtils.endOfFooterLineKey(raw, keyStart);
+                       if (keyEnd < 0)
+                               continue; // Not a well formed footer line, skip it.
 
                        // Skip over the ': *' at the end of the key before the value.
-                       int valStart, valEnd;
-                       for (valStart = keyEnd + 1; valStart < raw.length && raw[valStart] == ' '; ++valStart);
-
-                       for(ptr = valStart;;) {
-                               ptr = RawParseUtils.nextLF(raw, ptr);
-                               // Next line starts with whitespace for a multiline footer.
-                               if (ptr == raw.length || raw[ptr] != ' ') {
-                                       valEnd = raw[ptr - 1] == '\n' ? ptr - 1 : ptr;
-                                       break;
-                               }
-                       }
+                       //
+                       int valStart = keyEnd + 1;
+                       while (valStart < raw.length && raw[valStart] == ' ')
+                               valStart++;
+
+                       // Value ends at the LF, and does not include it.
+                       //
+                       int valEnd = RawParseUtils.nextLF(raw, valStart);
+                       if (raw[valEnd - 1] == '\n')
+                               valEnd--;
+
                        r.add(new FooterLine(raw, enc, keyStart, keyEnd, valStart, valEnd));
                }
-
+               Collections.reverse(r);
                return r;
        }
 
@@ -200,7 +199,7 @@ public final class FooterLine {
         *         character encoding.
         */
        public String getValue() {
-               return RawParseUtils.decode(enc, buffer, valStart, valEnd).replaceAll("\n +", " ");
+               return RawParseUtils.decode(enc, buffer, valStart, valEnd);
        }
 
        /**

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
index 1f0f13152eaa354100ba70752ff9db2ca3d65746..0392ea428cdae2d639bf40d9ea6c34c623b49644 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
@@ -570,9 +570,8 @@ public class RevCommit extends RevObject {
         * the order of the line's appearance in the commit message itself.
         * <p>
         * A footer line's key must match the pattern {@code ^[A-Za-z0-9-]+:}, while
-        * the value is free-form. The Value may be split over multiple lines with
-        * each subsequent line starting with at least one whitespace. Very common
-        * keys seen in the wild are:
+        * the value is free-form, but must not contain an LF. Very common keys seen
+        * in the wild are:
         * <ul>
         * <li>{@code Signed-off-by} (agrees to Developer Certificate of Origin)
         * <li>{@code Acked-by} (thinks change looks sane in context)