diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableTest.java index db9a4d0ac..fc363ecf2 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableTest.java @@ -46,13 +46,16 @@ import static org.eclipse.jgit.lib.Constants.HEAD; import static org.eclipse.jgit.lib.Constants.OBJECT_ID_LENGTH; import static org.eclipse.jgit.lib.Constants.R_HEADS; +import static org.eclipse.jgit.lib.MoreAsserts.assertThrows; import static org.eclipse.jgit.lib.Ref.Storage.NEW; import static org.eclipse.jgit.lib.Ref.Storage.PACKED; +import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -414,6 +417,54 @@ public void noIndexSeek() throws IOException { assertSeek(refs, read(table)); } + @Test + public void invalidRefWriteOrder() throws IOException { + Ref master = ref(MASTER, 1); + Ref next = ref(NEXT, 2); + ReftableWriter writer = new ReftableWriter() + .setMinUpdateIndex(1) + .setMaxUpdateIndex(1) + .begin(new ByteArrayOutputStream()); + + writer.writeRef(next); + IllegalArgumentException e = assertThrows( + IllegalArgumentException.class, + () -> writer.writeRef(master)); + assertThat(e.getMessage(), containsString("records must be increasing")); + } + + @Test + public void invalidReflogWriteOrderUpdateIndex() throws IOException { + ReftableWriter writer = new ReftableWriter() + .setMinUpdateIndex(1) + .setMaxUpdateIndex(2) + .begin(new ByteArrayOutputStream()); + PersonIdent who = new PersonIdent("Log", "Ger", 1500079709, -8 * 60); + String msg = "test"; + + writer.writeLog(MASTER, 1, who, ObjectId.zeroId(), id(1), msg); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> writer.writeLog( + MASTER, 2, who, ObjectId.zeroId(), id(2), msg)); + assertThat(e.getMessage(), containsString("records must be increasing")); + } + + @Test + public void invalidReflogWriteOrderName() throws IOException { + ReftableWriter writer = new ReftableWriter() + .setMinUpdateIndex(1) + .setMaxUpdateIndex(1) + .begin(new ByteArrayOutputStream()); + PersonIdent who = new PersonIdent("Log", "Ger", 1500079709, -8 * 60); + String msg = "test"; + + writer.writeLog(NEXT, 1, who, ObjectId.zeroId(), id(1), msg); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> writer.writeLog( + MASTER, 1, who, ObjectId.zeroId(), id(2), msg)); + assertThat(e.getMessage(), containsString("records must be increasing")); + } + @Test public void withReflog() throws IOException { Ref master = ref(MASTER, 1); @@ -577,7 +628,7 @@ public void logScan() throws IOException { List refs = new ArrayList<>(); for (int i = 1; i <= 5670; i++) { - Ref ref = ref(String.format("refs/heads/%03d", i), i); + Ref ref = ref(String.format("refs/heads/%04d", i), i); refs.add(ref); writer.writeRef(ref); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableWriter.java index f8b9ffd17..726339617 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableWriter.java @@ -44,6 +44,7 @@ package org.eclipse.jgit.internal.storage.reftable; import static java.lang.Math.log; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jgit.internal.storage.reftable.BlockWriter.padBetweenBlocks; import static org.eclipse.jgit.internal.storage.reftable.ReftableConstants.FILE_FOOTER_LEN; import static org.eclipse.jgit.internal.storage.reftable.ReftableConstants.FILE_HEADER_LEN; @@ -108,6 +109,8 @@ public class ReftableWriter { private ReftableOutputStream out; private ObjectIdSubclassMap obj2ref; + private BlockWriter.Entry lastRef; + private BlockWriter.Entry lastLog; private BlockWriter cur; private Section refs; private Section objs; @@ -120,6 +123,8 @@ public class ReftableWriter { */ public ReftableWriter() { this(new ReftableConfig()); + lastRef = null; + lastLog = null; } /** @@ -269,10 +274,22 @@ public void writeRef(Ref ref, long updateIndex) throws IOException { throw new IllegalArgumentException(); } long d = updateIndex - minUpdateIndex; - long blockPos = refs.write(new RefEntry(ref, d)); + RefEntry entry = new RefEntry(ref, d); + if (lastRef != null && Entry.compare(lastRef, entry) >= 0) { + throwIllegalEntry(lastRef, entry); + } + lastRef = entry; + + long blockPos = refs.write(entry); indexRef(ref, blockPos); } + private void throwIllegalEntry(Entry last, Entry now) { + throw new IllegalArgumentException( + String.format("records must be increasing: last %s, this %s", + new String(last.key, UTF_8), new String(now.key, UTF_8))); + } + private void indexRef(Ref ref, long blockPos) { if (indexObjects && !ref.isSymbolic()) { indexId(ref.getObjectId(), blockPos); @@ -322,7 +339,12 @@ public void writeLog(String ref, long updateIndex, PersonIdent who, throws IOException { String msg = message != null ? message : ""; //$NON-NLS-1$ beginLog(); - logs.write(new LogEntry(ref, updateIndex, who, oldId, newId, msg)); + LogEntry entry = new LogEntry(ref, updateIndex, who, oldId, newId, msg); + if (lastLog != null && Entry.compare(lastLog, entry) >= 0) { + throwIllegalEntry(lastLog, entry); + } + lastLog = entry; + logs.write(entry); } /**