From 945d43d50730277f875ff46e823e72f35493ccad Mon Sep 17 00:00:00 2001 From: Demetr Starshov Date: Tue, 8 Oct 2019 16:53:22 -0700 Subject: [PATCH] ReceivePack: Moves connectivity check to separate class Move all connectivity check to separate classes. Set default to be FullConnectivityChecker i.e. checker which will check connectivity from all advertised refs. Add ability to set other connectivity checker which can use different approach. Signed-off-by: Demetr Starshov Change-Id: Ibb107dbfbdde8ad109be25b5ecf42be7660ef736 --- .../eclipse/jgit/transport/ReceivePack.java | 109 +++------- .../internal/ConnectivityChecker.java | 138 ++++++++++++ .../internal/FullConnectivityChecker.java | 200 ++++++++++++++++++ 3 files changed, 362 insertions(+), 85 deletions(-) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/ConnectivityChecker.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/FullConnectivityChecker.java diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 4e425fe97..3876958b3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -74,7 +74,6 @@ import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.errors.InvalidObjectIdException; import org.eclipse.jgit.errors.LargeObjectException; -import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.errors.TooLargePackException; import org.eclipse.jgit.errors.UnpackException; @@ -93,24 +92,21 @@ import org.eclipse.jgit.lib.ObjectChecker; import org.eclipse.jgit.lib.ObjectDatabase; import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.ObjectIdSubclassMap; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.revwalk.ObjectWalk; -import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevFlag; import org.eclipse.jgit.revwalk.RevObject; -import org.eclipse.jgit.revwalk.RevSort; -import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.PacketLineIn.InputOverLimitIOException; import org.eclipse.jgit.transport.ReceiveCommand.Result; import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser; +import org.eclipse.jgit.transport.internal.ConnectivityChecker.ConnectivityCheckInfo; +import org.eclipse.jgit.transport.internal.ConnectivityChecker; +import org.eclipse.jgit.transport.internal.FullConnectivityChecker; import org.eclipse.jgit.util.io.InterruptTimer; import org.eclipse.jgit.util.io.LimitedInputStream; import org.eclipse.jgit.util.io.TimeoutInputStream; @@ -273,7 +269,7 @@ public Set getCapabilities() { /** Lock around the received pack file, while updating refs. */ private PackLock packLock; - private boolean checkReferencedIsReachable; + private boolean checkReferencedAreReachable; /** Git object size limit */ private long maxObjectSizeLimit; @@ -292,6 +288,9 @@ public Set getCapabilities() { private ReceivedPackStatistics stats; + /** Connectivity checker to use. */ + protected ConnectivityChecker connectivityChecker = new FullConnectivityChecker(); + /** Hook to validate the update commands before execution. */ private PreReceiveHook preReceive; @@ -514,7 +513,7 @@ public final Set getAdvertisedObjects() { * reference. */ public boolean isCheckReferencedObjectsAreReachable() { - return checkReferencedIsReachable; + return checkReferencedAreReachable; } /** @@ -539,7 +538,7 @@ public boolean isCheckReferencedObjectsAreReachable() { * {@code true} to enable the additional check. */ public void setCheckReferencedObjectsAreReachable(boolean b) { - this.checkReferencedIsReachable = b; + this.checkReferencedAreReachable = b; } /** @@ -1514,10 +1513,10 @@ private void receivePack() throws IOException { parser = ins.newPackParser(packInputStream()); parser.setAllowThin(true); - parser.setNeedNewObjectIds(checkReferencedIsReachable); - parser.setNeedBaseObjectIds(checkReferencedIsReachable); - parser.setCheckEofAfterPackFooter( - !biDirectionalPipe && !isExpectDataAfterPackFooter()); + parser.setNeedNewObjectIds(checkReferencedAreReachable); + parser.setNeedBaseObjectIds(checkReferencedAreReachable); + parser.setCheckEofAfterPackFooter(!biDirectionalPipe + && !isExpectDataAfterPackFooter()); parser.setExpectDataAfterPackFooter(isExpectDataAfterPackFooter()); parser.setObjectChecker(objectChecker); parser.setLockMessage(lockMsg); @@ -1567,8 +1566,6 @@ private void checkSubmodules() throws IOException, LargeObjectException, } private void checkConnectivity() throws IOException { - ObjectIdSubclassMap baseObjects = null; - ObjectIdSubclassMap providedObjects = null; ProgressMonitor checking = NullProgressMonitor.INSTANCE; if (sideBand && !quiet) { SideBandProgressMonitor m = new SideBandProgressMonitor(msgOut); @@ -1576,76 +1573,18 @@ private void checkConnectivity() throws IOException { checking = m; } - if (checkReferencedIsReachable) { - baseObjects = parser.getBaseObjectIds(); - providedObjects = parser.getNewObjectIds(); - } - parser = null; + connectivityChecker.checkConnectivity(createConnectivityCheckInfo(), + advertisedHaves, checking); + } - try (ObjectWalk ow = new ObjectWalk(db)) { - if (baseObjects != null) { - ow.sort(RevSort.TOPO); - if (!baseObjects.isEmpty()) - ow.sort(RevSort.BOUNDARY, true); - } - - for (ReceiveCommand cmd : commands) { - if (cmd.getResult() != Result.NOT_ATTEMPTED) - continue; - if (cmd.getType() == ReceiveCommand.Type.DELETE) - continue; - ow.markStart(ow.parseAny(cmd.getNewId())); - } - for (ObjectId have : advertisedHaves) { - RevObject o = ow.parseAny(have); - ow.markUninteresting(o); - - if (baseObjects != null && !baseObjects.isEmpty()) { - o = ow.peel(o); - if (o instanceof RevCommit) - o = ((RevCommit) o).getTree(); - if (o instanceof RevTree) - ow.markUninteresting(o); - } - } - - checking.beginTask(JGitText.get().countingObjects, - ProgressMonitor.UNKNOWN); - RevCommit c; - while ((c = ow.next()) != null) { - checking.update(1); - if (providedObjects != null // - && !c.has(RevFlag.UNINTERESTING) // - && !providedObjects.contains(c)) - throw new MissingObjectException(c, Constants.TYPE_COMMIT); - } - - RevObject o; - while ((o = ow.nextObject()) != null) { - checking.update(1); - if (o.has(RevFlag.UNINTERESTING)) - continue; - - if (providedObjects != null) { - if (providedObjects.contains(o)) { - continue; - } - throw new MissingObjectException(o, o.getType()); - } - - if (o instanceof RevBlob && !db.getObjectDatabase().has(o)) - throw new MissingObjectException(o, Constants.TYPE_BLOB); - } - checking.endTask(); - - if (baseObjects != null) { - for (ObjectId id : baseObjects) { - o = ow.parseAny(id); - if (!o.has(RevFlag.UNINTERESTING)) - throw new MissingObjectException(o, o.getType()); - } - } - } + private ConnectivityCheckInfo createConnectivityCheckInfo() { + ConnectivityCheckInfo info = new ConnectivityCheckInfo(); + info.setCheckObjects(checkReferencedAreReachable); + info.setCommands(getAllCommands()); + info.setRepository(db); + info.setParser(parser); + info.setWalk(walk); + return info; } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/ConnectivityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/ConnectivityChecker.java new file mode 100644 index 000000000..d6efada65 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/ConnectivityChecker.java @@ -0,0 +1,138 @@ +/* + * Copyright (c) 2019, Google LLC and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.transport.internal; + +import java.io.IOException; +import java.util.List; +import java.util.Set; + +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ProgressMonitor; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.transport.PackParser; +import org.eclipse.jgit.transport.ReceiveCommand; + +/** + * Checks that a received pack only depends on objects which are reachable from + * a defined set of references. + */ +public interface ConnectivityChecker { + + /** + * Checks connectivity of the commit graph after pack uploading. + * + * @param connectivityCheckInfo + * Input for the connectivity check. + * @param haves + * Set of references known for client. + * @param pm + * Monitor to publish progress to. + * @throws IOException + * an error occurred during connectivity checking. + * + */ + void checkConnectivity(ConnectivityCheckInfo connectivityCheckInfo, + Set haves, ProgressMonitor pm) + throws IOException; + + /** + * POJO which is used to pass all information which is needed to perform + * connectivity check. + */ + public static class ConnectivityCheckInfo { + private Repository repository; + + private PackParser parser; + + private boolean checkObjects; + + private List commands; + + private RevWalk walk; + + /** + * @return database we write the stored objects into. + */ + public Repository getRepository() { + return repository; + } + + /** + * @param repository + * set database we write the stored objects into. + */ + public void setRepository(Repository repository) { + this.repository = repository; + } + + /** + * @return the parser used to parse pack. + */ + public PackParser getParser() { + return parser; + } + + /** + * @param parser + * the parser to set + */ + public void setParser(PackParser parser) { + this.parser = parser; + } + + /** + * @return if checker should check objects. + */ + public boolean isCheckObjects() { + return checkObjects; + } + + /** + * @param checkObjects + * set if checker should check referenced objects outside of + * the received pack are reachable. + */ + public void setCheckObjects(boolean checkObjects) { + this.checkObjects = checkObjects; + } + + /** + * @return command received by the current request. + */ + public List getCommands() { + return commands; + } + + /** + * @param commands + * set command received by the current request. + */ + public void setCommands(List commands) { + this.commands = commands; + } + + /** + * @param walk + * the walk to parse commits + */ + public void setWalk(RevWalk walk) { + this.walk = walk; + } + + /** + * @return the walk to parse commits + */ + public RevWalk getWalk() { + return walk; + } + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/FullConnectivityChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/FullConnectivityChecker.java new file mode 100644 index 000000000..4adddf087 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/FullConnectivityChecker.java @@ -0,0 +1,200 @@ +/* + * Copyright (c) 2019, Google LLC and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.transport.internal; + +import java.io.IOException; +import java.util.Set; + +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectIdSubclassMap; +import org.eclipse.jgit.lib.ProgressMonitor; +import org.eclipse.jgit.revwalk.ObjectWalk; +import org.eclipse.jgit.revwalk.RevBlob; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevFlag; +import org.eclipse.jgit.revwalk.RevObject; +import org.eclipse.jgit.revwalk.RevSort; +import org.eclipse.jgit.revwalk.RevTree; +import org.eclipse.jgit.transport.ReceiveCommand; +import org.eclipse.jgit.transport.ReceiveCommand.Result; + +/** + * A connectivity checker that uses the entire reference database to perform + * reachability checks when checking the connectivity of objects. If + * info.isCheckObjects() is set it will also check that objects referenced by + * deltas are either provided or reachable as well. + */ +public final class FullConnectivityChecker implements ConnectivityChecker { + @Override + public void checkConnectivity(ConnectivityCheckInfo connectivityCheckInfo, + Set haves, ProgressMonitor pm) + throws MissingObjectException, IOException { + pm.beginTask(JGitText.get().countingObjects, + ProgressMonitor.UNKNOWN); + try (ObjectWalk ow = new ObjectWalk(connectivityCheckInfo.getRepository())) { + if (!markStartAndKnownNodes(connectivityCheckInfo, ow, haves, + pm)) { + return; + } + checkCommitTree(connectivityCheckInfo, ow, pm); + checkObjects(connectivityCheckInfo, ow, pm); + } finally { + pm.endTask(); + } + } + + /** + * @param connectivityCheckInfo + * Source for connectivity check. + * @param ow + * Walk which can also check blobs. + * @param haves + * Set of references known for client. + * @param pm + * Monitor to publish progress to. + * @return true if at least one new node was marked. + * @throws IOException + * an error occurred during connectivity checking. + */ + private boolean markStartAndKnownNodes( + ConnectivityCheckInfo connectivityCheckInfo, + ObjectWalk ow, + Set haves, ProgressMonitor pm) + throws IOException { + boolean markTrees = connectivityCheckInfo + .isCheckObjects() + && !connectivityCheckInfo.getParser().getBaseObjectIds() + .isEmpty(); + if (connectivityCheckInfo.isCheckObjects()) { + ow.sort(RevSort.TOPO); + if (!connectivityCheckInfo.getParser().getBaseObjectIds() + .isEmpty()) { + ow.sort(RevSort.BOUNDARY, true); + } + } + boolean hasInteresting = false; + + for (ReceiveCommand cmd : connectivityCheckInfo.getCommands()) { + if (cmd.getResult() != Result.NOT_ATTEMPTED) { + continue; + } + if (cmd.getType() == ReceiveCommand.Type.DELETE) { + continue; + } + if (haves.contains(cmd.getNewId())) { + continue; + } + ow.markStart(ow.parseAny(cmd.getNewId())); + pm.update(1); + hasInteresting = true; + } + if (!hasInteresting) { + return false; + } + for (ObjectId have : haves) { + RevObject o = ow.parseAny(have); + ow.markUninteresting(o); + pm.update(1); + + if (markTrees) { + o = ow.peel(o); + if (o instanceof RevCommit) { + o = ((RevCommit) o).getTree(); + } + if (o instanceof RevTree) { + ow.markUninteresting(o); + } + } + } + return true; + } + + /** + * @param connectivityCheckInfo + * Source for connectivity check. + * @param ow + * Walk which can also check blobs. + * @param pm + * Monitor to publish progress to. + * @throws IOException + * an error occurred during connectivity checking. + */ + private void checkCommitTree(ConnectivityCheckInfo connectivityCheckInfo, + ObjectWalk ow, + ProgressMonitor pm) throws IOException { + RevCommit c; + ObjectIdSubclassMap newObjectIds = connectivityCheckInfo + .getParser() + .getNewObjectIds(); + while ((c = ow.next()) != null) { + pm.update(1); + if (connectivityCheckInfo.isCheckObjects() + && !c.has(RevFlag.UNINTERESTING) + && !newObjectIds.contains(c)) { + throw new MissingObjectException(c, Constants.TYPE_COMMIT); + } + } + } + + /** + * @param connectivityCheckInfo + * Source for connectivity check. + * @param ow + * Walk which can also check blobs. + * @param pm + * Monitor to publish progress to. + * @throws IOException + * an error occurred during connectivity checking. + * + */ + private void checkObjects(ConnectivityCheckInfo connectivityCheckInfo, + ObjectWalk ow, + ProgressMonitor pm) throws IOException { + RevObject o; + ObjectIdSubclassMap newObjectIds = connectivityCheckInfo + .getParser() + .getNewObjectIds(); + + while ((o = ow.nextObject()) != null) { + pm.update(1); + if (o.has(RevFlag.UNINTERESTING)) { + continue; + } + + if (connectivityCheckInfo.isCheckObjects()) { + if (newObjectIds.contains(o)) { + continue; + } + throw new MissingObjectException(o, o.getType()); + + } + + if (o instanceof RevBlob + && !connectivityCheckInfo.getRepository().getObjectDatabase() + .has(o)) { + throw new MissingObjectException(o, Constants.TYPE_BLOB); + } + } + + if (connectivityCheckInfo.isCheckObjects()) { + for (ObjectId id : connectivityCheckInfo.getParser() + .getBaseObjectIds()) { + o = ow.parseAny(id); + if (!o.has(RevFlag.UNINTERESTING)) { + throw new MissingObjectException(o, o.getType()); + } + } + } + } +}