Browse Source

UploadPack: Return correct peer user agent on v2 requests

UploadPack.getPeerUserAgent() doesn't produce the expected results for
protocol v2 requests. In v2, the agent reported in the request (in an
"agent=" line) is not in the clientCapabilities but in a field on its
own. This makes getPeerUserAgent default to the transport user agent.

Making "agent" a shared property between protocol v0/v1 and v2 fixes the
problem, simplifies the function and harmonizes the implementation
between protocol versions.

In a follow up commit the "agent" will be identified on parsing time,
instead of taking it from the client capabilities.

Change-Id: Idf9825ec4e0b81a1458c8e3701f3e28aafd8a32a
Signed-off-by: Ivan Frade <ifrade@google.com>
stable-5.2
Ivan Frade 6 years ago
parent
commit
8d4f8d55d3
  1. 70
      org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
  2. 24
      org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchRequest.java
  3. 29
      org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java
  4. 15
      org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java
  5. 7
      org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

70
org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java

@ -429,17 +429,7 @@ public class UploadPackTest {
RefFilter refFilter, ProtocolV2Hook hook, String... inputLines) RefFilter refFilter, ProtocolV2Hook hook, String... inputLines)
throws Exception { throws Exception {
ByteArrayOutputStream send = new ByteArrayOutputStream(); ByteArrayInputStream send = linesAsInputStream(inputLines);
PacketLineOut pckOut = new PacketLineOut(send);
for (String line : inputLines) {
if (line == PacketLineIn.END) {
pckOut.end();
} else if (line == PacketLineIn.DELIM) {
pckOut.writeDelim();
} else {
pckOut.writeString(line);
}
}
server.getConfig().setString("protocol", null, "version", "2"); server.getConfig().setString("protocol", null, "version", "2");
UploadPack up = new UploadPack(server); UploadPack up = new UploadPack(server);
@ -453,11 +443,28 @@ public class UploadPackTest {
} }
ByteArrayOutputStream recv = new ByteArrayOutputStream(); ByteArrayOutputStream recv = new ByteArrayOutputStream();
up.upload(new ByteArrayInputStream(send.toByteArray()), recv, null); up.upload(send, recv, null);
return new ByteArrayInputStream(recv.toByteArray()); return new ByteArrayInputStream(recv.toByteArray());
} }
private static ByteArrayInputStream linesAsInputStream(String... inputLines)
throws IOException {
try (ByteArrayOutputStream send = new ByteArrayOutputStream()) {
PacketLineOut pckOut = new PacketLineOut(send);
for (String line : inputLines) {
if (line == PacketLineIn.END) {
pckOut.end();
} else if (line == PacketLineIn.DELIM) {
pckOut.writeDelim();
} else {
pckOut.writeString(line);
}
}
return new ByteArrayInputStream(send.toByteArray());
}
}
/* /*
* Invokes UploadPack with protocol v2 and sends it the given lines. * Invokes UploadPack with protocol v2 and sends it the given lines.
* Returns UploadPack's output stream, not including the capability * Returns UploadPack's output stream, not including the capability
@ -1552,6 +1559,45 @@ public class UploadPackTest {
assertTrue(client.hasObject(three.toObjectId())); assertTrue(client.hasObject(three.toObjectId()));
} }
@Test
public void testGetPeerAgentProtocolV0() throws Exception {
RevCommit one = remote.commit().message("1").create();
remote.update("one", one);
UploadPack up = new UploadPack(server);
ByteArrayInputStream send = linesAsInputStream(
"want " + one.getName() + " agent=JGit-test/1.2.3\n",
PacketLineIn.END,
"have 11cedf1b796d44207da702f7d420684022fc0f09\n", "done\n");
ByteArrayOutputStream recv = new ByteArrayOutputStream();
up.upload(send, recv, null);
assertEquals(up.getPeerUserAgent(), "JGit-test/1.2.3");
}
@Test
public void testGetPeerAgentProtocolV2() throws Exception {
server.getConfig().setString("protocol", null, "version", "2");
RevCommit one = remote.commit().message("1").create();
remote.update("one", one);
UploadPack up = new UploadPack(server);
up.setExtraParameters(Sets.of("version=2"));
ByteArrayInputStream send = linesAsInputStream(
"command=fetch\n", "agent=JGit-test/1.2.4\n",
PacketLineIn.DELIM, "want " + one.getName() + "\n",
"have 11cedf1b796d44207da702f7d420684022fc0f09\n", "done\n",
PacketLineIn.END);
ByteArrayOutputStream recv = new ByteArrayOutputStream();
up.upload(send, recv, null);
assertEquals(up.getPeerUserAgent(), "JGit-test/1.2.4");
}
private static class RejectAllRefFilter implements RefFilter { private static class RejectAllRefFilter implements RefFilter {
@Override @Override
public Map<String, Ref> filter(Map<String, Ref> refs) { public Map<String, Ref> filter(Map<String, Ref> refs) {

24
org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchRequest.java

@ -48,6 +48,7 @@ import java.util.List;
import java.util.Set; import java.util.Set;
import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
/** /**
@ -69,6 +70,9 @@ abstract class FetchRequest {
final List<String> deepenNotRefs; final List<String> deepenNotRefs;
@Nullable
final String agent;
/** /**
* Initialize the common fields of a fetch request. * Initialize the common fields of a fetch request.
* *
@ -88,11 +92,13 @@ abstract class FetchRequest {
* @param deepenSince * @param deepenSince
* Requests that the shallow clone/fetch should be cut at a * Requests that the shallow clone/fetch should be cut at a
* specific time, instead of depth * specific time, instead of depth
* @param agent
* agent as reported by the client in the request body
*/ */
FetchRequest(@NonNull Set<ObjectId> wantIds, int depth, FetchRequest(@NonNull Set<ObjectId> wantIds, int depth,
@NonNull Set<ObjectId> clientShallowCommits, long filterBlobLimit, @NonNull Set<ObjectId> clientShallowCommits, long filterBlobLimit,
@NonNull Set<String> clientCapabilities, int deepenSince, @NonNull Set<String> clientCapabilities, int deepenSince,
@NonNull List<String> deepenNotRefs) { @NonNull List<String> deepenNotRefs, @Nullable String agent) {
this.wantIds = requireNonNull(wantIds); this.wantIds = requireNonNull(wantIds);
this.depth = depth; this.depth = depth;
this.clientShallowCommits = requireNonNull(clientShallowCommits); this.clientShallowCommits = requireNonNull(clientShallowCommits);
@ -100,6 +106,7 @@ abstract class FetchRequest {
this.clientCapabilities = requireNonNull(clientCapabilities); this.clientCapabilities = requireNonNull(clientCapabilities);
this.deepenSince = deepenSince; this.deepenSince = deepenSince;
this.deepenNotRefs = requireNonNull(deepenNotRefs); this.deepenNotRefs = requireNonNull(deepenNotRefs);
this.agent = agent;
} }
/** /**
@ -146,7 +153,11 @@ abstract class FetchRequest {
* These options are listed and well-defined in the git protocol * These options are listed and well-defined in the git protocol
* specification. * specification.
* *
* @return capabilities sent by the client * The agent capability is not included in this set. It can be retrieved via
* {@link #getAgent()}.
*
* @return capabilities sent by the client (excluding the "agent"
* capability)
*/ */
@NonNull @NonNull
Set<String> getClientCapabilities() { Set<String> getClientCapabilities() {
@ -171,4 +182,13 @@ abstract class FetchRequest {
List<String> getDeepenNotRefs() { List<String> getDeepenNotRefs() {
return deepenNotRefs; return deepenNotRefs;
} }
/**
* @return string identifying the agent (as sent in the request body by the
* client)
*/
@Nullable
String getAgent() {
return agent;
}
} }

29
org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java

@ -48,6 +48,7 @@ import java.util.HashSet;
import java.util.Set; import java.util.Set;
import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
/** /**
@ -57,9 +58,9 @@ final class FetchV0Request extends FetchRequest {
FetchV0Request(@NonNull Set<ObjectId> wantIds, int depth, FetchV0Request(@NonNull Set<ObjectId> wantIds, int depth,
@NonNull Set<ObjectId> clientShallowCommits, long filterBlobLimit, @NonNull Set<ObjectId> clientShallowCommits, long filterBlobLimit,
@NonNull Set<String> clientCapabilities) { @NonNull Set<String> clientCapabilities, @Nullable String agent) {
super(wantIds, depth, clientShallowCommits, filterBlobLimit, super(wantIds, depth, clientShallowCommits, filterBlobLimit,
clientCapabilities, 0, Collections.emptyList()); clientCapabilities, 0, Collections.emptyList(), agent);
} }
static final class Builder { static final class Builder {
@ -74,6 +75,8 @@ final class FetchV0Request extends FetchRequest {
final Set<String> clientCaps = new HashSet<>(); final Set<String> clientCaps = new HashSet<>();
String agent;
/** /**
* @param objectId * @param objectId
* object id received in a "want" line * object id received in a "want" line
@ -111,7 +114,24 @@ final class FetchV0Request extends FetchRequest {
* @return this builder * @return this builder
*/ */
Builder addClientCapabilities(Collection<String> clientCapabilities) { Builder addClientCapabilities(Collection<String> clientCapabilities) {
clientCaps.addAll(clientCapabilities); for (String cap: clientCapabilities) {
// TODO(ifrade): Do this is done on parse time
if (cap.startsWith("agent=")) { //$NON-NLS-1$
agent = cap.substring("agent=".length()); //$NON-NLS-1$
} else {
clientCaps.add(cap);
}
}
return this;
}
/**
* @param clientAgent
* agent line sent by the client in the request body
* @return this builder
*/
Builder setAgent(String clientAgent) {
agent = clientAgent;
return this; return this;
} }
@ -127,7 +147,8 @@ final class FetchV0Request extends FetchRequest {
FetchV0Request build() { FetchV0Request build() {
return new FetchV0Request(wantIds, depth, clientShallowCommits, return new FetchV0Request(wantIds, depth, clientShallowCommits,
filterBlobLimit, clientCaps); filterBlobLimit, clientCaps, agent);
} }
} }
} }

