Make UploadPack observe exceptions reading refs

Now if refs are unreadable when serving an upload pack the handler
will fail due to the actual underlying failure. Previously all wants
would be rejected as invalid because Repository.getAllRefs() returned
an empty map.

Testing this required a new subclass of InMemoryRepository so that
an IOException could be injected at the correct time.

Signed-off-by: Michael Edgar <adgar@google.com>
Change-Id: Iac708b1db9d0ccce08c4ef5ace599ea0b57afdc0

2 files changed, 124 insertions, 12 deletions

diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RefsUnreadableInMemoryRepository.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RefsUnreadableInMemoryRepository.java
new file mode 100644
index 0000000000..485cced778
--- /dev/null
+++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RefsUnreadableInMemoryRepository.java
@@ -0,0 +1,52 @@
+package org.eclipse.jgit.http.test;
+
+import java.io.IOException;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.lib.RefDatabase;
+
+/**
+ * An {@link InMemoryRepository} whose refs can be made unreadable for testing
+ * purposes.
+ */
+class RefsUnreadableInMemoryRepository extends InMemoryRepository {
+
+	private final RefsUnreadableRefDatabase refs;
+
+	private volatile boolean failing;
+
+	RefsUnreadableInMemoryRepository(DfsRepositoryDescription repoDesc) {
+		super(repoDesc);
+		refs = new RefsUnreadableRefDatabase();
+		failing = false;
+	}
+
+	@Override
+	public RefDatabase getRefDatabase() {
+		return refs;
+	}
+
+	/**
+	 * Make the ref database unable to scan its refs.
+	 * <p>
+	 * It may be useful to follow a call to startFailing with a call to
+	 * {@link RefDatabase#refresh()}, ensuring the next ref read fails.
+	 */
+	void startFailing() {
+		failing = true;
+	}
+
+	private class RefsUnreadableRefDatabase extends MemRefDatabase {
+
+		@Override
+		protected RefCache scanAllRefs() throws IOException {
+			if (failing) {
+				throw new IOException("disk failed, no refs found");
+			} else {
+				return super.scanAllRefs();
+			}
+		}
+	}
+}
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 0f3d3c6cf5..073c751286 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
@@ -82,9 +82,11 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.http.server.GitServlet;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.junit.TestRng;
 import org.eclipse.jgit.junit.http.AccessEvent;
+import org.eclipse.jgit.junit.http.AppServer;
 import org.eclipse.jgit.junit.http.HttpTestCase;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
@@ -155,18 +157,7 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 
 		ServletContextHandler app = server.addContext("/git");
 		GitServlet gs = new GitServlet();
-		gs.setRepositoryResolver(new RepositoryResolver<HttpServletRequest>() {
-			public Repository open(HttpServletRequest req, String name)
-					throws RepositoryNotFoundException,
-					ServiceNotEnabledException {
-				if (!name.equals(srcName))
-					throw new RepositoryNotFoundException(name);
-
-				final Repository db = src.getRepository();
-				db.incrementOpen();
-				return db;
-			}
-		});
+		gs.setRepositoryResolver(new TestRepoResolver(src, srcName));
 		app.addServlet(new ServletHolder(gs), "/*");
 
 		ServletContextHandler broken = server.addContext("/bad");
@@ -509,6 +500,51 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 	}
 
 	@Test
+	public void testFetch_RefsUnreadableOnUpload() throws Exception {
+		AppServer noRefServer = new AppServer();
+		try {
+			final String repoName = "refs-unreadable";
+			RefsUnreadableInMemoryRepository badRefsRepo = new RefsUnreadableInMemoryRepository(
+					new DfsRepositoryDescription(repoName));
+			final TestRepository<Repository> repo = new TestRepository<Repository>(
+					badRefsRepo);
+
+			ServletContextHandler app = noRefServer.addContext("/git");
+			GitServlet gs = new GitServlet();
+			gs.setRepositoryResolver(new TestRepoResolver(repo, repoName));
+			app.addServlet(new ServletHolder(gs), "/*");
+			noRefServer.setUp();
+
+			RevBlob A2_txt = repo.blob("A2");
+			RevCommit A2 = repo.commit().add("A2_txt", A2_txt).create();
+			RevCommit B2 = repo.commit().parent(A2).add("A2_txt", "C2")
+					.add("B2", "B2").create();
+			repo.update(master, B2);
+
+			URIish badRefsURI = new URIish(noRefServer.getURI()
+					.resolve(app.getContextPath() + "/" + repoName).toString());
+
+			Repository dst = createBareRepository();
+			try (Transport t = Transport.open(dst, badRefsURI);
+					FetchConnection c = t.openFetch()) {
+				// We start failing here to exercise the post-advertisement
+				// upload pack handler.
+				badRefsRepo.startFailing();
+				// Need to flush caches because ref advertisement populated them.
+				badRefsRepo.getRefDatabase().refresh();
+				c.fetch(NullProgressMonitor.INSTANCE,
+						Collections.singleton(c.getRef(master)),
+						Collections.<ObjectId> emptySet());
+				fail("Successfully served ref with value " + c.getRef(master));
+			} catch (TransportException err) {
+				assertEquals("internal server error", err.getMessage());
+			}
+		} finally {
+			noRefServer.tearDown();
+		}
+	}
+
+	@Test
 	public void testPush_NotAuthorized() throws Exception {
 		final TestRepository src = createTestRepository();
 		final RevBlob Q_txt = src.blob("new text");
@@ -677,4 +713,28 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 		cfg.setBoolean("http", null, "receivepack", true);
 		cfg.save();
 	}
+
+	private final class TestRepoResolver
+			implements RepositoryResolver<HttpServletRequest> {
+
+		private final TestRepository<Repository> repo;
+
+		private final String repoName;
+
+		private TestRepoResolver(TestRepository<Repository> repo,
+				String repoName) {
+			this.repo = repo;
+			this.repoName = repoName;
+		}
+
+		public Repository open(HttpServletRequest req, String name)
+				throws RepositoryNotFoundException, ServiceNotEnabledException {
+			if (!name.equals(repoName))
+				throw new RepositoryNotFoundException(name);
+
+			Repository db = repo.getRepository();
+			db.incrementOpen();
+			return db;
+		}
+	}
 }