diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java index f15ff36b2f..23839203a5 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java @@ -358,6 +358,43 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { assertDraftsUpToDate(true, id, user); } + @Test + public void pushCert() throws Exception { + // We don't have the code in our test harness to do signed pushes, so just + // use a hard-coded cert. This cert was actually generated by C git 2.2.0 + // (albeit not for sending to Gerrit). + String cert = "certificate version 0.1\n" + + "pusher Dave Borowitz 1433954361 -0700\n" + + "pushee git://localhost/repo.git\n" + + "nonce 1433954361-bde756572d665bba81d8\n" + + "\n" + + "0000000000000000000000000000000000000000" + + "b981a177396fb47345b7df3e4d3f854c6bea7" + + "s/heads/master\n" + + "-----BEGIN PGP SIGNATURE-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "iQEcBAABAgAGBQJVeGg5AAoJEPfTicJkUdPkUggH/RKAeI9/i/LduuiqrL/SSdIa\n" + + "9tYaSqJKLbXz63M/AW4Sp+4u+dVCQvnAt/a35CVEnpZz6hN4Kn/tiswOWVJf4CO7\n" + + "htNubGs5ZMwvD6sLYqKAnrM3WxV/2TbbjzjZW6Jkidz3jz/WRT4SmjGYiEO7aA+V\n" + + "4ZdIS9f7sW5VsHHYlNThCA7vH8Uu48bUovFXyQlPTX0pToSgrWV3JnTxDNxfn3iG\n" + + "IL0zTY/qwVCdXgFownLcs6J050xrrBWIKqfcWr3u4D2aCLyR0v+S/KArr7ulZygY\n" + + "+SOklImn8TAZiNxhWtA6ens66IiammUkZYFv7SSzoPLFZT4dC84SmGPWgf94NoQ=\n" + + "=XFeC\n" + + "-----END PGP SIGNATURE-----\n"; + + PushOneCommit.Result r = createChange(); + PatchSet.Id psId = r.getPatchSetId(); + Change.Id id = psId.getParentKey(); + + PatchSet ps = db.patchSets().get(psId); + ps.setPushCertificate(cert); + db.patchSets().update(Collections.singleton(ps)); + indexer.index(db, project, id); + + checker.rebuildAndCheckChanges(id); + } + private void setInvalidNoteDbState(Change.Id id) throws Exception { ReviewDb db = unwrapDb(); Change c = db.changes().get(id); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java index 459ba1fc04..e6332ea0a4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java @@ -22,6 +22,7 @@ import static com.google.gerrit.reviewdb.server.ReviewDbUtil.intKeyOrdering; import static com.google.gerrit.server.notedb.ChangeBundle.Source.NOTE_DB; import static com.google.gerrit.server.notedb.ChangeBundle.Source.REVIEW_DB; +import com.google.common.base.CharMatcher; import com.google.common.base.Joiner; import com.google.common.collect.ComparisonChain; import com.google.common.collect.ImmutableCollection; @@ -379,10 +380,20 @@ public class ChangeBundle { PatchSet a = as.get(id); PatchSet b = bs.get(id); String desc = describe(id); - diffColumns(diffs, PatchSet.class, desc, bundleA, a, bundleB, b); + String pushCertField = "pushCertificate"; + diffColumnsExcluding(diffs, PatchSet.class, desc, bundleA, a, bundleB, b, + pushCertField); + diffValues(diffs, desc, trimPushCert(a), trimPushCert(b), pushCertField); } } + private static String trimPushCert(PatchSet ps) { + if (ps.getPushCertificate() == null) { + return null; + } + return CharMatcher.is('\n').trimTrailingFrom(ps.getPushCertificate()); + } + private static void diffPatchSetApprovals(List diffs, ChangeBundle bundleA, ChangeBundle bundleB) { Map as = bundleA.patchSetApprovals; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java index 974538c77f..73ad68e0be 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RevisionNote.java @@ -37,7 +37,7 @@ class RevisionNote { "certificate version ".getBytes(UTF_8); // See org.eclipse.jgit.transport.PushCertificateParser.END_SIGNATURE private static final byte[] END_SIGNATURE = - "-----END PGP SIGNATURE-----".getBytes(UTF_8); + "-----END PGP SIGNATURE-----\n".getBytes(UTF_8); private static void trimLeadingEmptyLines(byte[] bytes, MutableInteger p) { while (p.value < bytes.length && bytes[p.value] == '\n') { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java index c4f5e19da2..ea12ecaffe 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java @@ -482,6 +482,34 @@ public class ChangeBundleTest { assertDiffs(b3, b1, msg); } + @Test + public void diffPatchSetsIgnoresTrailingNewlinesInPushCertificate() + throws Exception { + subWindowResolution(); + Change c = TestChanges.newChange(project, accountId); + PatchSet ps1 = new PatchSet(c.currentPatchSetId()); + ps1.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef")); + ps1.setUploader(accountId); + ps1.setCreatedOn(roundToSecond(TimeUtil.nowTs())); + ps1.setPushCertificate("some cert"); + PatchSet ps2 = clone(ps1); + ps2.setPushCertificate(ps2.getPushCertificate() + "\n\n"); + + ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps1), + approvals(), comments(), NOTE_DB); + ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps2), + approvals(), comments(), REVIEW_DB); + assertNoDiffs(b1, b2); + assertNoDiffs(b2, b1); + + b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(), + comments(), REVIEW_DB); + b2 = new ChangeBundle(c, messages(), patchSets(ps2), approvals(), + comments(), NOTE_DB); + assertNoDiffs(b1, b2); + assertNoDiffs(b2, b1); + } + @Test public void diffPatchSetApprovalKeySets() throws Exception { Change c = TestChanges.newChange(project, accountId); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index 4d80866275..a796f9c91b 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -23,7 +23,6 @@ import static com.google.gerrit.testutil.TestChanges.incrementPatchSet; import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; -import com.google.common.base.CharMatcher; import com.google.common.base.Function; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; @@ -900,7 +899,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { + "\n" + "Nor is this a real signature.\n" + "-----END PGP SIGNATURE-----\n"; - String trimmedCert = CharMatcher.is('\n').trimTrailingFrom(pushCert); // ps2 with push cert Change c = newChange(); @@ -917,8 +915,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { assertThat(readNote(notes, commit)).isEqualTo(pushCert); Map patchSets = notes.getPatchSets(); assertThat(patchSets.get(psId1).getPushCertificate()).isNull(); - assertThat(patchSets.get(psId2).getPushCertificate()) - .isEqualTo(trimmedCert); + assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert); assertThat(notes.getComments()).isEmpty(); // comment on ps2 @@ -946,8 +943,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { + "\n"); patchSets = notes.getPatchSets(); assertThat(patchSets.get(psId1).getPushCertificate()).isNull(); - assertThat(patchSets.get(psId2).getPushCertificate()) - .isEqualTo(trimmedCert); + assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert); assertThat(notes.getComments()).isNotEmpty(); }