From fee679b587b189d6eec1948beb780ec44bc3e887 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 18 Jun 2013 14:58:44 -0700 Subject: [PATCH 1/6] Add RequestPolicy.TIP to allow fetching non-advertised ref tips Users of UploadPack may set a custom RefFilter or AdvertisedRefsHook that limits which refs are advertised, but clients may learn of a SHA-1 that the server should have as a ref tip through some alternative means. Support serving such objects from the server side with a new RequestPolicy. As with ADVERTISED, we need a special relaxed RequestPolicy to allow commits reachable from the set of valid tips for unidirectional connections. Change-Id: I0d0cc4f8ee04d265e5be8221b9384afb1b374315 --- .../eclipse/jgit/transport/UploadPack.java | 94 ++++++++++++++----- 1 file changed, 73 insertions(+), 21 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index e5e04d014..6ed6bcb59 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -49,6 +49,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -113,8 +114,27 @@ public class UploadPack { public static enum RequestPolicy { /** Client may only ask for objects the server advertised a reference for. */ ADVERTISED, - /** Client may ask for any commit reachable from a reference. */ + + /** + * Client may ask for any commit reachable from a reference advertised by + * the server. + */ REACHABLE_COMMIT, + + /** + * Client may ask for objects that are the tip of some reference, even if + * that reference wasn't advertised. + *

+ * This may happen, for example, when a custom {@link RefFilter} is set. + */ + TIP, + + /** + * Client may ask for any commit reachable from any reference, even if that + * reference wasn't advertised. + */ + REACHABLE_COMMIT_TIP, + /** Client may ask for any SHA-1 in the repository. */ ANY; } @@ -363,8 +383,12 @@ public class UploadPack { */ public void setBiDirectionalPipe(final boolean twoWay) { biDirectionalPipe = twoWay; - if (!biDirectionalPipe && requestPolicy == RequestPolicy.ADVERTISED) - requestPolicy = RequestPolicy.REACHABLE_COMMIT; + if (!biDirectionalPipe) { + if (requestPolicy == RequestPolicy.ADVERTISED) + requestPolicy = RequestPolicy.REACHABLE_COMMIT; + else if (requestPolicy == RequestPolicy.TIP) + requestPolicy = RequestPolicy.REACHABLE_COMMIT_TIP; + } } /** @return policy used by the service to validate client requests. */ @@ -378,8 +402,9 @@ public class UploadPack { * By default the policy is {@link RequestPolicy#ADVERTISED}, * which is the Git default requiring clients to only ask for an * object that a reference directly points to. This may be relaxed - * to {@link RequestPolicy#REACHABLE_COMMIT} when callers - * have {@link #setBiDirectionalPipe(boolean)} set to false. + * to {@link RequestPolicy#REACHABLE_COMMIT} or + * {@link RequestPolicy#REACHABLE_COMMIT_TIP} when callers have + * {@link #setBiDirectionalPipe(boolean)} set to false. */ public void setRequestPolicy(RequestPolicy policy) { requestPolicy = policy != null ? policy : RequestPolicy.ADVERTISED; @@ -560,13 +585,8 @@ public class UploadPack { sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut)); else if (requestPolicy == RequestPolicy.ANY) advertised = Collections.emptySet(); - else { - advertised = new HashSet(); - for (Ref ref : getAdvertisedOrDefaultRefs().values()) { - if (ref.getObjectId() != null) - advertised.add(ref.getObjectId()); - } - } + else + advertised = refIdSet(getAdvertisedOrDefaultRefs().values()); boolean sendPack; try { @@ -618,6 +638,15 @@ public class UploadPack { sendPack(); } + private static Set refIdSet(Collection refs) { + Set ids = new HashSet(refs.size()); + for (Ref ref : refs) { + if (ref.getObjectId() != null) + ids.add(ref.getObjectId()); + } + return ids; + } + private void reportErrorDuringNegotiate(String msg) { try { pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ @@ -922,6 +951,8 @@ public class UploadPack { AsyncRevObjectQueue q = walk.parseAny(wantIds, true); try { List checkReachable = null; + Set reachableFrom = null; + Set tips = null; RevObject obj; while ((obj = q.next()) != null) { if (!advertised.contains(obj)) { @@ -935,8 +966,28 @@ public class UploadPack { throw new PackProtocolException(MessageFormat.format( JGitText.get().wantNotValid, obj)); } - if (checkReachable == null) + if (checkReachable == null) { + checkReachable = new ArrayList(); + reachableFrom = advertised; + } + checkReachable.add((RevCommit) obj); + break; + case TIP: + if (tips == null) + tips = refIdSet(db.getAllRefs().values()); + if (!tips.contains(obj)) + throw new PackProtocolException(MessageFormat.format( + JGitText.get().wantNotValid, obj)); + break; + case REACHABLE_COMMIT_TIP: + if (!(obj instanceof RevCommit)) { + throw new PackProtocolException(MessageFormat.format( + JGitText.get().wantNotValid, obj)); + } + if (checkReachable == null) { checkReachable = new ArrayList(); + reachableFrom = refIdSet(db.getAllRefs().values()); + } checkReachable.add((RevCommit) obj); break; case ANY: @@ -954,7 +1005,7 @@ public class UploadPack { } } if (checkReachable != null) - checkNotAdvertisedWants(checkReachable); + checkNotAdvertisedWants(checkReachable, reachableFrom); wantIds.clear(); } catch (MissingObjectException notFound) { ObjectId id = notFound.getObjectId(); @@ -972,17 +1023,18 @@ public class UploadPack { } } - private void checkNotAdvertisedWants(List notAdvertisedWants) + private void checkNotAdvertisedWants(List notAdvertisedWants, + Set reachableFrom) throws MissingObjectException, IncorrectObjectTypeException, IOException { - // Walk the requested commits back to the advertised 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. + // 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. for (RevCommit c : notAdvertisedWants) walk.markStart(c); - for (ObjectId id : advertised) { + for (ObjectId id : reachableFrom) { try { walk.markUninteresting(walk.parseCommit(id)); } catch (IncorrectObjectTypeException notCommit) { From b4b84a84bd38eb88710e45a41fff2a8884dc2eb2 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 18 Jun 2013 15:07:19 -0700 Subject: [PATCH 2/6] UploadPack: advertise allow-tip-sha1-in-want This protocol capability, new in C git 1.8.2, corresponds to RequestPolicy.TIP, so advertise it if that request policy was set. Change-Id: I0d52af8a7747e951a87f060a5124f822ce1b2b26 --- .../eclipse/jgit/transport/BasePackFetchConnection.java | 7 +++++++ .../src/org/eclipse/jgit/transport/UploadPack.java | 9 +++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java index c82a0cde8..1b13d0ba2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java @@ -181,6 +181,13 @@ public abstract class BasePackFetchConnection extends BasePackConnection */ public static final String OPTION_NO_DONE = "no-done"; //$NON-NLS-1$ + /** + * The client supports fetching objects at the tip of any ref, even if not + * advertised. + * @since 3.1 + */ + public static final String OPTION_ALLOW_TIP_SHA1_IN_WANT = "allow-tip-sha1-in-want"; //$NON-NLS-1$ + static enum MultiAck { OFF, CONTINUE, DETAILED; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 6ed6bcb59..7f35eacaf 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -90,6 +90,8 @@ import org.eclipse.jgit.util.io.TimeoutOutputStream; * Implements the server side of a fetch connection, transmitting objects. */ public class UploadPack { + static final String OPTION_ALLOW_TIP_SHA1_IN_WANT = BasePackFetchConnection.OPTION_ALLOW_TIP_SHA1_IN_WANT; + static final String OPTION_INCLUDE_TAG = BasePackFetchConnection.OPTION_INCLUDE_TAG; static final String OPTION_MULTI_ACK = BasePackFetchConnection.OPTION_MULTI_ACK; @@ -122,8 +124,8 @@ public class UploadPack { REACHABLE_COMMIT, /** - * Client may ask for objects that are the tip of some reference, even if - * that reference wasn't advertised. + * Client may ask for objects that are the tip of any reference, even if not + * advertised. *

* This may happen, for example, when a custom {@link RefFilter} is set. */ @@ -722,6 +724,9 @@ public class UploadPack { adv.advertiseCapability(OPTION_SHALLOW); if (!biDirectionalPipe) adv.advertiseCapability(OPTION_NO_DONE); + if (requestPolicy == RequestPolicy.TIP + || requestPolicy == RequestPolicy.REACHABLE_COMMIT_TIP) + adv.advertiseCapability(OPTION_ALLOW_TIP_SHA1_IN_WANT); adv.setDerefTags(true); advertised = adv.send(getAdvertisedOrDefaultRefs()); adv.end(); From e599af19002bfce471a6ad278125c73c07e9d559 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 18 Jun 2013 15:31:17 -0700 Subject: [PATCH 3/6] UploadPack: configure RequestPolicy with TransportConfig C git 1.8.2 supports setting the equivalent of RequestPolicy.TIP with uploadpack.allowtipsha1. Parse this into TransportConfig and use it from UploadPack. An explicitly set RequestPolicy overrides the config, and the policy may still be upgraded on a unidirectional connection to avoid races. Defer figuring out the effective RequestPolicy to later in the process. This is a minor semantic change to fix a bug: previously, calling setRequestPolicy(ADVERTISED) _after_ calling setBiDirectionalPipe(true) would have reintroduced the race condition otherwise fixed by 01888db892aa9590862d886c01f3b293140db153. Change-Id: I264e028a76574434cecb34904d9f5944b290df78 --- .../jgit/transport/TransferConfig.java | 18 +++++++- .../eclipse/jgit/transport/UploadPack.java | 45 +++++++++++++++---- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java index c3e39868d..22888afc0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java @@ -45,9 +45,11 @@ package org.eclipse.jgit.transport; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config.SectionParser; +import org.eclipse.jgit.lib.Repository; /** - * The standard "transfer", "fetch" and "receive" configuration parameters. + * The standard "transfer", "fetch", "receive", and "uploadpack" configuration + * parameters. */ public class TransferConfig { /** Key for {@link Config#get(SectionParser)}. */ @@ -58,9 +60,16 @@ public class TransferConfig { }; private final boolean fsckObjects; + private final boolean allowTipSha1InWant; + + TransferConfig(final Repository db) { + this(db.getConfig()); + } private TransferConfig(final Config rc) { fsckObjects = rc.getBoolean("receive", "fsckobjects", false); //$NON-NLS-1$ //$NON-NLS-2$ + allowTipSha1InWant = + rc.getBoolean("uploadpack", "allowtipsha1inwant", false); //$NON-NLS-1$ //$NON-NLS-2$ } /** @@ -69,4 +78,11 @@ public class TransferConfig { public boolean isFsckObjects() { return fsckObjects; } + + /** + * @return allow clients to request non-advertised tip SHA-1s? + */ + public boolean isAllowTipSha1InWant() { + return allowTipSha1InWant; + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 7f35eacaf..8478dffe4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -188,6 +188,9 @@ public class UploadPack { /** Configuration to pass into the PackWriter. */ private PackConfig packConfig; + /** Configuration for various transfer options. */ + private TransferConfig transferConfig; + /** Timeout in seconds to wait for client interaction. */ private int timeout; @@ -275,7 +278,7 @@ public class UploadPack { private final RevFlagSet SAVE; - private RequestPolicy requestPolicy = RequestPolicy.ADVERTISED; + private RequestPolicy requestPolicy; private MultiAck multiAck = MultiAck.OFF; @@ -307,6 +310,8 @@ public class UploadPack { SAVE.add(PEER_HAS); SAVE.add(COMMON); SAVE.add(SATISFIED); + + transferConfig = new TransferConfig(db); } /** @return the repository this upload is reading from. */ @@ -385,12 +390,6 @@ public class UploadPack { */ public void setBiDirectionalPipe(final boolean twoWay) { biDirectionalPipe = twoWay; - if (!biDirectionalPipe) { - if (requestPolicy == RequestPolicy.ADVERTISED) - requestPolicy = RequestPolicy.REACHABLE_COMMIT; - else if (requestPolicy == RequestPolicy.TIP) - requestPolicy = RequestPolicy.REACHABLE_COMMIT_TIP; - } } /** @return policy used by the service to validate client requests. */ @@ -407,9 +406,10 @@ public class UploadPack { * to {@link RequestPolicy#REACHABLE_COMMIT} or * {@link RequestPolicy#REACHABLE_COMMIT_TIP} when callers have * {@link #setBiDirectionalPipe(boolean)} set to false. + * Overrides any policy specified in a {@link TransferConfig}. */ public void setRequestPolicy(RequestPolicy policy) { - requestPolicy = policy != null ? policy : RequestPolicy.ADVERTISED; + requestPolicy = policy; } /** @return the hook used while advertising the refs to the client */ @@ -479,6 +479,15 @@ public class UploadPack { this.packConfig = pc; } + /** + * @param tc + * configuration controlling transfer options. If null the source + * repository's settings will be used. + */ + public void setTransferConfig(TransferConfig tc) { + this.transferConfig = tc != null ? tc : new TransferConfig(db); + } + /** @return the configured logger. */ public UploadPackLogger getLogger() { return logger; @@ -582,7 +591,27 @@ public class UploadPack { return refs; } + private RequestPolicy getEffectiveRequestPolicy() { + RequestPolicy rp; + if (requestPolicy != null) + rp = requestPolicy; + else if (transferConfig.isAllowTipSha1InWant()) + rp = RequestPolicy.TIP; + else + rp = RequestPolicy.ADVERTISED; + + if (!biDirectionalPipe) { + if (rp == RequestPolicy.ADVERTISED) + rp = RequestPolicy.REACHABLE_COMMIT; + else if (rp == RequestPolicy.TIP) + rp = RequestPolicy.REACHABLE_COMMIT_TIP; + } + return rp; + } + private void service() throws IOException { + requestPolicy = getEffectiveRequestPolicy(); + if (biDirectionalPipe) sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut)); else if (requestPolicy == RequestPolicy.ANY) From e74751769e7be0901d1190e789bf360512ff3213 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 18 Jun 2013 16:41:58 -0700 Subject: [PATCH 4/6] UploadPack: set RefFilter from TransportConfig Teach TransportConfig to respect uploadpack.hiderefs, which is new in C git 1.8.2. Change-Id: Id12e7f00b9a60258e996410f67fa10616459f53f --- .../jgit/transport/TransferConfig.java | 40 ++++++++++++++++++- .../eclipse/jgit/transport/UploadPack.java | 8 +++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java index 22888afc0..1286718de 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java @@ -43,8 +43,12 @@ package org.eclipse.jgit.transport; +import java.util.HashMap; +import java.util.Map; + import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config.SectionParser; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; /** @@ -61,6 +65,7 @@ public class TransferConfig { private final boolean fsckObjects; private final boolean allowTipSha1InWant; + private final String[] hideRefs; TransferConfig(final Repository db) { this(db.getConfig()); @@ -68,8 +73,9 @@ public class TransferConfig { private TransferConfig(final Config rc) { fsckObjects = rc.getBoolean("receive", "fsckobjects", false); //$NON-NLS-1$ //$NON-NLS-2$ - allowTipSha1InWant = - rc.getBoolean("uploadpack", "allowtipsha1inwant", false); //$NON-NLS-1$ //$NON-NLS-2$ + allowTipSha1InWant = rc.getBoolean( + "uploadpack", "allowtipsha1inwant", false); //$NON-NLS-1$ //$NON-NLS-2$ + hideRefs = rc.getStringList("uploadpack", null, "hiderefs"); //$NON-NLS-1$ //$NON-NLS-2$ } /** @@ -85,4 +91,34 @@ public class TransferConfig { public boolean isAllowTipSha1InWant() { return allowTipSha1InWant; } + + /** + * @return {@link RefFilter} respecting configured hidden refs. + */ + public RefFilter getRefFilter() { + if (hideRefs.length == 0) + return RefFilter.DEFAULT; + + return new RefFilter() { + public Map filter(Map refs) { + Map result = new HashMap(); + for (Map.Entry e : refs.entrySet()) { + boolean add = true; + for (String hide : hideRefs) { + if (e.getKey().equals(hide) || prefixMatch(hide, e.getKey())) { + add = false; + break; + } + } + if (add) + result.put(e.getKey(), e.getValue()); + } + return result; + } + + private boolean prefixMatch(String p, String s) { + return p.charAt(p.length() - 1) == '/' && s.startsWith(p); + } + }; + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 8478dffe4..f1028a0f8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -351,7 +351,10 @@ public class UploadPack { refs = allRefs; else refs = db.getAllRefs(); - refs = refFilter.filter(refs); + if (refFilter == RefFilter.DEFAULT) + refs = transferConfig.getRefFilter().filter(refs); + else + refs = refFilter.filter(refs); } /** @return timeout (in seconds) before aborting an IO operation. */ @@ -444,7 +447,8 @@ public class UploadPack { *

* Only refs allowed by this filter will be sent to the client. * The filter is run against the refs specified by the - * {@link AdvertiseRefsHook} (if applicable). + * {@link AdvertiseRefsHook} (if applicable). If null or not set, uses the + * filter implied by the {@link TransferConfig}. * * @param refFilter * the filter; may be null to show all refs. From 68ccc5d2137322118ed2a55f355e014a3670293a Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Sun, 23 Jun 2013 17:02:29 -0700 Subject: [PATCH 5/6] UploadPack: refactor want validation Associate each RequestPolicy with an implementation of a RequestValidator interface that contains the validation logic. The checkWants method is only called if there are wants that were not advertised, since clients may always request any advertised want according to the git protocol. Calling the method only once at the end of parsing the want list also means policy implementations can be stateful, unlike the previous switch statement inside a loop. For the special handling of unidirectional pipes, simply check isBiDirectional() and delegate to other implementations if necessary. Change-Id: I52a174999ac3a5aca46d3469cb0b81edd1710580 --- .../eclipse/jgit/transport/UploadPack.java | 172 +++++++++++------- 1 file changed, 105 insertions(+), 67 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index f1028a0f8..43f854e17 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -141,6 +141,25 @@ public class UploadPack { ANY; } + /** Validator for client requests. */ + private interface RequestValidator { + /** + * Check a list of client wants against the request policy. + * + * @param up + * {@link UploadPack} instance. + * @param wants + * objects the client requested that were not advertised. + * + * @throws PackProtocolException + * if one or more wants is not valid. + * @throws IOException + * if a low-level exception occurred. + */ + void checkWants(UploadPack up, List wants) + throws PackProtocolException, IOException; + } + /** Data in the first line of a request, the line itself plus options. */ public static class FirstLine { private final String line; @@ -311,7 +330,7 @@ public class UploadPack { SAVE.add(COMMON); SAVE.add(SATISFIED); - transferConfig = new TransferConfig(db); + setTransferConfig(null); } /** @return the repository this upload is reading from. */ @@ -490,6 +509,8 @@ public class UploadPack { */ public void setTransferConfig(TransferConfig tc) { this.transferConfig = tc != null ? tc : new TransferConfig(db); + this.requestPolicy = transferConfig.isAllowTipSha1InWant() + ? RequestPolicy.TIP : RequestPolicy.ADVERTISED; } /** @return the configured logger. */ @@ -595,27 +616,7 @@ public class UploadPack { return refs; } - private RequestPolicy getEffectiveRequestPolicy() { - RequestPolicy rp; - if (requestPolicy != null) - rp = requestPolicy; - else if (transferConfig.isAllowTipSha1InWant()) - rp = RequestPolicy.TIP; - else - rp = RequestPolicy.ADVERTISED; - - if (!biDirectionalPipe) { - if (rp == RequestPolicy.ADVERTISED) - rp = RequestPolicy.REACHABLE_COMMIT; - else if (rp == RequestPolicy.TIP) - rp = RequestPolicy.REACHABLE_COMMIT_TIP; - } - return rp; - } - private void service() throws IOException { - requestPolicy = getEffectiveRequestPolicy(); - if (biDirectionalPipe) sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut)); else if (requestPolicy == RequestPolicy.ANY) @@ -988,49 +989,13 @@ public class UploadPack { private void parseWants() throws IOException { AsyncRevObjectQueue q = walk.parseAny(wantIds, true); try { - List checkReachable = null; - Set reachableFrom = null; - Set tips = null; + List notAdvertisedWants = null; RevObject obj; while ((obj = q.next()) != null) { if (!advertised.contains(obj)) { - switch (requestPolicy) { - case ADVERTISED: - default: - throw new PackProtocolException(MessageFormat.format( - JGitText.get().wantNotValid, obj)); - case REACHABLE_COMMIT: - if (!(obj instanceof RevCommit)) { - throw new PackProtocolException(MessageFormat.format( - JGitText.get().wantNotValid, obj)); - } - if (checkReachable == null) { - checkReachable = new ArrayList(); - reachableFrom = advertised; - } - checkReachable.add((RevCommit) obj); - break; - case TIP: - if (tips == null) - tips = refIdSet(db.getAllRefs().values()); - if (!tips.contains(obj)) - throw new PackProtocolException(MessageFormat.format( - JGitText.get().wantNotValid, obj)); - break; - case REACHABLE_COMMIT_TIP: - if (!(obj instanceof RevCommit)) { - throw new PackProtocolException(MessageFormat.format( - JGitText.get().wantNotValid, obj)); - } - if (checkReachable == null) { - checkReachable = new ArrayList(); - reachableFrom = refIdSet(db.getAllRefs().values()); - } - checkReachable.add((RevCommit) obj); - break; - case ANY: - break; - } + if (notAdvertisedWants == null) + notAdvertisedWants = new ArrayList(); + notAdvertisedWants.add(obj); } want(obj); @@ -1042,8 +1007,8 @@ public class UploadPack { want(obj); } } - if (checkReachable != null) - checkNotAdvertisedWants(checkReachable, reachableFrom); + if (notAdvertisedWants != null) + getRequestValidator().checkWants(this, notAdvertisedWants); wantIds.clear(); } catch (MissingObjectException notFound) { ObjectId id = notFound.getObjectId(); @@ -1061,8 +1026,77 @@ public class UploadPack { } } - private void checkNotAdvertisedWants(List notAdvertisedWants, - Set reachableFrom) + private RequestValidator getRequestValidator() { + switch (requestPolicy) { + case ADVERTISED: + default: + return new AdvertisedRequestValidator(); + case REACHABLE_COMMIT: + return new ReachableCommitRequestValidator(); + case TIP: + return new TipRequestValidator(); + case REACHABLE_COMMIT_TIP: + return new ReachableCommitTipRequestValidator(); + case ANY: + return new AnyRequestValidator(); + } + } + + private static class AdvertisedRequestValidator implements RequestValidator { + public void checkWants(UploadPack up, List wants) + throws PackProtocolException, IOException { + if (!up.isBiDirectionalPipe()) + new ReachableCommitRequestValidator().checkWants(up, wants); + else if (!wants.isEmpty()) + throw new PackProtocolException(MessageFormat.format( + JGitText.get().wantNotValid, wants.iterator().next().name())); + } + } + + private static class ReachableCommitRequestValidator + implements RequestValidator { + public void checkWants(UploadPack up, List wants) + throws PackProtocolException, IOException { + checkNotAdvertisedWants(up.getRevWalk(), wants, + refIdSet(up.getAdvertisedRefs().values())); + } + } + + private static class TipRequestValidator implements RequestValidator { + public void checkWants(UploadPack up, List wants) + throws PackProtocolException, IOException { + if (!up.isBiDirectionalPipe()) + new ReachableCommitTipRequestValidator().checkWants(up, wants); + else if (!wants.isEmpty()) { + Set refIds = + refIdSet(up.getRepository().getAllRefs().values()); + for (RevObject obj : wants) { + if (!refIds.contains(obj)) + throw new PackProtocolException(MessageFormat.format( + JGitText.get().wantNotValid, obj.name())); + } + } + } + } + + private static class ReachableCommitTipRequestValidator + implements RequestValidator { + public void checkWants(UploadPack up, List wants) + throws PackProtocolException, IOException { + checkNotAdvertisedWants(up.getRevWalk(), wants, + refIdSet(up.getRepository().getAllRefs().values())); + } + } + + private static class AnyRequestValidator implements RequestValidator { + public void checkWants(UploadPack up, List wants) + throws PackProtocolException, IOException { + // All requests are valid. + } + } + + private static void checkNotAdvertisedWants(RevWalk walk, + List notAdvertisedWants, Set reachableFrom) throws MissingObjectException, IncorrectObjectTypeException, 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 @@ -1070,8 +1104,12 @@ public class UploadPack { // into an advertised branch it will be marked UNINTERESTING and no commits // return. - for (RevCommit c : notAdvertisedWants) - walk.markStart(c); + for (RevObject obj : notAdvertisedWants) { + if (!(obj instanceof RevCommit)) + throw new PackProtocolException(MessageFormat.format( + JGitText.get().wantNotValid, obj.name())); + walk.markStart((RevCommit) obj); + } for (ObjectId id : reachableFrom) { try { walk.markUninteresting(walk.parseCommit(id)); From 360d8624ca42ed13ec99fb5f30c887118e581080 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Sun, 23 Jun 2013 17:28:34 -0700 Subject: [PATCH 6/6] UploadPack: allow custom RequestValidator instances Make the existing concrete implementations public as well so custom implementations may delegate to them where appropriate. Treat all custom implementations as providing allow-tip-sha1 in want. Change-Id: If386fe25c0d3b4551a97c16a22350714453b03e9 --- .../eclipse/jgit/transport/UploadPack.java | 123 +++++++++++++----- 1 file changed, 90 insertions(+), 33 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 43f854e17..0d23cf792 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -141,8 +141,12 @@ public class UploadPack { ANY; } - /** Validator for client requests. */ - private interface RequestValidator { + /** + * Validator for client requests. + * + * @since 3.1 + */ + public interface RequestValidator { /** * Check a list of client wants against the request policy. * @@ -155,6 +159,7 @@ public class UploadPack { * if one or more wants is not valid. * @throws IOException * if a low-level exception occurred. + * @since 3.1 */ void checkWants(UploadPack up, List wants) throws PackProtocolException, IOException; @@ -297,7 +302,7 @@ public class UploadPack { private final RevFlagSet SAVE; - private RequestPolicy requestPolicy; + private RequestValidator requestValidator = new AdvertisedRequestValidator(); private MultiAck multiAck = MultiAck.OFF; @@ -414,9 +419,22 @@ public class UploadPack { biDirectionalPipe = twoWay; } - /** @return policy used by the service to validate client requests. */ + /** + * @return policy used by the service to validate client requests, or null for + * a custom request validator. + */ public RequestPolicy getRequestPolicy() { - return requestPolicy; + if (requestValidator instanceof AdvertisedRequestValidator) + return RequestPolicy.ADVERTISED; + if (requestValidator instanceof ReachableCommitRequestValidator) + return RequestPolicy.REACHABLE_COMMIT; + if (requestValidator instanceof TipRequestValidator) + return RequestPolicy.TIP; + if (requestValidator instanceof ReachableCommitTipRequestValidator) + return RequestPolicy.REACHABLE_COMMIT_TIP; + if (requestValidator instanceof AnyRequestValidator) + return RequestPolicy.ANY; + return null; } /** @@ -431,7 +449,34 @@ public class UploadPack { * Overrides any policy specified in a {@link TransferConfig}. */ public void setRequestPolicy(RequestPolicy policy) { - requestPolicy = policy; + switch (policy) { + case ADVERTISED: + default: + requestValidator = new AdvertisedRequestValidator(); + break; + case REACHABLE_COMMIT: + requestValidator = new ReachableCommitRequestValidator(); + break; + case TIP: + requestValidator = new TipRequestValidator(); + break; + case REACHABLE_COMMIT_TIP: + requestValidator = new ReachableCommitTipRequestValidator(); + break; + case ANY: + requestValidator = new AnyRequestValidator(); + break; + } + } + + /** + * @param validator + * custom validator for client want list. + * @since 3.1 + */ + public void setRequestValidator(RequestValidator validator) { + requestValidator = validator != null ? validator + : new AdvertisedRequestValidator(); } /** @return the hook used while advertising the refs to the client */ @@ -509,8 +554,8 @@ public class UploadPack { */ public void setTransferConfig(TransferConfig tc) { this.transferConfig = tc != null ? tc : new TransferConfig(db); - this.requestPolicy = transferConfig.isAllowTipSha1InWant() - ? RequestPolicy.TIP : RequestPolicy.ADVERTISED; + setRequestPolicy(transferConfig.isAllowTipSha1InWant() + ? RequestPolicy.TIP : RequestPolicy.ADVERTISED); } /** @return the configured logger. */ @@ -619,7 +664,7 @@ public class UploadPack { private void service() throws IOException { if (biDirectionalPipe) sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut)); - else if (requestPolicy == RequestPolicy.ANY) + else if (requestValidator instanceof AnyRequestValidator) advertised = Collections.emptySet(); else advertised = refIdSet(getAdvertisedOrDefaultRefs().values()); @@ -758,8 +803,10 @@ public class UploadPack { adv.advertiseCapability(OPTION_SHALLOW); if (!biDirectionalPipe) adv.advertiseCapability(OPTION_NO_DONE); - if (requestPolicy == RequestPolicy.TIP - || requestPolicy == RequestPolicy.REACHABLE_COMMIT_TIP) + RequestPolicy policy = getRequestPolicy(); + if (policy == RequestPolicy.TIP + || policy == RequestPolicy.REACHABLE_COMMIT_TIP + || policy == null) adv.advertiseCapability(OPTION_ALLOW_TIP_SHA1_IN_WANT); adv.setDerefTags(true); advertised = adv.send(getAdvertisedOrDefaultRefs()); @@ -1008,7 +1055,7 @@ public class UploadPack { } } if (notAdvertisedWants != null) - getRequestValidator().checkWants(this, notAdvertisedWants); + requestValidator.checkWants(this, notAdvertisedWants); wantIds.clear(); } catch (MissingObjectException notFound) { ObjectId id = notFound.getObjectId(); @@ -1026,23 +1073,13 @@ public class UploadPack { } } - private RequestValidator getRequestValidator() { - switch (requestPolicy) { - case ADVERTISED: - default: - return new AdvertisedRequestValidator(); - case REACHABLE_COMMIT: - return new ReachableCommitRequestValidator(); - case TIP: - return new TipRequestValidator(); - case REACHABLE_COMMIT_TIP: - return new ReachableCommitTipRequestValidator(); - case ANY: - return new AnyRequestValidator(); - } - } - - private static class AdvertisedRequestValidator implements RequestValidator { + /** + * Validator corresponding to {@link RequestPolicy#ADVERTISED}. + * + * @since 3.1 + */ + public static final class AdvertisedRequestValidator + implements RequestValidator { public void checkWants(UploadPack up, List wants) throws PackProtocolException, IOException { if (!up.isBiDirectionalPipe()) @@ -1053,7 +1090,12 @@ public class UploadPack { } } - private static class ReachableCommitRequestValidator + /** + * Validator corresponding to {@link RequestPolicy#REACHABLE_COMMIT}. + * + * @since 3.1 + */ + public static final class ReachableCommitRequestValidator implements RequestValidator { public void checkWants(UploadPack up, List wants) throws PackProtocolException, IOException { @@ -1062,7 +1104,12 @@ public class UploadPack { } } - private static class TipRequestValidator implements RequestValidator { + /** + * Validator corresponding to {@link RequestPolicy#TIP}. + * + * @since 3.1 + */ + public static final class TipRequestValidator implements RequestValidator { public void checkWants(UploadPack up, List wants) throws PackProtocolException, IOException { if (!up.isBiDirectionalPipe()) @@ -1079,7 +1126,12 @@ public class UploadPack { } } - private static class ReachableCommitTipRequestValidator + /** + * Validator corresponding to {@link RequestPolicy#REACHABLE_COMMIT_TIP}. + * + * @since 3.1 + */ + public static final class ReachableCommitTipRequestValidator implements RequestValidator { public void checkWants(UploadPack up, List wants) throws PackProtocolException, IOException { @@ -1088,7 +1140,12 @@ public class UploadPack { } } - private static class AnyRequestValidator implements RequestValidator { + /** + * Validator corresponding to {@link RequestPolicy#ANY}. + * + * @since 3.1 + */ + public static final class AnyRequestValidator implements RequestValidator { public void checkWants(UploadPack up, List wants) throws PackProtocolException, IOException { // All requests are valid.