github
/
jgit
mirror of https://gitee.com/github-mirror-repo/jgit.git
1
0
Fork 0
Code Issues Releases Wiki Activity
Browse Source

UploadPack: Defer want-ref resolution to after parsing 

ProtocolV2Parser explains:

  // TODO(ifrade): This validation should be done after the
  // protocol parsing. It is not a protocol problem asking for an
  // unexisting ref and we wouldn't need the ref database here.

Do so.  This way all ref database accesses are in one place, in the
UploadPack class.

No user-visible change intended --- this is just to make the code
easier to manipulate.

Change-Id: I68e87dff7b9a63ccc169bd0836e8e8baaf5d1048
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
stable-5.1
Jonathan Nieder 6 years ago committed by Matthias Sohn
parent
25deb30460
commit
1638a2fce8
4 changed files with 50 additions and 77 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 45
      org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java
  2. 22
      org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java
  3. 32
      org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java
  4. 28
      org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

45
org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java
Unescape View File

@ -42,7 +42,6 @@
*/
package org.eclipse.jgit.transport;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItems;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
@ -160,8 +159,7 @@ public class ProtocolV2ParserTest {
PacketLineIn.END);
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.getDefault());
FetchV2Request request = parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertTrue(request.getOptions()
.contains(GitProtocolConstants.OPTION_THIN_PACK));
assertTrue(request.getOptions()
@ -191,8 +189,7 @@ public class ProtocolV2ParserTest {
PacketLineIn.END);
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.getDefault());
FetchV2Request request = parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertThat(objIdsAsStrings(request.getClientShallowCommits()),
hasItems("28274d02c489f4c7e68153056e9061a46f62d7a0",
"145e683b229dcab9d0e2ccb01b386f9ecc17d29d"));
@ -211,8 +208,7 @@ public class ProtocolV2ParserTest {
PacketLineIn.END);
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.getDefault());
FetchV2Request request = parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertThat(objIdsAsStrings(request.getClientShallowCommits()),
hasItems("28274d02c489f4c7e68153056e9061a46f62d7a0",
"145e683b229dcab9d0e2ccb01b386f9ecc17d29d"));
@ -229,8 +225,7 @@ public class ProtocolV2ParserTest {
PacketLineIn.END);
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.getDefault());
FetchV2Request request = parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertThat(objIdsAsStrings(request.getClientShallowCommits()),
hasItems("28274d02c489f4c7e68153056e9061a46f62d7a0",
"145e683b229dcab9d0e2ccb01b386f9ecc17d29d"));
@ -244,8 +239,7 @@ public class ProtocolV2ParserTest {
PacketLineIn.END);
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.start().allowFilter().done());
FetchV2Request request = parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertEquals(0, request.getFilterBlobLimit());
}
@ -256,8 +250,7 @@ public class ProtocolV2ParserTest {
PacketLineIn.END);
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.start().allowFilter().done());
FetchV2Request request = parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertEquals(15, request.getFilterBlobLimit());
}
@ -270,8 +263,7 @@ public class ProtocolV2ParserTest {
PacketLineIn.END);
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.start().allowFilter().done());
FetchV2Request request = parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertEquals(0, request.getFilterBlobLimit());
}
@ -282,8 +274,7 @@ public class ProtocolV2ParserTest {
"filter blob:limit=12", PacketLineIn.END);
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.getDefault());
parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
parser.parseFetchRequest(pckIn);
}
@Test
@ -300,23 +291,16 @@ public class ProtocolV2ParserTest {
ProtocolV2Parser parser = new ProtocolV2Parser(
ConfigBuilder.start().allowRefInWant().done());
FetchV2Request request = parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertEquals(1, request.getWantedRefs().size());
assertThat(request.getWantedRefs().keySet(),
hasItems("refs/heads/branchA"));
assertEquals(2, request.getWantsIds().size());
assertThat(request.getWantedRefs(), hasItems("refs/heads/branchA"));
assertEquals(1, request.getWantsIds().size());
assertThat(objIdsAsStrings(request.getWantsIds()),
hasItems("e4980cdc48cfa1301493ca94eb70523f6788b819",
one.getName()));
hasItems("e4980cdc48cfa1301493ca94eb70523f6788b819"));
}
@Test
public void testFetchWithRefInWantUnknownRef() throws Exception {
thrown.expect(PackProtocolException.class);
thrown.expectMessage(containsString("refs/heads/branchC"));
PacketLineIn pckIn = formatAsPacketLine(PacketLineIn.DELIM,
"want e4980cdc48cfa1301493ca94eb70523f6788b819",
"want-ref refs/heads/branchC",
@ -329,8 +313,9 @@ public class ProtocolV2ParserTest {
testRepo.update("branchA", one);
testRepo.update("branchB", two);
parser.parseFetchRequest(pckIn,
testRepo.getRepository().getRefDatabase());
FetchV2Request request = parser.parseFetchRequest(pckIn);
assertEquals(1, request.getWantedRefs().size());
assertThat(request.getWantedRefs(), hasItems("refs/heads/branchC"));
}
}

22
org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java
Unescape View File

@ -45,9 +45,7 @@ package org.eclipse.jgit.transport;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.lib.ObjectId;
@ -63,7 +61,7 @@ import org.eclipse.jgit.lib.ObjectId;
public final class FetchV2Request {
private final List<ObjectId> peerHas;
private final TreeMap<String, ObjectId> wantedRefs;
private final List<String> wantedRefs;
private final Set<ObjectId> wantsIds;
@ -82,7 +80,7 @@ public final class FetchV2Request {
private final boolean doneReceived;
private FetchV2Request(List<ObjectId> peerHas,
TreeMap<String, ObjectId> wantedRefs, Set<ObjectId> wantsIds,
List<String> wantedRefs, Set<ObjectId> wantsIds,
Set<ObjectId> clientShallowCommits, int deepenSince,
List<String> deepenNotRefs, int depth, long filterBlobLimit,
boolean doneReceived, Set<String> options) {
@ -110,12 +108,12 @@ public final class FetchV2Request {
* @return list of references in the "want-ref" lines of the request
*/
@NonNull
Map<String, ObjectId> getWantedRefs() {
return this.wantedRefs;
List<String> getWantedRefs() {
return wantedRefs;
}
/**
* @return object ids in the "want" (and "want-ref") lines of the request
* @return object ids in the "want" (but not "want-ref") lines of the request
*/
@NonNull
Set<ObjectId> getWantsIds() {
@ -198,7 +196,7 @@ public final class FetchV2Request {
static final class Builder {
List<ObjectId> peerHas = new ArrayList<>();
TreeMap<String, ObjectId> wantedRefs = new TreeMap<>();
List<String> wantedRefs = new ArrayList<>();
Set<ObjectId> wantsIds = new HashSet<>();
@ -234,12 +232,10 @@ public final class FetchV2Request {
*
* @param refName
* reference name
* @param oid
* object id
* @return the builder
*/
Builder addWantedRef(String refName, ObjectId oid) {
wantedRefs.put(refName, oid);
Builder addWantedRef(String refName) {
wantedRefs.add(refName);
return this;
}
@ -258,7 +254,7 @@ public final class FetchV2Request {
* from a "want" line in a fetch request
* @return the builder
*/
Builder addWantsIds(ObjectId objectId) {
Builder addWantsId(ObjectId objectId) {
wantsIds.add(objectId);
return this;
}

32
org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java
Unescape View File

@ -57,8 +57,6 @@ import java.text.MessageFormat;
import org.eclipse.jgit.errors.PackProtocolException;
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
/**
* Parse the incoming git protocol lines from the wire and translate them into a
@ -79,24 +77,17 @@ final class ProtocolV2Parser {
* 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.
*
* This operation requires the reference database to validate incoming
* references.
*
* @param pckIn
* incoming lines
* @param refdb
* reference database (to validate that received references exist
* and point to valid objects)
* @return A FetchV2Request populated with information received from the
* wire.
* @throws PackProtocolException
* incompatible options, wrong type of arguments or other issues
* where the request breaks the protocol.
* @throws IOException
* an IO error prevented reading the incoming message or
* accessing the ref database.
* an IO error prevented reading the incoming message.
*/
FetchV2Request parseFetchRequest(PacketLineIn pckIn, RefDatabase refdb)
FetchV2Request parseFetchRequest(PacketLineIn pckIn)
throws PackProtocolException, IOException {
FetchV2Request.Builder reqBuilder = FetchV2Request.builder();
@ -116,25 +107,10 @@ final class ProtocolV2Parser {
boolean filterReceived = false;
while ((line = pckIn.readString()) != PacketLineIn.END) {
if (line.startsWith("want ")) { //$NON-NLS-1$
reqBuilder.addWantsIds(ObjectId.fromString(line.substring(5)));
reqBuilder.addWantsId(ObjectId.fromString(line.substring(5)));
} else if (transferConfig.isAllowRefInWant()
&& line.startsWith(OPTION_WANT_REF + " ")) { //$NON-NLS-1$
String refName = line.substring(OPTION_WANT_REF.length() + 1);
// TODO(ifrade): This validation should be done after the
// protocol parsing. It is not a protocol problem asking for an
// unexisting ref and we wouldn't need the ref database here
Ref ref = refdb.exactRef(refName);
if (ref == null) {
throw new PackProtocolException(MessageFormat
.format(JGitText.get().invalidRefName, refName));
}
ObjectId oid = ref.getObjectId();
if (oid == null) {
throw new PackProtocolException(MessageFormat
.format(JGitText.get().invalidRefName, refName));
}
reqBuilder.addWantedRef(refName, oid);
reqBuilder.addWantsIds(oid);
reqBuilder.addWantedRef(line.substring(OPTION_WANT_REF.length() + 1));
} else if (line.startsWith("have ")) { //$NON-NLS-1$
reqBuilder.addPeerHas(ObjectId.fromString(line.substring(5)));
} else if (line.equals("done")) { //$NON-NLS-1$

28
org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
Unescape View File

@ -79,6 +79,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.errors.CorruptObjectException;
@ -994,8 +995,7 @@ public class UploadPack {
}
ProtocolV2Parser parser = new ProtocolV2Parser(transferConfig);
FetchV2Request req = parser.parseFetchRequest(pckIn,
db.getRefDatabase());
FetchV2Request req = parser.parseFetchRequest(pckIn);
rawOut.stopBuffering();
protocolV2Hook.onFetch(req);
@ -1003,13 +1003,29 @@ public class UploadPack {
// TODO(ifrade): Refactor to pass around the Request object, instead of
// copying data back to class fields
options = req.getOptions();
wantIds.addAll(req.getWantsIds());
clientShallowCommits = req.getClientShallowCommits();
depth = req.getDepth();
shallowSince = req.getDeepenSince();
filterBlobLimit = req.getFilterBlobLimit();
deepenNotRefs = req.getDeepenNotRefs();
wantIds.addAll(req.getWantsIds());
Map<String, ObjectId> wantedRefs = new TreeMap<>();
for (String refName : req.getWantedRefs()) {
Ref ref = db.getRefDatabase().exactRef(refName);
if (ref == null) {
throw new PackProtocolException(MessageFormat
.format(JGitText.get().invalidRefName, refName));
}
ObjectId oid = ref.getObjectId();
if (oid == null) {
throw new PackProtocolException(MessageFormat
.format(JGitText.get().invalidRefName, refName));
}
wantIds.add(oid);
wantedRefs.put(refName, oid);
}
boolean sectionSent = false;
@Nullable List<ObjectId> shallowCommits = null;
List<ObjectId> unshallowCommits = new ArrayList<>();
@ -1059,13 +1075,13 @@ public class UploadPack {
sectionSent = true;
}
if (!req.getWantedRefs().isEmpty()) {
if (!wantedRefs.isEmpty()) {
if (sectionSent) {
pckOut.writeDelim();
}
pckOut.writeString("wanted-refs\n"); //$NON-NLS-1$
for (Map.Entry<String, ObjectId> entry : req.getWantedRefs()
.entrySet()) {
for (Map.Entry<String, ObjectId> entry :
wantedRefs.entrySet()) {
pckOut.writeString(entry.getValue().getName() + ' ' +
entry.getKey() + '\n');
}

Write Preview
Loading…
Cancel
Save