15
org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java

@ -71,9 +71,6 @@ public final class FetchV2Request extends FetchRequest {
private final boolean doneReceived; private final boolean doneReceived;
@Nullable
private final String agent;
@NonNull @NonNull
private final List<String> serverOptions; private final List<String> serverOptions;
@ -86,11 +83,10 @@ public final class FetchV2Request extends FetchRequest {
boolean doneReceived, @NonNull Set<String> clientCapabilities, boolean doneReceived, @NonNull Set<String> clientCapabilities,
@Nullable String agent, @NonNull List<String> serverOptions) { @Nullable String agent, @NonNull List<String> serverOptions) {
super(wantIds, depth, clientShallowCommits, filterBlobLimit, super(wantIds, depth, clientShallowCommits, filterBlobLimit,
clientCapabilities, deepenSince, deepenNotRefs); clientCapabilities, deepenSince, deepenNotRefs, agent);
this.peerHas = requireNonNull(peerHas); this.peerHas = requireNonNull(peerHas);
this.wantedRefs = requireNonNull(wantedRefs); this.wantedRefs = requireNonNull(wantedRefs);
this.doneReceived = doneReceived; this.doneReceived = doneReceived;
this.agent = agent;
this.serverOptions = requireNonNull(serverOptions); this.serverOptions = requireNonNull(serverOptions);
} }
@ -117,15 +113,6 @@ public final class FetchV2Request extends FetchRequest {
return doneReceived; 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 * Options received in server-option lines. The caller can choose to act on
* these in an application-specific way * these in an application-specific way

7
org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

@ -1369,12 +1369,11 @@ public class UploadPack {
* @since 4.0 * @since 4.0
*/ */
public String getPeerUserAgent() { public String getPeerUserAgent() {
if (currentRequest == null) { if (currentRequest != null && currentRequest.getAgent() != null) {
return userAgent; return currentRequest.getAgent();
} }
return UserAgent.getAgent(currentRequest.getClientCapabilities(), return userAgent;
userAgent);
} }
private boolean negotiate(FetchRequest req, private boolean negotiate(FetchRequest req,

Loading…
Cancel
Save