diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java index a50f170ff..69d3af53f 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java @@ -1,19 +1,26 @@ package org.eclipse.jgit.transport; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.theInstance; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.StringWriter; import org.eclipse.jgit.errors.TransportException; import org.eclipse.jgit.internal.storage.dfs.DfsGarbageCollector; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Sets; import org.eclipse.jgit.revwalk.RevBlob; @@ -24,6 +31,7 @@ import org.eclipse.jgit.transport.UploadPack.RequestPolicy; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.eclipse.jgit.transport.resolver.UploadPackFactory; +import org.eclipse.jgit.util.io.NullOutputStream; import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; @@ -344,7 +352,8 @@ public class UploadPackTest { * Returns UploadPack's output stream, not including the capability * advertisement by the server. */ - private ByteArrayInputStream uploadPackV2(String... inputLines) throws Exception { + private ByteArrayInputStream uploadPackV2(RequestPolicy requestPolicy, + RefFilter refFilter, String... inputLines) throws Exception { ByteArrayOutputStream send = new ByteArrayOutputStream(); PacketLineOut pckOut = new PacketLineOut(send); for (String line : inputLines) { @@ -359,6 +368,10 @@ public class UploadPackTest { server.getConfig().setString("protocol", null, "version", "2"); UploadPack up = new UploadPack(server); + if (requestPolicy != null) + up.setRequestPolicy(requestPolicy); + if (refFilter != null) + up.setRefFilter(refFilter); up.setExtraParameters(Sets.of("version=2")); ByteArrayOutputStream recv = new ByteArrayOutputStream(); @@ -369,11 +382,23 @@ public class UploadPackTest { // capability advertisement (always sent) assertThat(pckIn.readString(), is("version 2")); - assertThat(pckIn.readString(), is("ls-refs")); + assertThat( + Arrays.asList(pckIn.readString(), pckIn.readString()), + // TODO(jonathantanmy) This check is written this way + // to make it simple to see that we expect this list of + // capabilities, but probably should be loosened to + // allow additional commands to be added to the list, + // and additional capabilities to be added to existing + // commands without requiring test changes. + hasItems("ls-refs", "fetch")); assertTrue(pckIn.readString() == PacketLineIn.END); return recvStream; } + private ByteArrayInputStream uploadPackV2(String... inputLines) throws Exception { + return uploadPackV2(null, null, inputLines); + } + @Test public void testV2EmptyRequest() throws Exception { ByteArrayInputStream recvStream = uploadPackV2(PacketLineIn.END); @@ -500,4 +525,235 @@ public class UploadPackTest { assertThat(pckIn.readString(), is(tip.toObjectId().getName() + " refs/heads/other")); assertTrue(pckIn.readString() == PacketLineIn.END); } + + /* + * Parse multiplexed packfile output from upload-pack using protocol V2 + * into the client repository. + */ + private void parsePack(ByteArrayInputStream recvStream) throws Exception { + SideBandInputStream sb = new SideBandInputStream( + recvStream, NullProgressMonitor.INSTANCE, + new StringWriter(), NullOutputStream.INSTANCE); + client.newObjectInserter().newPackParser(sb).parse(NullProgressMonitor.INSTANCE); + } + + @Test + public void testV2FetchRequestPolicyAdvertised() throws Exception { + RevCommit advertized = remote.commit().message("x").create(); + RevCommit unadvertized = remote.commit().message("y").create(); + remote.update("branch1", advertized); + + // This works + uploadPackV2( + RequestPolicy.ADVERTISED, + null, + "command=fetch\n", + PacketLineIn.DELIM, + "want " + advertized.name() + "\n", + PacketLineIn.END); + + // This doesn't + thrown.expect(TransportException.class); + thrown.expectMessage(Matchers.containsString( + "want " + unadvertized.name() + " not valid")); + uploadPackV2( + RequestPolicy.ADVERTISED, + null, + "command=fetch\n", + PacketLineIn.DELIM, + "want " + unadvertized.name() + "\n", + PacketLineIn.END); + } + + @Test + public void testV2FetchRequestPolicyReachableCommit() throws Exception { + RevCommit reachable = remote.commit().message("x").create(); + RevCommit advertized = remote.commit().message("x").parent(reachable).create(); + RevCommit unreachable = remote.commit().message("y").create(); + remote.update("branch1", advertized); + + // This works + uploadPackV2( + RequestPolicy.REACHABLE_COMMIT, + null, + "command=fetch\n", + PacketLineIn.DELIM, + "want " + reachable.name() + "\n", + PacketLineIn.END); + + // This doesn't + thrown.expect(TransportException.class); + thrown.expectMessage(Matchers.containsString( + "want " + unreachable.name() + " not valid")); + uploadPackV2( + RequestPolicy.REACHABLE_COMMIT, + null, + "command=fetch\n", + PacketLineIn.DELIM, + "want " + unreachable.name() + "\n", + PacketLineIn.END); + } + + @Test + public void testV2FetchRequestPolicyTip() throws Exception { + RevCommit parentOfTip = remote.commit().message("x").create(); + RevCommit tip = remote.commit().message("y").parent(parentOfTip).create(); + remote.update("secret", tip); + + // This works + uploadPackV2( + RequestPolicy.TIP, + new RejectAllRefFilter(), + "command=fetch\n", + PacketLineIn.DELIM, + "want " + tip.name() + "\n", + PacketLineIn.END); + + // This doesn't + thrown.expect(TransportException.class); + thrown.expectMessage(Matchers.containsString( + "want " + parentOfTip.name() + " not valid")); + uploadPackV2( + RequestPolicy.TIP, + new RejectAllRefFilter(), + "command=fetch\n", + PacketLineIn.DELIM, + "want " + parentOfTip.name() + "\n", + PacketLineIn.END); + } + + @Test + public void testV2FetchRequestPolicyReachableCommitTip() throws Exception { + RevCommit parentOfTip = remote.commit().message("x").create(); + RevCommit tip = remote.commit().message("y").parent(parentOfTip).create(); + RevCommit unreachable = remote.commit().message("y").create(); + remote.update("secret", tip); + + // This works + uploadPackV2( + RequestPolicy.REACHABLE_COMMIT_TIP, + new RejectAllRefFilter(), + "command=fetch\n", + PacketLineIn.DELIM, + "want " + parentOfTip.name() + "\n", + PacketLineIn.END); + + // This doesn't + thrown.expect(TransportException.class); + thrown.expectMessage(Matchers.containsString( + "want " + unreachable.name() + " not valid")); + uploadPackV2( + RequestPolicy.REACHABLE_COMMIT_TIP, + new RejectAllRefFilter(), + "command=fetch\n", + PacketLineIn.DELIM, + "want " + unreachable.name() + "\n", + PacketLineIn.END); + } + + @Test + public void testV2FetchRequestPolicyAny() throws Exception { + RevCommit unreachable = remote.commit().message("y").create(); + + // Exercise to make sure that even unreachable commits can be fetched + uploadPackV2( + RequestPolicy.ANY, + null, + "command=fetch\n", + PacketLineIn.DELIM, + "want " + unreachable.name() + "\n", + PacketLineIn.END); + } + + @Test + public void testV2FetchServerDoesNotStopNegotiation() throws Exception { + RevCommit fooParent = remote.commit().message("x").create(); + RevCommit fooChild = remote.commit().message("x").parent(fooParent).create(); + RevCommit barParent = remote.commit().message("y").create(); + RevCommit barChild = remote.commit().message("y").parent(barParent).create(); + remote.update("branch1", fooChild); + remote.update("branch2", barChild); + + ByteArrayInputStream recvStream = uploadPackV2( + "command=fetch\n", + PacketLineIn.DELIM, + "want " + fooChild.toObjectId().getName() + "\n", + "want " + barChild.toObjectId().getName() + "\n", + "have " + fooParent.toObjectId().getName() + "\n", + PacketLineIn.END); + PacketLineIn pckIn = new PacketLineIn(recvStream); + + assertThat(pckIn.readString(), is("acknowledgments")); + assertThat(pckIn.readString(), is("ACK " + fooParent.toObjectId().getName())); + assertThat(pckIn.readString(), theInstance(PacketLineIn.END)); + } + + @Test + public void testV2FetchServerStopsNegotiation() throws Exception { + RevCommit fooParent = remote.commit().message("x").create(); + RevCommit fooChild = remote.commit().message("x").parent(fooParent).create(); + RevCommit barParent = remote.commit().message("y").create(); + RevCommit barChild = remote.commit().message("y").parent(barParent).create(); + remote.update("branch1", fooChild); + remote.update("branch2", barChild); + + ByteArrayInputStream recvStream = uploadPackV2( + "command=fetch\n", + PacketLineIn.DELIM, + "want " + fooChild.toObjectId().getName() + "\n", + "want " + barChild.toObjectId().getName() + "\n", + "have " + fooParent.toObjectId().getName() + "\n", + "have " + barParent.toObjectId().getName() + "\n", + PacketLineIn.END); + PacketLineIn pckIn = new PacketLineIn(recvStream); + + assertThat(pckIn.readString(), is("acknowledgments")); + assertThat( + Arrays.asList(pckIn.readString(), pckIn.readString()), + hasItems( + "ACK " + fooParent.toObjectId().getName(), + "ACK " + barParent.toObjectId().getName())); + assertThat(pckIn.readString(), is("ready")); + assertThat(pckIn.readString(), theInstance(PacketLineIn.DELIM)); + assertThat(pckIn.readString(), is("packfile")); + parsePack(recvStream); + assertFalse(client.hasObject(fooParent.toObjectId())); + assertTrue(client.hasObject(fooChild.toObjectId())); + assertFalse(client.hasObject(barParent.toObjectId())); + assertTrue(client.hasObject(barChild.toObjectId())); + } + + @Test + public void testV2FetchClientStopsNegotiation() throws Exception { + RevCommit fooParent = remote.commit().message("x").create(); + RevCommit fooChild = remote.commit().message("x").parent(fooParent).create(); + RevCommit barParent = remote.commit().message("y").create(); + RevCommit barChild = remote.commit().message("y").parent(barParent).create(); + remote.update("branch1", fooChild); + remote.update("branch2", barChild); + + ByteArrayInputStream recvStream = uploadPackV2( + "command=fetch\n", + PacketLineIn.DELIM, + "want " + fooChild.toObjectId().getName() + "\n", + "want " + barChild.toObjectId().getName() + "\n", + "have " + fooParent.toObjectId().getName() + "\n", + "done\n", + PacketLineIn.END); + PacketLineIn pckIn = new PacketLineIn(recvStream); + + assertThat(pckIn.readString(), is("packfile")); + parsePack(recvStream); + assertFalse(client.hasObject(fooParent.toObjectId())); + assertTrue(client.hasObject(fooChild.toObjectId())); + assertTrue(client.hasObject(barParent.toObjectId())); + assertTrue(client.hasObject(barChild.toObjectId())); + } + + private static class RejectAllRefFilter implements RefFilter { + @Override + public Map filter(Map refs) { + return new HashMap(); + } + }; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java index ccefb5168..572549e97 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java @@ -229,6 +229,13 @@ public class GitProtocolConstants { */ public static final String COMMAND_LS_REFS = "ls-refs"; //$NON-NLS-1$ + /** + * The server supports fetch using protocol v2. + * + * @since 5.0 + */ + public static final String COMMAND_FETCH = "fetch"; //$NON-NLS-1$ + static enum MultiAck { OFF, CONTINUE, DETAILED; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 355b1b9df..931062c0b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -45,6 +45,7 @@ package org.eclipse.jgit.transport; import static org.eclipse.jgit.lib.RefDatabase.ALL; import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_LS_REFS; +import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_FETCH; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_AGENT; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_ALLOW_REACHABLE_SHA1_IN_WANT; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_ALLOW_TIP_SHA1_IN_WANT; @@ -119,7 +120,8 @@ public class UploadPack { // supports protocol version 2. private static final String[] v2CapabilityAdvertisement = { "version 2", - COMMAND_LS_REFS + COMMAND_LS_REFS, + COMMAND_FETCH }; /** Policy the server uses to validate client requests */ @@ -916,6 +918,74 @@ public class UploadPack { adv.end(); } + private void fetchV2() throws IOException { + options = new HashSet<>(); + + // Packs are always sent multiplexed and using full 64K + // lengths. + options.add(OPTION_SIDE_BAND_64K); + + // Depending on the requestValidator, #processHaveLines may + // require that advertised be set. Set it only in the required + // circumstances (to avoid a full ref lookup in the case that + // we don't need it). + if (requestValidator instanceof TipRequestValidator || + requestValidator instanceof ReachableCommitTipRequestValidator || + requestValidator instanceof AnyRequestValidator) { + advertised = Collections.emptySet(); + } else { + advertised = refIdSet(getAdvertisedOrDefaultRefs().values()); + } + + String line; + List peerHas = new ArrayList<>(); + boolean doneReceived = false; + + // Currently, we do not support any capabilities, so the next + // line is DELIM. + if ((line = pckIn.readString()) != PacketLineIn.DELIM) { + throw new PackProtocolException("unexpected " + line); + } + + while ((line = pckIn.readString()) != PacketLineIn.END) { + if (line.startsWith("want ")) { + wantIds.add(ObjectId.fromString(line.substring(5))); + } else if (line.startsWith("have ")) { + peerHas.add(ObjectId.fromString(line.substring(5))); + } else if (line.equals("done")) { + doneReceived = true; + } + // else ignore it + } + rawOut.stopBuffering(); + + boolean sectionSent = false; + if (doneReceived) { + processHaveLines(peerHas, ObjectId.zeroId(), new PacketLineOut(NullOutputStream.INSTANCE)); + } else { + pckOut.writeString("acknowledgments\n"); + for (ObjectId id : peerHas) { + if (walk.getObjectReader().has(id)) { + pckOut.writeString("ACK " + id.getName() + "\n"); + } + } + processHaveLines(peerHas, ObjectId.zeroId(), new PacketLineOut(NullOutputStream.INSTANCE)); + if (okToGiveUp()) { + pckOut.writeString("ready\n"); + } else if (commonBase.isEmpty()) { + pckOut.writeString("NAK\n"); + } + sectionSent = true; + } + if (doneReceived || okToGiveUp()) { + if (sectionSent) + pckOut.writeDelim(); + pckOut.writeString("packfile\n"); + sendPack(new PackStatistics.Accumulator()); + } + pckOut.end(); + } + /* * Returns true if this is the last command and we should tear down the * connection. @@ -938,6 +1008,10 @@ public class UploadPack { lsRefsV2(); return false; } + if (command.equals("command=" + COMMAND_FETCH)) { + fetchV2(); + return false; + } throw new PackProtocolException("unknown command " + command); } @@ -1281,7 +1355,7 @@ public class UploadPack { } if (line == PacketLineIn.END) { - last = processHaveLines(peerHas, last); + last = processHaveLines(peerHas, last, pckOut); if (commonBase.isEmpty() || multiAck != MultiAck.OFF) pckOut.writeString("NAK\n"); //$NON-NLS-1$ if (noDone && sentReady) { @@ -1296,7 +1370,7 @@ public class UploadPack { peerHas.add(ObjectId.fromString(line.substring(5))); accumulator.haves++; } else if (line.equals("done")) { //$NON-NLS-1$ - last = processHaveLines(peerHas, last); + last = processHaveLines(peerHas, last, pckOut); if (commonBase.isEmpty()) pckOut.writeString("NAK\n"); //$NON-NLS-1$ @@ -1312,7 +1386,7 @@ public class UploadPack { } } - private ObjectId processHaveLines(List peerHas, ObjectId last) + private ObjectId processHaveLines(List peerHas, ObjectId last, PacketLineOut out) throws IOException { preUploadHook.onBeginNegotiateRound(this, wantIds, peerHas.size()); if (wantAll.isEmpty() && !wantIds.isEmpty()) @@ -1357,13 +1431,13 @@ public class UploadPack { switch (multiAck) { case OFF: if (commonBase.size() == 1) - pckOut.writeString("ACK " + obj.name() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ + out.writeString("ACK " + obj.name() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ break; case CONTINUE: - pckOut.writeString("ACK " + obj.name() + " continue\n"); //$NON-NLS-1$ //$NON-NLS-2$ + out.writeString("ACK " + obj.name() + " continue\n"); //$NON-NLS-1$ //$NON-NLS-2$ break; case DETAILED: - pckOut.writeString("ACK " + obj.name() + " common\n"); //$NON-NLS-1$ //$NON-NLS-2$ + out.writeString("ACK " + obj.name() + " common\n"); //$NON-NLS-1$ //$NON-NLS-2$ break; } } @@ -1389,10 +1463,10 @@ public class UploadPack { case OFF: break; case CONTINUE: - pckOut.writeString("ACK " + id.name() + " continue\n"); //$NON-NLS-1$ //$NON-NLS-2$ + out.writeString("ACK " + id.name() + " continue\n"); //$NON-NLS-1$ //$NON-NLS-2$ break; case DETAILED: - pckOut.writeString("ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$ + out.writeString("ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$ sentReady = true; break; } @@ -1404,7 +1478,7 @@ public class UploadPack { if (multiAck == MultiAck.DETAILED && !didOkToGiveUp && okToGiveUp()) { ObjectId id = peerHas.get(peerHas.size() - 1); - pckOut.writeString("ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$ + out.writeString("ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$ sentReady = true; }