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 20fb12b3e..aaef45883 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 @@ -7,6 +7,7 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.theInstance; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -485,6 +486,8 @@ public class UploadPackTest { private LsRefsV2Request lsRefsRequest; + private FetchV2Request fetchRequest; + @Override public void onCapabilities(CapabilitiesV2Request req) { capabilitiesRequest = req; @@ -494,6 +497,11 @@ public class UploadPackTest { public void onLsRefs(LsRefsV2Request req) { lsRefsRequest = req; } + + @Override + public void onFetch(FetchV2Request req) { + fetchRequest = req; + } } @Test @@ -502,18 +510,18 @@ public class UploadPackTest { ByteArrayInputStream recvStream = uploadPackV2Setup(null, null, hook, PacketLineIn.END); PacketLineIn pckIn = new PacketLineIn(recvStream); - assertThat(hook.capabilitiesRequest, notNullValue()); assertThat(pckIn.readString(), is("version 2")); 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=shallow")); + Arrays.asList(pckIn.readString(), 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=shallow", "server-option")); assertTrue(pckIn.readString() == PacketLineIn.END); } @@ -526,10 +534,11 @@ public class UploadPackTest { assertThat(pckIn.readString(), is("version 2")); assertThat( - Arrays.asList(pckIn.readString(), pckIn.readString()), - // TODO(jonathantanmy) This check overspecifies the - // order of the capabilities of "fetch". - hasItems("ls-refs", "fetch=filter shallow")); + Arrays.asList(pckIn.readString(), pckIn.readString(), + pckIn.readString()), + // TODO(jonathantanmy) This check overspecifies the + // order of the capabilities of "fetch". + hasItems("ls-refs", "fetch=filter shallow", "server-option")); assertTrue(pckIn.readString() == PacketLineIn.END); } @@ -542,10 +551,12 @@ public class UploadPackTest { assertThat(pckIn.readString(), is("version 2")); assertThat( - Arrays.asList(pckIn.readString(), pckIn.readString()), - // TODO(jonathantanmy) This check overspecifies the - // order of the capabilities of "fetch". - hasItems("ls-refs", "fetch=ref-in-want shallow")); + Arrays.asList(pckIn.readString(), pckIn.readString(), + pckIn.readString()), + // TODO(jonathantanmy) This check overspecifies the + // order of the capabilities of "fetch". + hasItems("ls-refs", "fetch=ref-in-want shallow", + "server-option")); assertTrue(pckIn.readString() == PacketLineIn.END); } @@ -558,8 +569,9 @@ public class UploadPackTest { assertThat(pckIn.readString(), is("version 2")); assertThat( - Arrays.asList(pckIn.readString(), pckIn.readString()), - hasItems("ls-refs", "fetch=shallow")); + Arrays.asList(pckIn.readString(), pckIn.readString(), + pckIn.readString()), + hasItems("ls-refs", "fetch=shallow", "server-option")); assertTrue(pckIn.readString() == PacketLineIn.END); } @@ -573,8 +585,9 @@ public class UploadPackTest { assertThat(pckIn.readString(), is("version 2")); assertThat( - Arrays.asList(pckIn.readString(), pckIn.readString()), - hasItems("ls-refs", "fetch=shallow")); + Arrays.asList(pckIn.readString(), pckIn.readString(), + pckIn.readString()), + hasItems("ls-refs", "fetch=shallow", "server-option")); assertTrue(pckIn.readString() == PacketLineIn.END); } @@ -719,6 +732,21 @@ public class UploadPackTest { PacketLineIn.END); } + @Test + public void testV2LsRefsServerOptions() throws Exception { + String[] lines = { "command=ls-refs\n", + "server-option=one\n", "server-option=two\n", + PacketLineIn.DELIM, + PacketLineIn.END }; + + TestV2Hook testHook = new TestV2Hook(); + uploadPackV2Setup(null, null, testHook, lines); + + LsRefsV2Request req = testHook.lsRefsRequest; + assertEquals(2, req.getServerOptions().size()); + assertThat(req.getServerOptions(), hasItems("one", "two")); + } + /* * Parse multiplexed packfile output from upload-pack using protocol V2 * into the client repository. @@ -1255,6 +1283,21 @@ public class UploadPackTest { PacketLineIn.END); } + @Test + public void testV2FetchServerOptions() throws Exception { + String[] lines = { "command=fetch\n", "server-option=one\n", + "server-option=two\n", PacketLineIn.DELIM, + PacketLineIn.END }; + + TestV2Hook testHook = new TestV2Hook(); + uploadPackV2Setup(null, null, testHook, lines); + + FetchV2Request req = testHook.fetchRequest; + assertNotNull(req); + assertEquals(2, req.getServerOptions().size()); + assertThat(req.getServerOptions(), hasItems("one", "two")); + } + @Test public void testV2FetchFilter() throws Exception { RevBlob big = remote.blob("foobar"); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java index cd3906ea6..951d2d79e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java @@ -45,6 +45,7 @@ package org.eclipse.jgit.transport; import static java.util.Objects.requireNonNull; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -52,6 +53,7 @@ import java.util.Set; import java.util.TreeMap; import org.eclipse.jgit.annotations.NonNull; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.lib.ObjectId; /** @@ -69,18 +71,27 @@ public final class FetchV2Request extends FetchRequest { private final boolean doneReceived; + @Nullable + private final String agent; + + @NonNull + private final List serverOptions; + FetchV2Request(@NonNull List peerHas, @NonNull TreeMap wantedRefs, @NonNull Set wantIds, @NonNull Set clientShallowCommits, int deepenSince, @NonNull List deepenNotRefs, int depth, long filterBlobLimit, - boolean doneReceived, @NonNull Set clientCapabilities) { + boolean doneReceived, @NonNull Set clientCapabilities, + @Nullable String agent, @NonNull List serverOptions) { super(wantIds, depth, clientShallowCommits, filterBlobLimit, clientCapabilities, deepenSince, deepenNotRefs); this.peerHas = requireNonNull(peerHas); this.wantedRefs = requireNonNull(wantedRefs); this.doneReceived = doneReceived; + this.agent = agent; + this.serverOptions = requireNonNull(serverOptions); } /** @@ -106,6 +117,28 @@ public final class FetchV2Request extends FetchRequest { return doneReceived; } + /** + * @return string identifying the agent (as sent in the request body by the + * client) + */ + @Nullable + String getAgent() { + return agent; + } + + /** + * Options received in server-option lines. The caller can choose to act on + * these in an application-specific way + * + * @return Immutable list of server options received in the request + * + * @since 5.2 + */ + @NonNull + public List getServerOptions() { + return serverOptions; + } + /** @return A builder of {@link FetchV2Request}. */ static Builder builder() { return new Builder(); @@ -133,6 +166,11 @@ public final class FetchV2Request extends FetchRequest { boolean doneReceived; + @Nullable + String agent; + + final List serverOptions = new ArrayList<>(); + private Builder() { } @@ -264,13 +302,43 @@ public final class FetchV2Request extends FetchRequest { return this; } + /** + * Value of an agent line received after the command and before the + * arguments. E.g. "agent=a.b.c/1.0" should set "a.b.c/1.0". + * + * @param agentValue + * the client-supplied agent capability, without the leading + * "agent=" + * @return this builder + */ + Builder setAgent(@Nullable String agentValue) { + agent = agentValue; + return this; + } + + /** + * Records an application-specific option supplied in a server-option + * line, for later retrieval with + * {@link FetchV2Request#getServerOptions}. + * + * @param value + * the client-supplied server-option capability, without + * leading "server-option=". + * @return this builder + */ + Builder addServerOption(@NonNull String value) { + serverOptions.add(value); + return this; + } + /** * @return Initialized fetch request */ FetchV2Request build() { return new FetchV2Request(peerHas, wantedRefs, wantIds, clientShallowCommits, deepenSince, deepenNotRefs, - depth, filterBlobLimit, doneReceived, clientCapabilities); + depth, filterBlobLimit, doneReceived, clientCapabilities, + agent, Collections.unmodifiableList(serverOptions)); } } } 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 760ac6c1d..1561c93b9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java @@ -244,6 +244,20 @@ public final class GitProtocolConstants { */ public static final String CAPABILITY_REF_IN_WANT = "ref-in-want"; //$NON-NLS-1$ + /** + * The server supports arbitrary options + * + * @since 5.2 + */ + public static final String CAPABILITY_SERVER_OPTION = "server-option"; //$NON-NLS-1$ + + /** + * Option for passing application-specific options to the server. + * + * @since 5.2 + */ + public static final String OPTION_SERVER_OPTION = "server-option"; //$NON-NLS-1$ + /** * The server supports listing refs using protocol v2. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/LsRefsV2Request.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/LsRefsV2Request.java index 3aff584a0..add373147 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/LsRefsV2Request.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/LsRefsV2Request.java @@ -42,9 +42,15 @@ */ package org.eclipse.jgit.transport; +import static java.util.Objects.requireNonNull; + +import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.eclipse.jgit.annotations.NonNull; +import org.eclipse.jgit.annotations.Nullable; + /** * ls-refs protocol v2 request. * @@ -60,11 +66,20 @@ public final class LsRefsV2Request { private final boolean peel; + @Nullable + private final String agent; + + @NonNull + private final List serverOptions; + private LsRefsV2Request(List refPrefixes, boolean symrefs, - boolean peel) { + boolean peel, @Nullable String agent, + @NonNull List serverOptions) { this.refPrefixes = refPrefixes; this.symrefs = symrefs; this.peel = peel; + this.agent = agent; + this.serverOptions = requireNonNull(serverOptions); } /** @return ref prefixes that the client requested. */ @@ -82,6 +97,34 @@ public final class LsRefsV2Request { return peel; } + /** + * @return agent as reported by the client + * + * @since 5.2 + */ + @Nullable + public String getAgent() { + return agent; + } + + /** + * Get application-specific options provided by the client using + * --server-option. + *

+ * It returns just the content, without the "server-option=" prefix. E.g. a + * request with server-option=A and server-option=B lines returns the list + * [A, B]. + * + * @return application-specific options from the client as an unmodifiable + * list + * + * @since 5.2 + */ + @NonNull + public List getServerOptions() { + return serverOptions; + } + /** @return A builder of {@link LsRefsV2Request}. */ public static Builder builder() { return new Builder(); @@ -95,6 +138,10 @@ public final class LsRefsV2Request { private boolean peel; + private final List serverOptions = new ArrayList<>(); + + private String agent; + private Builder() { } @@ -125,10 +172,43 @@ public final class LsRefsV2Request { return this; } + /** + * Records an application-specific option supplied in a server-option + * line, for later retrieval with + * {@link LsRefsV2Request#getServerOptions}. + * + * @param value + * the client-supplied server-option capability, without + * leading "server-option=". + * @return this builder + * @since 5.2 + */ + public Builder addServerOption(@NonNull String value) { + serverOptions.add(value); + return this; + } + + /** + * Value of an agent line received after the command and before the + * arguments. E.g. "agent=a.b.c/1.0" should set "a.b.c/1.0". + * + * @param value + * the client-supplied agent capability, without leading + * "agent=" + * @return this builder + * + * @since 5.2 + */ + public Builder setAgent(@Nullable String value) { + agent = value; + return this; + } + /** @return LsRefsV2Request */ public LsRefsV2Request build() { return new LsRefsV2Request( - Collections.unmodifiableList(refPrefixes), symrefs, peel); + Collections.unmodifiableList(refPrefixes), symrefs, peel, + agent, Collections.unmodifiableList(serverOptions)); } } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java index 71b5773a4..a03f02146 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java @@ -44,9 +44,11 @@ package org.eclipse.jgit.transport; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_DEEPEN_RELATIVE; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_FILTER; +import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_AGENT; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_INCLUDE_TAG; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_NO_PROGRESS; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_OFS_DELTA; +import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SERVER_OPTION; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_THIN_PACK; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WANT_REF; @@ -55,6 +57,7 @@ import java.io.IOException; import java.text.MessageFormat; import java.util.ArrayList; import java.util.List; +import java.util.function.Consumer; import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.internal.JGitText; @@ -77,6 +80,35 @@ final class ProtocolV2Parser { this.transferConfig = transferConfig; } + /* + * Read lines until DELIM or END, calling the appropiate consumer. + * + * Returns the last read line (so caller can check if there is more to read + * in the line). + */ + private static String consumeCapabilities(PacketLineIn pckIn, + Consumer serverOptionConsumer, + Consumer agentConsumer) throws IOException { + + String serverOptionPrefix = OPTION_SERVER_OPTION + '='; + String agentPrefix = OPTION_AGENT + '='; + + String line = pckIn.readString(); + while (line != PacketLineIn.DELIM && line != PacketLineIn.END) { + if (line.startsWith(serverOptionPrefix)) { + serverOptionConsumer + .accept(line.substring(serverOptionPrefix.length())); + } else if (line.startsWith(agentPrefix)) { + agentConsumer.accept(line.substring(agentPrefix.length())); + } else { + // Unrecognized capability. Ignore it. + } + line = pckIn.readString(); + } + + return line; + } + /** * Parse the incoming fetch request arguments from the wire. The caller must * be sure that what is comings is a fetch request before coming here. @@ -106,13 +138,18 @@ final class ProtocolV2Parser { // lengths. reqBuilder.addClientCapability(OPTION_SIDE_BAND_64K); - String line; + String line = consumeCapabilities(pckIn, + serverOption -> reqBuilder.addServerOption(serverOption), + agent -> reqBuilder.setAgent(agent)); - // Currently, we do not support any capabilities, so the next - // line is DELIM. - if ((line = pckIn.readString()) != PacketLineIn.DELIM) { - throw new PackProtocolException(MessageFormat - .format(JGitText.get().unexpectedPacketLine, line)); + if (line == PacketLineIn.END) { + return reqBuilder.build(); + } + + if (line != PacketLineIn.DELIM) { + throw new PackProtocolException( + MessageFormat.format(JGitText.get().unexpectedPacketLine, + line)); } boolean filterReceived = false; @@ -226,27 +263,33 @@ final class ProtocolV2Parser { throws PackProtocolException, IOException { LsRefsV2Request.Builder builder = LsRefsV2Request.builder(); List prefixes = new ArrayList<>(); - String line = pckIn.readString(); - // Currently, we do not support any capabilities, so the next - // line is DELIM if there are arguments or END if not. - if (line == PacketLineIn.DELIM) { - while ((line = pckIn.readString()) != PacketLineIn.END) { - if (line.equals("peel")) { //$NON-NLS-1$ - builder.setPeel(true); - } else if (line.equals("symrefs")) { //$NON-NLS-1$ - builder.setSymrefs(true); - } else if (line.startsWith("ref-prefix ")) { //$NON-NLS-1$ - prefixes.add(line.substring("ref-prefix ".length())); //$NON-NLS-1$ - } else { - throw new PackProtocolException(MessageFormat - .format(JGitText.get().unexpectedPacketLine, line)); - } - } - } else if (line != PacketLineIn.END) { + + String line = consumeCapabilities(pckIn, + serverOption -> builder.addServerOption(serverOption), + agent -> builder.setAgent(agent)); + + if (line == PacketLineIn.END) { + return builder.build(); + } + + if (line != PacketLineIn.DELIM) { throw new PackProtocolException(MessageFormat .format(JGitText.get().unexpectedPacketLine, line)); } + while ((line = pckIn.readString()) != PacketLineIn.END) { + if (line.equals("peel")) { //$NON-NLS-1$ + builder.setPeel(true); + } else if (line.equals("symrefs")) { //$NON-NLS-1$ + builder.setSymrefs(true); + } else if (line.startsWith("ref-prefix ")) { //$NON-NLS-1$ + prefixes.add(line.substring("ref-prefix ".length())); //$NON-NLS-1$ + } else { + throw new PackProtocolException(MessageFormat + .format(JGitText.get().unexpectedPacketLine, line)); + } + } + return builder.setRefPrefixes(prefixes).build(); } 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 58cd56411..535c914fc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -48,6 +48,7 @@ import static org.eclipse.jgit.lib.RefDatabase.ALL; import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REF_IN_WANT; import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_FETCH; import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_LS_REFS; +import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SERVER_OPTION; 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; @@ -1082,6 +1083,7 @@ public class UploadPack { (transferConfig.isAllowFilter() ? OPTION_FILTER + ' ' : "") + //$NON-NLS-1$ (advertiseRefInWant ? CAPABILITY_REF_IN_WANT + ' ' : "") + //$NON-NLS-1$ OPTION_SHALLOW); + caps.add(CAPABILITY_SERVER_OPTION); return caps; }