motiejus
/
jgit
1
Fork 0
Code Releases Activity

UploadPack: avoid conflating shallow commit lists in protocol v2 

At the moment there are two copies of the client shallow commit list:
one in the request and another in the clientShallowCommits member of
the class.

The verifyShallowCommit function was removing missing object ids
from the member but not the request list, and code afterwards was
using the request's version.

In practice, this didn't cause trouble because these shallow commits
are used as endpoint for a walk, and missing ids are just never reached.

Change-Id: I70a8f1fd46de135da09f16e5d954693c8438ffcb
Signed-off-by: Ivan Frade <ifrade@google.com>
This commit is contained in:
Ivan Frade 2018-08-29 16:36:42 -07:00
parent aa5007c576
commit e665e3fcd4
2 changed files with 50 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
View File

@ -1417,6 +1417,44 @@ public void testV2FetchWantRefAndDeepen() throws Exception {
assertFalse(client.hasObject(parent.toObjectId()));
}
@Test
public void testV2FetchMissingShallow() throws Exception {
RevCommit one = remote.commit().message("1").create();
RevCommit two = remote.commit().message("2").parent(one).create();
RevCommit three = remote.commit().message("3").parent(two).create();
remote.update("three", three);
server.getConfig().setBoolean("uploadpack", null, "allowrefinwant",
true);
ByteArrayInputStream recvStream = uploadPackV2("command=fetch\n",
PacketLineIn.DELIM,
"want-ref refs/heads/three\n",
"deepen 3",
"shallow 0123012301230123012301230123012301230123",
"shallow " + two.getName() + '\n',
"done\n",
PacketLineIn.END);
PacketLineIn pckIn = new PacketLineIn(recvStream);
assertThat(pckIn.readString(), is("shallow-info"));
assertThat(pckIn.readString(),
is("shallow " + one.toObjectId().getName()));
assertThat(pckIn.readString(),
is("unshallow " + two.toObjectId().getName()));
assertThat(pckIn.readString(), theInstance(PacketLineIn.DELIM));
assertThat(pckIn.readString(), is("wanted-refs"));
assertThat(pckIn.readString(),
is(three.toObjectId().getName() + " refs/heads/three"));
assertThat(pckIn.readString(), theInstance(PacketLineIn.DELIM));
assertThat(pckIn.readString(), is("packfile"));
parsePack(recvStream);
assertTrue(client.hasObject(one.toObjectId()));
assertTrue(client.hasObject(two.toObjectId()));
assertTrue(client.hasObject(three.toObjectId()));
}
private static class RejectAllRefFilter implements RefFilter {
@Override
public Map<String, Ref> filter(Map<String, Ref> refs) {

19
org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
View File

@ -295,7 +295,7 @@ public Set<String> getOptions() {
private final Set<RevObject> commonBase = new HashSet<>();
/** Shallow commits the client already has. */
private final Set<ObjectId> clientShallowCommits = new HashSet<>();
private Set<ObjectId> clientShallowCommits = new HashSet<>();
/** Desired depth from the client on a shallow request. */
private int depth;
@ -832,7 +832,7 @@ else if (requestValidator instanceof AnyRequestValidator)
multiAck = MultiAck.OFF;
if (!clientShallowCommits.isEmpty())
verifyClientShallow();
verifyClientShallow(clientShallowCommits);
if (depth != 0)
processShallow(null, unshallowCommits, true);
if (!clientShallowCommits.isEmpty())
@ -1068,7 +1068,7 @@ private void fetchV2() throws IOException {
// copying data back to class fields
options = req.getOptions();
wantIds.addAll(req.getWantsIds());
clientShallowCommits.addAll(req.getClientShallowCommits());
clientShallowCommits = req.getClientShallowCommits();
depth = req.getDepth();
shallowSince = req.getShallowSince();
filterBlobLimit = req.getFilterBlobLimit();
@ -1079,7 +1079,7 @@ private void fetchV2() throws IOException {
List<ObjectId> unshallowCommits = new ArrayList<>();
if (!req.getClientShallowCommits().isEmpty()) {
verifyClientShallow();
verifyClientShallow(req.getClientShallowCommits());
}
if (req.getDepth() != 0 || req.getShallowSince() != 0
|| !req.getShallowExcludeRefs().isEmpty()) {
@ -1303,9 +1303,14 @@ private void processShallow(@Nullable List<ObjectId> shallowCommits,
}
}
private void verifyClientShallow()
/*
* Verify all shallow lines refer to commits
*
* It can mutate the input set (removing missing object ids from it)
*/
private void verifyClientShallow(Set<ObjectId> shallowCommits)
throws IOException, PackProtocolException {
AsyncRevObjectQueue q = walk.parseAny(clientShallowCommits, true);
AsyncRevObjectQueue q = walk.parseAny(shallowCommits, true);
try {
for (;;) {
try {
@ -1323,7 +1328,7 @@ private void verifyClientShallow()
} catch (MissingObjectException notCommit) {
// shallow objects not known at the server are ignored
// by git-core upload-pack, match that behavior.
clientShallowCommits.remove(notCommit.getObjectId());
shallowCommits.remove(notCommit.getObjectId());
continue;
}
}