Merge branch 'stable-5.1' into stable-5.2

* stable-5.1:
  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: I5879df9b723a0dbf6a1eff89a34bbb269f3b773d
Signed-off-by: Jonathan Nieder <jrn@google.com>

org.eclipse.jgit.http.test/META-INF/MANIFEST.MF

@@ -50,5 +50,7 @@ Import-Package: javax.servlet;version="[2.5.0,3.2.0)",
  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)"

org.eclipse.jgit.http.test/pom.xml

@@ -70,6 +70,13 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>org.hamcrest</groupId>
+      <artifactId>hamcrest-library</artifactId>
+      <scope>test</scope>
+      <version>[1.1.0,2.0.0)</version>
+    </dependency>
+
     <dependency>
       <groupId>org.eclipse.jgit</groupId>
       <artifactId>org.eclipse.jgit</artifactId>
@@ -83,13 +90,6 @@
       <scope>test</scope>
     </dependency>
 
-    <dependency>
-      <groupId>org.hamcrest</groupId>
-      <artifactId>hamcrest-library</artifactId>
-      <scope>test</scope>
-      <version>[1.1.0,2.0.0)</version>
-    </dependency>
-
     <dependency>
       <groupId>org.eclipse.jgit</groupId>
       <artifactId>org.eclipse.jgit.junit.http</artifactId>

org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java

@@ -85,6 +85,7 @@ import org.eclipse.jgit.errors.RemoteRepositoryException;
 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.Repository;
 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 @@ import org.junit.runners.Parameterized.Parameters;
 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 class SmartClientSmartServerTest extends HttpTestCase {
 						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 class SmartClientSmartServerTest extends HttpTestCase {
 		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);
 	}
 
@@ -452,6 +484,56 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 				info.getResponseHeader(HDR_CONTENT_TYPE));
 	}
 
+	@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();

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 class ProtocolV2ParserTest {
 				PacketLineIn.END);
 		ProtocolV2Parser parser = new ProtocolV2Parser(
 				ConfigBuilder.getDefault());
-		FetchV2Request request = parser.parseFetchRequest(pckIn,
+		FetchV2Request request = parser.parseFetchRequest(pckIn);
-				testRepo.getRepository().getRefDatabase());
 		assertTrue(request.getClientCapabilities()
 				.contains(GitProtocolConstants.OPTION_THIN_PACK));
 		assertTrue(request.getClientCapabilities()
@@ -183,8 +181,7 @@ public class ProtocolV2ParserTest {
 				PacketLineIn.END);
 		ProtocolV2Parser parser = new ProtocolV2Parser(
 				ConfigBuilder.getDefault());
-		FetchV2Request request = parser.parseFetchRequest(pckIn,
+		FetchV2Request request = parser.parseFetchRequest(pckIn);
-				testRepo.getRepository().getRefDatabase());
 		assertThat(request.getClientShallowCommits(),
 				hasOnlyObjectIds("28274d02c489f4c7e68153056e9061a46f62d7a0",
 						"145e683b229dcab9d0e2ccb01b386f9ecc17d29d"));
@@ -203,8 +200,7 @@ public class ProtocolV2ParserTest {
 				PacketLineIn.END);
 		ProtocolV2Parser parser = new ProtocolV2Parser(
 				ConfigBuilder.getDefault());
-		FetchV2Request request = parser.parseFetchRequest(pckIn,
+		FetchV2Request request = parser.parseFetchRequest(pckIn);
-				testRepo.getRepository().getRefDatabase());
 		assertThat(request.getClientShallowCommits(),
 				hasOnlyObjectIds("28274d02c489f4c7e68153056e9061a46f62d7a0",
 						"145e683b229dcab9d0e2ccb01b386f9ecc17d29d"));
@@ -221,8 +217,7 @@ public class ProtocolV2ParserTest {
 				PacketLineIn.END);
 		ProtocolV2Parser parser = new ProtocolV2Parser(
 				ConfigBuilder.getDefault());
-		FetchV2Request request = parser.parseFetchRequest(pckIn,
+		FetchV2Request request = parser.parseFetchRequest(pckIn);
-				testRepo.getRepository().getRefDatabase());
 		assertThat(request.getClientShallowCommits(),
 				hasOnlyObjectIds("28274d02c489f4c7e68153056e9061a46f62d7a0",
 						"145e683b229dcab9d0e2ccb01b386f9ecc17d29d"));
@@ -236,8 +231,7 @@ public class ProtocolV2ParserTest {
 				PacketLineIn.END);
 		ProtocolV2Parser parser = new ProtocolV2Parser(
 				ConfigBuilder.start().allowFilter().done());
-		FetchV2Request request = parser.parseFetchRequest(pckIn,
+		FetchV2Request request = parser.parseFetchRequest(pckIn);
-				testRepo.getRepository().getRefDatabase());
 		assertEquals(0, request.getFilterBlobLimit());
 	}
 
