From adc73c4ba1c5a0ddfaee9537df438bfa14f38a62 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Fri, 23 Feb 2018 14:07:02 -0800 Subject: [PATCH] Teach UploadPack basic "fetch" command Add basic support for the "fetch" command in the fetch-pack/upload-pack protocol v2. This patch teaches "have" and "done". The protocol specification (Documentation/technical/protocol-v2.txt in the Git project) states: want Indicates to the server an object which the client wants to retrieve. Wants can be anything and are not limited to advertised objects. It is unspecified whether the server should respect the uploadpack.allowtipsha1inwant option etc. when serving packfiles. This patch is conservative in that the server respects them. Change-Id: I3dbec172239712ef9286a15b8407e86b87ea7863 Signed-off-by: Jonathan Tan --- .../jgit/transport/UploadPackTest.java | 260 +++++++++++++++++- .../jgit/transport/GitProtocolConstants.java | 7 + .../eclipse/jgit/transport/UploadPack.java | 94 ++++++- 3 files changed, 349 insertions(+), 12 deletions(-) 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.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 @@ private static ByteArrayInputStream send(String... lines) throws Exception { * 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 @@ private ByteArrayInputStream uploadPackV2(String... inputLines) throws Exception 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 @@ private ByteArrayInputStream uploadPackV2(String... inputLines) throws Exception // 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 void testV2LsRefsRefPrefixNoSlash() throws Exception { 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 @@ 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 @@ private void lsRefsV2() throws IOException { 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 @@ private boolean serveOneCommandV2() throws IOException { lsRefsV2(); return false; } + if (command.equals("command=" + COMMAND_FETCH)) { + fetchV2(); + return false; + } throw new PackProtocolException("unknown command " + command); } @@ -1281,7 +1355,7 @@ private boolean negotiate(PackStatistics.Accumulator accumulator) } 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 @@ private boolean negotiate(PackStatistics.Accumulator accumulator) 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 @@ else if (multiAck != MultiAck.OFF) } } - 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 @@ private ObjectId processHaveLines(List peerHas, ObjectId last) 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 @@ private ObjectId processHaveLines(List peerHas, ObjectId last) 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 @@ private ObjectId processHaveLines(List peerHas, ObjectId last) 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; }