From e599af19002bfce471a6ad278125c73c07e9d559 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 18 Jun 2013 15:31:17 -0700 Subject: [PATCH] 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 @@ 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 TransferConfig parse(final Config cfg) { }; 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 @@ private TransferConfig(final Config rc) { 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 Set getOptions() { /** 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 Set getOptions() { private final RevFlagSet SAVE; - private RequestPolicy requestPolicy = RequestPolicy.ADVERTISED; + private RequestPolicy requestPolicy; private MultiAck multiAck = MultiAck.OFF; @@ -307,6 +310,8 @@ public UploadPack(final Repository copyFrom) { 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 boolean isBiDirectionalPipe() { */ 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 RequestPolicy getRequestPolicy() { * 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 void setPackConfig(PackConfig pc) { 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 @@ private Map getAdvertisedOrDefaultRefs() { 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)