UploadPack: Use reachability checker to validate non-advertised wants
In "Reachable commit" request validators, we need to check that a "want" in the request, that hasn't been advertised, is reachable from the refs visible to the user. Current code has intermixed the translation of ObjectIds to RevCommits (and its error handling) with the actual walk, with the delegation to bitmaps in restricted circunstances. Refactor the code to make it "flatter" and more readable. Move ObjectIds to RevCommits translation to its own functions. Use the reachability checker instead of a newly defined walk. Before the non-advertised wants were validated with bitmaps only if any "want" refered to an non-commit. Now they will be validated with bitmaps also if the "wants" refer all to commits. Change-Id: Ib925a48cde89672b07a88bba4e24d0457546baff Signed-off-by: Ivan Frade <ifrade@google.com>
This commit is contained in:
parent
601b0ae577
commit
7b96bd812e
|
@ -44,12 +44,11 @@
|
||||||
package org.eclipse.jgit.transport;
|
package org.eclipse.jgit.transport;
|
||||||
|
|
||||||
import static java.util.Collections.unmodifiableMap;
|
import static java.util.Collections.unmodifiableMap;
|
||||||
import static org.eclipse.jgit.util.RefMap.toRefMap;
|
|
||||||
import static org.eclipse.jgit.lib.Constants.R_TAGS;
|
import static org.eclipse.jgit.lib.Constants.R_TAGS;
|
||||||
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REF_IN_WANT;
|
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REF_IN_WANT;
|
||||||
|
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SERVER_OPTION;
|
||||||
import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_FETCH;
|
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.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_AGENT;
|
||||||
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_ALLOW_REACHABLE_SHA1_IN_WANT;
|
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;
|
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_ALLOW_TIP_SHA1_IN_WANT;
|
||||||
|
@ -65,6 +64,7 @@
|
||||||
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND;
|
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND;
|
||||||
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K;
|
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_THIN_PACK;
|
||||||
|
import static org.eclipse.jgit.util.RefMap.toRefMap;
|
||||||
|
|
||||||
import java.io.ByteArrayOutputStream;
|
import java.io.ByteArrayOutputStream;
|
||||||
import java.io.EOFException;
|
import java.io.EOFException;
|
||||||
|
@ -80,8 +80,10 @@
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Objects;
|
import java.util.Objects;
|
||||||
|
import java.util.Optional;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
import java.util.TreeMap;
|
import java.util.TreeMap;
|
||||||
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
import org.eclipse.jgit.annotations.NonNull;
|
import org.eclipse.jgit.annotations.NonNull;
|
||||||
import org.eclipse.jgit.annotations.Nullable;
|
import org.eclipse.jgit.annotations.Nullable;
|
||||||
|
@ -104,8 +106,11 @@
|
||||||
import org.eclipse.jgit.lib.Repository;
|
import org.eclipse.jgit.lib.Repository;
|
||||||
import org.eclipse.jgit.revwalk.AsyncRevObjectQueue;
|
import org.eclipse.jgit.revwalk.AsyncRevObjectQueue;
|
||||||
import org.eclipse.jgit.revwalk.BitmapWalker;
|
import org.eclipse.jgit.revwalk.BitmapWalker;
|
||||||
|
import org.eclipse.jgit.revwalk.BitmappedReachabilityChecker;
|
||||||
import org.eclipse.jgit.revwalk.DepthWalk;
|
import org.eclipse.jgit.revwalk.DepthWalk;
|
||||||
import org.eclipse.jgit.revwalk.ObjectWalk;
|
import org.eclipse.jgit.revwalk.ObjectWalk;
|
||||||
|
import org.eclipse.jgit.revwalk.PedestrianReachabilityChecker;
|
||||||
|
import org.eclipse.jgit.revwalk.ReachabilityChecker;
|
||||||
import org.eclipse.jgit.revwalk.RevCommit;
|
import org.eclipse.jgit.revwalk.RevCommit;
|
||||||
import org.eclipse.jgit.revwalk.RevFlag;
|
import org.eclipse.jgit.revwalk.RevFlag;
|
||||||
import org.eclipse.jgit.revwalk.RevFlagSet;
|
import org.eclipse.jgit.revwalk.RevFlagSet;
|
||||||
|
@ -1867,59 +1872,88 @@ private static void checkNotAdvertisedWantsUsingBitmap(ObjectReader reader,
|
||||||
|
|
||||||
private static void checkNotAdvertisedWants(UploadPack up,
|
private static void checkNotAdvertisedWants(UploadPack up,
|
||||||
List<ObjectId> notAdvertisedWants, Set<ObjectId> reachableFrom)
|
List<ObjectId> notAdvertisedWants, Set<ObjectId> reachableFrom)
|
||||||
throws MissingObjectException, IncorrectObjectTypeException, IOException {
|
throws IOException {
|
||||||
// Walk the requested commits back to the provided set of commits. If any
|
|
||||||
// commit exists, a branch was deleted or rewound and the repository owner
|
|
||||||
// no longer exports that requested item. If the requested commit is merged
|
|
||||||
// into an advertised branch it will be marked UNINTERESTING and no commits
|
|
||||||
// return.
|
|
||||||
|
|
||||||
ObjectReader reader = up.getRevWalk().getObjectReader();
|
ObjectReader reader = up.getRevWalk().getObjectReader();
|
||||||
try (RevWalk walk = new RevWalk(reader)) {
|
try (RevWalk walk = new RevWalk(reader)) {
|
||||||
walk.setRetainBody(false);
|
// Missing "wants" throw exception here
|
||||||
AsyncRevObjectQueue q = walk.parseAny(notAdvertisedWants, true);
|
List<RevObject> wantsAsObjs = objectIdsToRevObjects(walk,
|
||||||
try {
|
notAdvertisedWants);
|
||||||
RevObject obj;
|
List<RevCommit> wantsAsCommits = wantsAsObjs.stream()
|
||||||
while ((obj = q.next()) != null) {
|
.filter(obj -> obj instanceof RevCommit)
|
||||||
if (!(obj instanceof RevCommit)) {
|
.map(obj -> (RevCommit) obj)
|
||||||
// If unadvertized non-commits are requested, use
|
.collect(Collectors.toList());
|
||||||
// bitmaps. If there are no bitmaps, instead of
|
boolean allWantsAreCommits = wantsAsObjs.size() == wantsAsCommits
|
||||||
// incurring the expense of a manual walk, reject
|
.size();
|
||||||
// the request.
|
boolean repoHasBitmaps = reader.getBitmapIndex() != null;
|
||||||
BitmapIndex bitmapIndex = reader.getBitmapIndex();
|
|
||||||
if (bitmapIndex != null) {
|
if (!allWantsAreCommits) {
|
||||||
checkNotAdvertisedWantsUsingBitmap(
|
if (!repoHasBitmaps) {
|
||||||
reader,
|
// If unadvertized non-commits are requested, use
|
||||||
bitmapIndex,
|
// bitmaps. If there are no bitmaps, instead of
|
||||||
notAdvertisedWants,
|
// incurring the expense of a manual walk, reject
|
||||||
reachableFrom);
|
// the request.
|
||||||
return;
|
RevObject nonCommit = wantsAsObjs
|
||||||
}
|
.stream()
|
||||||
throw new WantNotValidException(obj);
|
.filter(obj -> !(obj instanceof RevCommit))
|
||||||
}
|
.limit(1)
|
||||||
walk.markStart((RevCommit) obj);
|
.collect(Collectors.toList()).get(0);
|
||||||
}
|
throw new WantNotValidException(nonCommit);
|
||||||
} catch (MissingObjectException notFound) {
|
|
||||||
throw new WantNotValidException(notFound.getObjectId(),
|
|
||||||
notFound);
|
|
||||||
} finally {
|
|
||||||
q.release();
|
|
||||||
}
|
|
||||||
for (ObjectId id : reachableFrom) {
|
|
||||||
try {
|
|
||||||
walk.markUninteresting(walk.parseCommit(id));
|
|
||||||
} catch (IncorrectObjectTypeException notCommit) {
|
|
||||||
continue;
|
|
||||||
}
|
}
|
||||||
|
checkNotAdvertisedWantsUsingBitmap(reader,
|
||||||
|
reader.getBitmapIndex(), notAdvertisedWants,
|
||||||
|
reachableFrom);
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
RevCommit bad = walk.next();
|
// All wants are commits, we can use ReachabilityChecker
|
||||||
if (bad != null) {
|
ReachabilityChecker reachabilityChecker = repoHasBitmaps
|
||||||
throw new WantNotValidException(bad);
|
? new BitmappedReachabilityChecker(walk)
|
||||||
|
: new PedestrianReachabilityChecker(true, walk);
|
||||||
|
|
||||||
|
List<RevCommit> starters = objectIdsToRevCommits(walk,
|
||||||
|
reachableFrom);
|
||||||
|
Optional<RevCommit> unreachable = reachabilityChecker
|
||||||
|
.areAllReachable(wantsAsCommits, starters);
|
||||||
|
if (unreachable.isPresent()) {
|
||||||
|
throw new WantNotValidException(unreachable.get());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
} catch (MissingObjectException notFound) {
|
||||||
|
throw new WantNotValidException(notFound.getObjectId(), notFound);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Resolve the ObjectIds into RevObjects. Any missing object raises an
|
||||||
|
// exception
|
||||||
|
private static List<RevObject> objectIdsToRevObjects(RevWalk walk,
|
||||||
|
Iterable<ObjectId> objectIds)
|
||||||
|
throws MissingObjectException, IOException {
|
||||||
|
List<RevObject> result = new ArrayList<>();
|
||||||
|
for (ObjectId objectId : objectIds) {
|
||||||
|
result.add(walk.parseAny(objectId));
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Get commits from object ids. If the id is not a commit, ignore it. If the
|
||||||
|
// id doesn't exist, report the missing object in a exception.
|
||||||
|
private static List<RevCommit> objectIdsToRevCommits(RevWalk walk,
|
||||||
|
Iterable<ObjectId> objectIds)
|
||||||
|
throws MissingObjectException, IOException {
|
||||||
|
List<RevCommit> result = new ArrayList<>();
|
||||||
|
for (ObjectId objectId : objectIds) {
|
||||||
|
try {
|
||||||
|
result.add(walk.parseCommit(objectId));
|
||||||
|
} catch (IncorrectObjectTypeException e) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
private void addCommonBase(RevObject o) {
|
private void addCommonBase(RevObject o) {
|
||||||
if (!o.has(COMMON)) {
|
if (!o.has(COMMON)) {
|
||||||
o.add(COMMON);
|
o.add(COMMON);
|
||||||
|
|
Loading…
Reference in New Issue