Merge branch 'stable-5.2' * stable-5.2: Prepare 5.2.2-SNAPSHOT builds JGit v5.2.1.201812262042-r Prepare 5.1.6-SNAPSHOT builds JGit v5.1.5.201812261915-r UploadPack: Filter refs used for deepen-not resolution UploadPack: Avoid calling AdvertiseRefsHook twice Prepare 5.1.5-SNAPSHOT builds JGit v5.1.4.201812251853-r UploadPack: Filter refs used for want-ref resolution UploadPack: Defer want-ref resolution to after parsing Call AdvertiseRefsHook for protocol v2 Prepare 4.11.7-SNAPSHOT builds JGit v4.11.6.201812241910-r Prepare 4.9.9-SNAPSHOT builds JGit v4.9.8.201812241815-r UploadPack: Test filtering by AdvertiseRefsHook in stateless transports Prepare 4.7.8-SNAPSHOT builds JGit v4.7.7.201812240805-r Fix feature versions imported by feature org.eclipse.jgit.pgm Prepare 4.5.6-SNAPSHOT builds JGit v4.5.5.201812240535-r Call AdvertiseRefsHook before validating wants Change-Id: Ia56348e54d62630d7c50a4747df89516fc5afad9 Signed-off-by: Jonathan Nieder <jrn@google.com>
diff --git a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF index e4c7986..15e9ac4 100644 --- a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF
@@ -50,5 +50,7 @@ org.hamcrest;version="[1.1.0,2.0.0)", org.hamcrest.core;version="[1.1.0,2.0.0)", org.junit;version="[4.12,5.0.0)", + org.junit.rules;version="[4.12,5.0.0)", org.junit.runner;version="[4.12,5.0.0)", org.junit.runners;version="[4.12,5.0.0)" +Require-Bundle: org.hamcrest.library;bundle-version="[1.1.0,2.0.0)"
diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java index 7ff7e5c..ecab61e 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
@@ -85,6 +85,7 @@ import org.eclipse.jgit.errors.TransportException; import org.eclipse.jgit.errors.UnsupportedCredentialItem; import org.eclipse.jgit.http.server.GitServlet; +import org.eclipse.jgit.http.server.resolver.DefaultUploadPackFactory; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.junit.TestRepository; @@ -105,24 +106,35 @@ import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook; +import org.eclipse.jgit.transport.AdvertiseRefsHook; import org.eclipse.jgit.transport.CredentialItem; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.FetchConnection; import org.eclipse.jgit.transport.HttpTransport; +import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.transport.RemoteRefUpdate; import org.eclipse.jgit.transport.Transport; import org.eclipse.jgit.transport.TransportHttp; import org.eclipse.jgit.transport.URIish; +import org.eclipse.jgit.transport.UploadPack; import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; import org.eclipse.jgit.transport.http.HttpConnectionFactory; import org.eclipse.jgit.transport.http.JDKHttpConnectionFactory; import org.eclipse.jgit.transport.http.apache.HttpClientConnectionFactory; +import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; +import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; +import org.eclipse.jgit.transport.resolver.UploadPackFactory; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.HttpSupport; import org.eclipse.jgit.util.SystemReader; +import org.hamcrest.Matchers; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; @@ -131,6 +143,11 @@ public class SmartClientSmartServerTest extends HttpTestCase { private static final String HDR_TRANSFER_ENCODING = "Transfer-Encoding"; + @Rule + public ExpectedException thrown = ExpectedException.none(); + + private AdvertiseRefsHook advertiseRefsHook; + private Repository remoteRepository; private CredentialsProvider testCredentials = new UsernamePasswordCredentialsProvider( @@ -148,7 +165,7 @@ public class SmartClientSmartServerTest extends HttpTestCase { private RevBlob A_txt; - private RevCommit A, B; + private RevCommit A, B, unreachableCommit; @Parameters public static Collection<Object[]> data() { @@ -175,6 +192,19 @@ public void setUp() throws Exception { ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, true); GitServlet gs = new GitServlet(); + gs.setUploadPackFactory(new UploadPackFactory<HttpServletRequest>() { + @Override + public UploadPack create(HttpServletRequest req, Repository db) + throws ServiceNotEnabledException, + ServiceNotAuthorizedException { + DefaultUploadPackFactory f = new DefaultUploadPackFactory(); + UploadPack up = f.create(req, db); + if (advertiseRefsHook != null) { + up.setAdvertiseRefsHook(advertiseRefsHook); + } + return up; + } + }); ServletContextHandler app = addNormalContext(gs, src, srcName); @@ -200,6 +230,8 @@ public void setUp() throws Exception { B = src.commit().parent(A).add("A_txt", "C").add("B", "B").create(); src.update(master, B); + unreachableCommit = src.commit().add("A_txt", A_txt).create(); + src.update("refs/garbage/a/very/long/ref/name/to/compress", B); } @@ -453,6 +485,56 @@ public void testListRemote_BadName() throws IOException, URISyntaxException { } @Test + public void testFetchBySHA1() throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + try (Transport t = Transport.open(dst, remoteURI)) { + t.fetch(NullProgressMonitor.INSTANCE, + Collections.singletonList(new RefSpec(B.name()))); + } + + assertTrue(dst.hasObject(A_txt)); + } + + @Test + public void testFetchBySHA1Unreachable() throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + try (Transport t = Transport.open(dst, remoteURI)) { + thrown.expect(TransportException.class); + thrown.expectMessage(Matchers.containsString( + "want " + unreachableCommit.name() + " not valid")); + t.fetch(NullProgressMonitor.INSTANCE, Collections + .singletonList(new RefSpec(unreachableCommit.name()))); + } + } + + @Test + public void testFetchBySHA1UnreachableByAdvertiseRefsHook() + throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + advertiseRefsHook = new AbstractAdvertiseRefsHook() { + @Override + protected Map<String, Ref> getAdvertisedRefs(Repository repository, + RevWalk revWalk) { + return Collections.emptyMap(); + } + }; + + try (Transport t = Transport.open(dst, remoteURI)) { + thrown.expect(TransportException.class); + thrown.expectMessage(Matchers.containsString( + "want " + A.name() + " not valid")); + t.fetch(NullProgressMonitor.INSTANCE, Collections + .singletonList(new RefSpec(A.name()))); + } + } + + @Test public void testInitialClone_Small() throws Exception { Repository dst = createBareRepository(); assertFalse(dst.getObjectDatabase().has(A_txt));
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java index 6b1cbdd..dafa81e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ProtocolV2ParserTest.java
@@ -42,7 +42,6 @@ */ package org.eclipse.jgit.transport; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasItems; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -152,8 +151,7 @@ public void testFetchBasicArguments() PacketLineIn.END); ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.getDefault()); - FetchV2Request request = parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + FetchV2Request request = parser.parseFetchRequest(pckIn); assertTrue(request.getClientCapabilities() .contains(GitProtocolConstants.OPTION_THIN_PACK)); assertTrue(request.getClientCapabilities() @@ -183,8 +181,7 @@ public void testFetchWithShallow_deepen() throws IOException { PacketLineIn.END); ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.getDefault()); - FetchV2Request request = parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + FetchV2Request request = parser.parseFetchRequest(pckIn); assertThat(request.getClientShallowCommits(), hasOnlyObjectIds("28274d02c489f4c7e68153056e9061a46f62d7a0", "145e683b229dcab9d0e2ccb01b386f9ecc17d29d")); @@ -203,8 +200,7 @@ public void testFetchWithShallow_deepenNot() throws IOException { PacketLineIn.END); ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.getDefault()); - FetchV2Request request = parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + FetchV2Request request = parser.parseFetchRequest(pckIn); assertThat(request.getClientShallowCommits(), hasOnlyObjectIds("28274d02c489f4c7e68153056e9061a46f62d7a0", "145e683b229dcab9d0e2ccb01b386f9ecc17d29d")); @@ -221,8 +217,7 @@ public void testFetchWithShallow_deepenSince() throws IOException { PacketLineIn.END); ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.getDefault()); - FetchV2Request request = parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + FetchV2Request request = parser.parseFetchRequest(pckIn); assertThat(request.getClientShallowCommits(), hasOnlyObjectIds("28274d02c489f4c7e68153056e9061a46f62d7a0", "145e683b229dcab9d0e2ccb01b386f9ecc17d29d")); @@ -236,8 +231,7 @@ public void testFetchWithNoneFilter() throws IOException { PacketLineIn.END); ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.start().allowFilter().done()); - FetchV2Request request = parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + FetchV2Request request = parser.parseFetchRequest(pckIn); assertEquals(0, request.getFilterBlobLimit()); } @@ -248,8 +242,7 @@ public void testFetchWithBlobSizeFilter() throws IOException { PacketLineIn.END); ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.start().allowFilter().done()); - FetchV2Request request = parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + FetchV2Request request = parser.parseFetchRequest(pckIn); assertEquals(15, request.getFilterBlobLimit()); } @@ -263,8 +256,7 @@ public void testFetchMustNotHaveMultipleFilters() throws IOException { ConfigBuilder.start().allowFilter().done()); thrown.expect(PackProtocolException.class); - parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + parser.parseFetchRequest(pckIn); } @Test @@ -275,8 +267,7 @@ public void testFetchFilterWithoutAllowFilter() throws IOException { ConfigBuilder.getDefault()); thrown.expect(PackProtocolException.class); - parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + parser.parseFetchRequest(pckIn); } @Test @@ -293,16 +284,13 @@ public void testFetchWithRefInWant() throws Exception { ProtocolV2Parser parser = new ProtocolV2Parser( ConfigBuilder.start().allowRefInWant().done()); - - FetchV2Request request = parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + FetchV2Request request = parser.parseFetchRequest(pckIn); assertEquals(1, request.getWantedRefs().size()); - assertThat(request.getWantedRefs().keySet(), + assertThat(request.getWantedRefs(), hasItems("refs/heads/branchA")); - assertEquals(2, request.getWantIds().size()); + assertEquals(1, request.getWantIds().size()); assertThat(request.getWantIds(), hasOnlyObjectIds( - "e4980cdc48cfa1301493ca94eb70523f6788b819", - one.getName())); + "e4980cdc48cfa1301493ca94eb70523f6788b819")); } @Test @@ -319,10 +307,9 @@ public void testFetchWithRefInWantUnknownRef() throws Exception { testRepo.update("branchA", one); testRepo.update("branchB", two); - thrown.expect(PackProtocolException.class); - thrown.expectMessage(containsString("refs/heads/branchC")); - parser.parseFetchRequest(pckIn, - testRepo.getRepository().getRefDatabase()); + FetchV2Request request = parser.parseFetchRequest(pckIn); + assertEquals(1, request.getWantedRefs().size()); + assertThat(request.getWantedRefs(), hasItems("refs/heads/branchC")); } @Test
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java index 8e36a10..ac6361c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java
@@ -48,9 +48,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.TreeMap; import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.Nullable; @@ -67,7 +65,7 @@ public final class FetchV2Request extends FetchRequest { private final List<ObjectId> peerHas; - private final TreeMap<String, ObjectId> wantedRefs; + private final List<String> wantedRefs; private final boolean doneReceived; @@ -75,7 +73,7 @@ public final class FetchV2Request extends FetchRequest { private final List<String> serverOptions; FetchV2Request(@NonNull List<ObjectId> peerHas, - @NonNull TreeMap<String, ObjectId> wantedRefs, + @NonNull List<String> wantedRefs, @NonNull Set<ObjectId> wantIds, @NonNull Set<ObjectId> clientShallowCommits, int deepenSince, @NonNull List<String> deepenNotRefs, int depth, @@ -102,7 +100,7 @@ List<ObjectId> getPeerHas() { * @return list of references received in "want-ref" lines */ @NonNull - Map<String, ObjectId> getWantedRefs() { + List<String> getWantedRefs() { return wantedRefs; } @@ -135,7 +133,7 @@ static Builder builder() { static final class Builder { final List<ObjectId> peerHas = new ArrayList<>(); - final TreeMap<String, ObjectId> wantedRefs = new TreeMap<>(); + final List<String> wantedRefs = new ArrayList<>(); final Set<ObjectId> wantIds = new HashSet<>(); @@ -176,12 +174,10 @@ Builder addPeerHas(ObjectId objectId) { * * @param refName * reference name - * @param oid - * object id the reference is pointing at * @return this builder */ - Builder addWantedRef(String refName, ObjectId oid) { - wantedRefs.put(refName, oid); + Builder addWantedRef(String refName) { + wantedRefs.add(refName); return this; }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java index a03f021..8f4b86e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java
@@ -62,8 +62,6 @@ import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.RefDatabase; /** * Parse the incoming git protocol lines from the wire and translate them into a @@ -113,24 +111,17 @@ private static String consumeCapabilities(PacketLineIn pckIn, * Parse the incoming fetch request arguments from the wire. The caller must * be sure that what is comings is a fetch request before coming here. * - * This operation requires the reference database to validate incoming - * references. - * * @param pckIn * incoming lines - * @param refdb - * reference database (to validate that received references exist - * and point to valid objects) * @return A FetchV2Request populated with information received from the * wire. * @throws PackProtocolException * incompatible options, wrong type of arguments or other issues * where the request breaks the protocol. * @throws IOException - * an IO error prevented reading the incoming message or - * accessing the ref database. + * an IO error prevented reading the incoming message. */ - FetchV2Request parseFetchRequest(PacketLineIn pckIn, RefDatabase refdb) + FetchV2Request parseFetchRequest(PacketLineIn pckIn) throws PackProtocolException, IOException { FetchV2Request.Builder reqBuilder = FetchV2Request.builder(); @@ -158,22 +149,7 @@ FetchV2Request parseFetchRequest(PacketLineIn pckIn, RefDatabase refdb) reqBuilder.addWantId(ObjectId.fromString(line.substring(5))); } else if (transferConfig.isAllowRefInWant() && line.startsWith(OPTION_WANT_REF + " ")) { //$NON-NLS-1$ - String refName = line.substring(OPTION_WANT_REF.length() + 1); - // TODO(ifrade): This validation should be done after the - // protocol parsing. It is not a protocol problem asking for an - // unexisting ref and we wouldn't need the ref database here - Ref ref = refdb.exactRef(refName); - if (ref == null) { - throw new PackProtocolException(MessageFormat - .format(JGitText.get().invalidRefName, refName)); - } - ObjectId oid = ref.getObjectId(); - if (oid == null) { - throw new PackProtocolException(MessageFormat - .format(JGitText.get().invalidRefName, refName)); - } - reqBuilder.addWantedRef(refName, oid); - reqBuilder.addWantId(oid); + reqBuilder.addWantedRef(line.substring(OPTION_WANT_REF.length() + 1)); } else if (line.startsWith("have ")) { //$NON-NLS-1$ reqBuilder.addPeerHas(ObjectId.fromString(line.substring(5))); } else if (line.equals("done")) { //$NON-NLS-1$
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 d0db9f0..a3e655c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java
@@ -327,6 +327,16 @@ private boolean prefixMatch(String p, String s) { }; } + /** + * Like {@code getRefFilter() == RefFilter.DEFAULT}, but faster. + * + * @return {@code true} if no ref filtering is needed because there + * are no configured hidden refs. + */ + boolean hasDefaultRefFilter() { + return hideRefs.length == 0; + } + static class FsckKeyNameHolder { private static final Map<String, ObjectChecker.ErrorType> errors;
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 7700eea..62c8dc9 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.CAPABILITY_REF_IN_WANT; import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_FETCH; import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_LS_REFS; @@ -75,11 +76,11 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.errors.CorruptObjectException; @@ -97,6 +98,7 @@ import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.AsyncRevObjectQueue; import org.eclipse.jgit.revwalk.BitmapWalker; @@ -274,7 +276,10 @@ private static interface IOConsumer<R> { 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 processing Git protocol v2 requests. */ @@ -283,6 +288,9 @@ private static interface IOConsumer<R> { /** 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; @@ -794,11 +802,115 @@ public PackStatistics getStatistics() { } private Map<String, Ref> getAdvertisedOrDefaultRefs() throws IOException { - if (refs == null) - setAdvertisedRefs(db.getRefDatabase().getRefs(ALL)); + if (refs != null) { + return refs; + } + + if (!advertiseRefsHookCalled) { + advertiseRefsHook.advertiseRefs(this); + advertiseRefsHookCalled = true; + } + if (refs == null) { + // 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. + String[] prefixes = refPrefixes.toArray(new String[0]); + Map<String, Ref> rs = + db.getRefDatabase().getRefsByPrefix(prefixes).stream() + .collect(toMap(Ref::getName, identity(), (a, b) -> b)); + 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())); + } + + /** + * Read a ref on behalf of the client. + * <p> + * This checks that the ref is present in the ref advertisement since + * otherwise the client might not be supposed to be able to read it. + * + * @param name + * the unabbreviated name of the reference. + * @return the requested Ref, or {@code null} if it is not visible or + * does not exist. + * @throws java.io.IOException + * on failure to read the ref or check it for visibility. + */ + @Nullable + private Ref getRef(String name) throws IOException { + if (refs != null) { + return refs.get(name); + } + if (!advertiseRefsHookCalled) { + advertiseRefsHook.advertiseRefs(this); + advertiseRefsHookCalled = true; + } + if (refs == null && + refFilter == RefFilter.DEFAULT && + transferConfig.hasDefaultRefFilter()) { + // Fast path: no ref filtering is needed. + return db.getRefDatabase().exactRef(name); + } + return getAdvertisedOrDefaultRefs().get(name); + } + + /** + * Find a ref in the usual search path on behalf of the client. + * <p> + * This checks that the ref is present in the ref advertisement since + * otherwise the client might not be supposed to be able to read it. + * + * @param name + * short name of the ref to find, e.g. "master" to find + * "refs/heads/master". + * @return the requested Ref, or {@code null} if it is not visible or + * does not exist. + * @throws java.io.IOException + * on failure to read the ref or check it for visibility. + */ + @Nullable + private Ref findRef(String name) throws IOException { + if (refs != null) { + return RefDatabase.findRef(refs, name); + } + if (!advertiseRefsHookCalled) { + advertiseRefsHook.advertiseRefs(this); + advertiseRefsHookCalled = true; + } + if (refs == null && + refFilter == RefFilter.DEFAULT && + transferConfig.hasDefaultRefFilter()) { + // Fast path: no ref filtering is needed. + return db.getRefDatabase().findRef(name); + } + return RefDatabase.findRef(getAdvertisedOrDefaultRefs(), name); + } + private void service() throws IOException { boolean sendPack = false; // If it's a non-bidi request, we need to read the entire request before @@ -921,16 +1033,7 @@ private void lsRefsV2() throws IOException { if (req.getPeel()) { adv.setDerefTags(true); } - Map<String, Ref> refsToSend; - if (req.getRefPrefixes().isEmpty()) { - refsToSend = getAdvertisedOrDefaultRefs(); - } else { - refsToSend = new HashMap<>(); - String[] prefixes = req.getRefPrefixes().toArray(new String[0]); - for (Ref ref : db.getRefDatabase().getRefsByPrefix(prefixes)) { - refsToSend.put(ref.getName(), ref); - } - } + Map<String, Ref> refsToSend = getFilteredRefs(req.getRefPrefixes()); if (req.getSymrefs()) { findSymrefs(adv, refsToSend); } @@ -953,8 +1056,7 @@ private void fetchV2() throws IOException { } ProtocolV2Parser parser = new ProtocolV2Parser(transferConfig); - FetchV2Request req = parser.parseFetchRequest(pckIn, - db.getRefDatabase()); + FetchV2Request req = parser.parseFetchRequest(pckIn); currentRequest = req; rawOut.stopBuffering(); @@ -962,11 +1064,9 @@ private void fetchV2() throws IOException { // TODO(ifrade): Refactor to pass around the Request object, instead of // copying data back to class fields - wantIds = req.getWantIds(); - List<ObjectId> deepenNots = new ArrayList<>(); for (String s : req.getDeepenNotRefs()) { - Ref ref = db.getRefDatabase().findRef(s); + Ref ref = findRef(s); if (ref == null) { throw new PackProtocolException(MessageFormat .format(JGitText.get().invalidRefName, s)); @@ -974,6 +1074,24 @@ private void fetchV2() throws IOException { deepenNots.add(ref.getObjectId()); } + Map<String, ObjectId> wantedRefs = new TreeMap<>(); + for (String refName : req.getWantedRefs()) { + Ref ref = getRef(refName); + if (ref == null) { + throw new PackProtocolException(MessageFormat + .format(JGitText.get().invalidRefName, refName)); + } + ObjectId oid = ref.getObjectId(); + if (oid == null) { + throw new PackProtocolException(MessageFormat + .format(JGitText.get().invalidRefName, refName)); + } + // TODO(ifrade): Avoid mutating the parsed request. + req.getWantIds().add(oid); + wantedRefs.put(refName, oid); + } + wantIds = req.getWantIds(); + boolean sectionSent = false; boolean mayHaveShallow = req.getDepth() != 0 || req.getDeepenSince() != 0 @@ -1027,13 +1145,13 @@ private void fetchV2() throws IOException { sectionSent = true; } - if (!req.getWantedRefs().isEmpty()) { + if (!wantedRefs.isEmpty()) { if (sectionSent) { pckOut.writeDelim(); } pckOut.writeString("wanted-refs\n"); //$NON-NLS-1$ - for (Map.Entry<String, ObjectId> entry : req.getWantedRefs() - .entrySet()) { + for (Map.Entry<String, ObjectId> entry : + wantedRefs.entrySet()) { pckOut.writeString(entry.getValue().getName() + ' ' + entry.getKey() + '\n'); } @@ -1286,15 +1404,7 @@ public void sendAdvertisedRefs(RefAdvertiser adv, return; } - try { - advertiseRefsHook.advertiseRefs(this); - } catch (ServiceMayNotContinueException fail) { - if (fail.getMessage() != null) { - adv.writeOne("ERR " + fail.getMessage()); //$NON-NLS-1$ - fail.setOutput(); - } - throw fail; - } + Map<String, Ref> advertisedOrDefaultRefs = getAdvertisedOrDefaultRefs(); if (serviceName != null) { adv.writeOne("# service=" + serviceName + '\n'); //$NON-NLS-1$ @@ -1326,7 +1436,6 @@ public void sendAdvertisedRefs(RefAdvertiser adv, adv.advertiseCapability(OPTION_FILTER); } adv.setDerefTags(true); - Map<String, Ref> advertisedOrDefaultRefs = getAdvertisedOrDefaultRefs(); findSymrefs(adv, advertisedOrDefaultRefs); advertised = adv.send(advertisedOrDefaultRefs); if (adv.isEmpty())