@@ -248,8 +242,7 @@ public class ProtocolV2ParserTest {
 				PacketLineIn.END);
 		ProtocolV2Parser parser = new ProtocolV2Parser(
 				ConfigBuilder.start().allowFilter().done());
-		FetchV2Request request = parser.parseFetchRequest(pckIn,
+		FetchV2Request request = parser.parseFetchRequest(pckIn);
-				testRepo.getRepository().getRefDatabase());
 		assertEquals(15, request.getFilterBlobLimit());
 	}
 
@@ -263,8 +256,7 @@ public class ProtocolV2ParserTest {
 				ConfigBuilder.start().allowFilter().done());
 
 		thrown.expect(PackProtocolException.class);
-		parser.parseFetchRequest(pckIn,
+		parser.parseFetchRequest(pckIn);
-				testRepo.getRepository().getRefDatabase());
 	}
 
 	@Test
@@ -275,8 +267,7 @@ public class ProtocolV2ParserTest {
 				ConfigBuilder.getDefault());
 
 		thrown.expect(PackProtocolException.class);
-		parser.parseFetchRequest(pckIn,
+		parser.parseFetchRequest(pckIn);
-				testRepo.getRepository().getRefDatabase());
 	}
 
 	@Test
@@ -293,16 +284,13 @@ public class ProtocolV2ParserTest {
 		ProtocolV2Parser parser = new ProtocolV2Parser(
 				ConfigBuilder.start().allowRefInWant().done());
 
-
+		FetchV2Request request = parser.parseFetchRequest(pckIn);
-		FetchV2Request request = parser.parseFetchRequest(pckIn,
-				testRepo.getRepository().getRefDatabase());
 		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",
+				"e4980cdc48cfa1301493ca94eb70523f6788b819"));
-				one.getName()));
 	}
 
 	@Test
@@ -319,10 +307,9 @@ public class ProtocolV2ParserTest {
 		testRepo.update("branchA", one);
 		testRepo.update("branchB", two);
 
-		thrown.expect(PackProtocolException.class);
+		FetchV2Request request = parser.parseFetchRequest(pckIn);
-		thrown.expectMessage(containsString("refs/heads/branchC"));
+		assertEquals(1, request.getWantedRefs().size());
-		parser.parseFetchRequest(pckIn,
+		assertThat(request.getWantedRefs(), hasItems("refs/heads/branchC"));
-				testRepo.getRepository().getRefDatabase());
 	}
 
 	@Test

org.eclipse.jgit/.settings/.api_filters

@@ -66,6 +66,20 @@
             </message_arguments>
         </filter>
     </resource>
+    <resource path="src/org/eclipse/jgit/transport/TransferConfig.java" type="org.eclipse.jgit.transport.TransferConfig">
+        <filter id="1159725059">
+            <message_arguments>
+                <message_argument value="5.1.4"/>
+                <message_argument value="TransferConfig(Config)"/>
+            </message_arguments>
+        </filter>
+        <filter id="1159725059">
+            <message_arguments>
+                <message_argument value="5.1.4"/>
+                <message_argument value="TransferConfig(Repository)"/>
+            </message_arguments>
+        </filter>
+    </resource>
     <resource path="src/org/eclipse/jgit/transport/http/HttpConnection.java" type="org.eclipse.jgit.transport.http.HttpConnection">
         <filter id="403804204">
             <message_arguments>

org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java

