Skip to main content
summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMasaya Suzuki2018-12-18 17:20:54 +0000
committerMatthias Sohn2018-12-25 22:36:11 +0000
commit8eecb4f8b746bc01f09df02870e89d4bc4e118b9 (patch)
tree61d9a1c133d27d7342ce5b375ffb75202147035f
parent9caa94239aa15f38402344176fa32c09a2bd121c (diff)
downloadjgit-8eecb4f8b746bc01f09df02870e89d4bc4e118b9.tar.gz
jgit-8eecb4f8b746bc01f09df02870e89d4bc4e118b9.tar.xz
jgit-8eecb4f8b746bc01f09df02870e89d4bc4e118b9.zip
Call AdvertiseRefsHook for protocol v2
AdvertiseRefsHook is used to limit the visibility of the refs in Gerrit. If this hook is not called, then all refs are treated as visible. In protocol v2, the hook is not called, causing the server to advertise all refs. This bug was introduced in v5.0.0.201805221745-rc1~1^2~9 (Execute AdvertiseRefsHook only for protocol v0 and v1, 2018-05-14). Even before then, the hook was not called in requests after the capability advertisement, so in transports like HTTP that do not retain state between round-trips, the server would advertise all refs in response to an ls-refs (ls-remote) request. Fix both cases by using getAdvertisedOrDefaultRefs to retrieve the advertised refs in lsRefs, ensuring the hook is called in all cases that use its result. [jn: backported to stable-5.0; split out from a larger patch that also fixes protocol v0; avoided filtering this.refs by ref prefix] Change-Id: I64bce0e72d15b90baccc235c067e57b6af21b55f Signed-off-by: Masaya Suzuki <masayasuzuki@google.com> Signed-off-by: Jonathan Nieder <jrn@google.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java60
1 files changed, 46 insertions, 14 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 3255a71b3e..1cdf8f7f1b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -43,8 +43,9 @@
package org.eclipse.jgit.transport;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
import static org.eclipse.jgit.lib.Constants.R_TAGS;
-import static org.eclipse.jgit.lib.RefDatabase.ALL;
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.OPTION_AGENT;
@@ -261,12 +262,18 @@ public class UploadPack {
private OutputStream msgOut = NullOutputStream.INSTANCE;
- /** The refs we advertised as existing at the start of the connection. */
+ /**
+ * Refs eligible for advertising to the client, set using
+ * {@link #setAdvertisedRefs}.
+ */
private Map<String, Ref> refs;
/** Hook used while advertising the refs to the client. */
private AdvertiseRefsHook advertiseRefsHook = AdvertiseRefsHook.DEFAULT;
+ /** Whether the {@link #advertiseRefsHook} has been invoked. */
+ private boolean advertiseRefsHookCalled;
+
/** Filter used while advertising the refs to the client. */
private RefFilter refFilter = RefFilter.DEFAULT;
@@ -784,12 +791,47 @@ public class UploadPack {
}
advertiseRefsHook.advertiseRefs(this);
+ advertiseRefsHookCalled = true;
if (refs == null) {
- setAdvertisedRefs(db.getRefDatabase().getRefs(ALL));
+ // Fall back to all refs.
+ setAdvertisedRefs(
+ db.getRefDatabase().getRefs().stream()
+ .collect(toMap(Ref::getName, identity())));
}
return refs;
}
+ private Map<String, Ref> getFilteredRefs(Collection<String> refPrefixes)
+ throws IOException {
+ if (refPrefixes.isEmpty()) {
+ return getAdvertisedOrDefaultRefs();
+ }
+ if (refs == null && !advertiseRefsHookCalled) {
+ advertiseRefsHook.advertiseRefs(this);
+ advertiseRefsHookCalled = true;
+ }
+ if (refs == null) {
+ // Fast path: the advertised refs hook did not set advertised refs.
+ Map<String, Ref> rs = new HashMap<>();
+ for (String p : refPrefixes) {
+ for (Ref r : db.getRefDatabase().getRefsByPrefix(p)) {
+ rs.put(r.getName(), r);
+ }
+ }
+ if (refFilter != RefFilter.DEFAULT) {
+ return refFilter.filter(rs);
+ }
+ return transferConfig.getRefFilter().filter(rs);
+ }
+
+ // Slow path: filter the refs provided by the advertised refs hook.
+ // refFilter has already been applied to refs.
+ return refs.values().stream()
+ .filter(ref -> refPrefixes.stream()
+ .anyMatch(ref.getName()::startsWith))
+ .collect(toMap(Ref::getName, identity()));
+ }
+
private void service() throws IOException {
boolean sendPack = false;
// If it's a non-bidi request, we need to read the entire request before
@@ -913,17 +955,7 @@ public class UploadPack {
}
rawOut.stopBuffering();
- Map<String, Ref> refsToSend;
- if (refPrefixes.isEmpty()) {
- refsToSend = getAdvertisedOrDefaultRefs();
- } else {
- refsToSend = new HashMap<>();
- for (String refPrefix : refPrefixes) {
- for (Ref ref : db.getRefDatabase().getRefsByPrefix(refPrefix)) {
- refsToSend.put(ref.getName(), ref);
- }
- }
- }
+ Map<String, Ref> refsToSend = getFilteredRefs(refPrefixes);
if (needToFindSymrefs) {
findSymrefs(adv, refsToSend);

Back to the top