@@ -48,9 +48,7 @@ import java.util.ArrayList;
 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 @@ import org.eclipse.jgit.lib.ObjectId;
 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 @@ public final class FetchV2Request extends FetchRequest {
 	 * @return list of references received in "want-ref" lines
 	 */
 	@NonNull
-	Map<String, ObjectId> getWantedRefs() {
+	List<String> getWantedRefs() {
 		return wantedRefs;
 	}
 
@@ -135,7 +133,7 @@ public final class FetchV2Request extends FetchRequest {
 	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 @@ public final class FetchV2Request extends FetchRequest {
 		 *
 		 * @param refName
 		 *            reference name
-		 * @param oid
-		 *            object id the reference is pointing at
 		 * @return this builder
 		 */
-		Builder addWantedRef(String refName, ObjectId oid) {
+		Builder addWantedRef(String refName) {
-			wantedRefs.put(refName, oid);
+			wantedRefs.add(refName);
 			return this;
 		}
 

org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java

@@ -62,8 +62,6 @@ import java.util.function.Consumer;
 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 @@ final class ProtocolV2Parser {
 	 * 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
+	 *             an IO error prevented reading the incoming message.
-	 *             accessing the ref database.
 	 */
-	FetchV2Request parseFetchRequest(PacketLineIn pckIn, RefDatabase refdb)
+	FetchV2Request parseFetchRequest(PacketLineIn pckIn)
 			throws PackProtocolException, IOException {
 		FetchV2Request.Builder reqBuilder = FetchV2Request.builder();
 
@@ -158,22 +149,7 @@ final class ProtocolV2Parser {
 				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);
+				reqBuilder.addWantedRef(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);
 			} else if (line.startsWith("have ")) { //$NON-NLS-1$
 				reqBuilder.addPeerHas(ObjectId.fromString(line.substring(5)));
 			} else if (line.equals("done")) { //$NON-NLS-1$

org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java

@@ -327,6 +327,16 @@ public class TransferConfig {
 		};
 	}
 
+	/**
+	 * 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;
 

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.text.MessageFormat;
 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;
@@ -274,7 +275,10 @@ 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 processing Git protocol v2 requests. */
@@ -283,6 +287,9 @@ public class UploadPack {
 	/** 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 +801,83 @@ public class UploadPack {
 	}
 
 	private Map<String, Ref> getAdvertisedOrDefaultRefs() throws IOException {
-		if (refs == null)
+		if (refs != null) {
-			setAdvertisedRefs(db.getRefDatabase().getRefs(ALL));
+			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);
+	}
+
 	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 +1000,7 @@ public class UploadPack {
 		if (req.getPeel()) {
 			adv.setDerefTags(true);
 		}
-		Map<String, Ref> refsToSend;
+		Map<String, Ref> refsToSend = getFilteredRefs(req.getRefPrefixes());
-		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);
-			}
-		}
 		if (req.getSymrefs()) {
 			findSymrefs(adv, refsToSend);
 		}
@@ -953,8 +1023,7 @@ public class UploadPack {
 		}
 
 		ProtocolV2Parser parser = new ProtocolV2Parser(transferConfig);
-		FetchV2Request req = parser.parseFetchRequest(pckIn,
+		FetchV2Request req = parser.parseFetchRequest(pckIn);
-				db.getRefDatabase());
 		currentRequest = req;
 		rawOut.stopBuffering();
 
@@ -962,8 +1031,6 @@ public class UploadPack {
 
 		// 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().getRef(s);
@@ -974,6 +1041,24 @@ public class UploadPack {
 			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 +1112,13 @@ public class UploadPack {
 				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()
+				for (Map.Entry<String, ObjectId> entry :
-						.entrySet()) {
+						wantedRefs.entrySet()) {
 					pckOut.writeString(entry.getValue().getName() + ' ' +
 							entry.getKey() + '\n');
 				}
@@ -1286,15 +1371,7 @@ public class UploadPack {
 			return;
 		}
 
-		try {
+		Map<String, Ref> advertisedOrDefaultRefs = getAdvertisedOrDefaultRefs();
-			advertiseRefsHook.advertiseRefs(this);
-		} catch (ServiceMayNotContinueException fail) {
-			if (fail.getMessage() != null) {
-				adv.writeOne("ERR " + fail.getMessage()); //$NON-NLS-1$
-				fail.setOutput();
-			}
-			throw fail;
-		}
 
 		if (serviceName != null) {
 			adv.writeOne("# service=" + serviceName + '\n'); //$NON-NLS-1$
@@ -1326,7 +1403,6 @@ public class UploadPack {
 			adv.advertiseCapability(OPTION_FILTER);
 		}
 		adv.setDerefTags(true);
-		Map<String, Ref> advertisedOrDefaultRefs = getAdvertisedOrDefaultRefs();
 		findSymrefs(adv, advertisedOrDefaultRefs);
 		advertised = adv.send(advertisedOrDefaultRefs);
 		if (adv.isEmpty())