diff --git a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF index 08924e72c..253679b2c 100644 --- a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF @@ -27,6 +27,7 @@ Import-Package: javax.servlet;version="[2.5.0,3.2.0)", org.eclipse.jgit.http.server.glue;version="[4.4.0,4.5.0)", org.eclipse.jgit.http.server.resolver;version="[4.4.0,4.5.0)", org.eclipse.jgit.internal;version="[4.4.0,4.5.0)", + org.eclipse.jgit.internal.storage.dfs;version="[4.4.0,4.5.0)", org.eclipse.jgit.internal.storage.file;version="[4.4.0,4.5.0)", org.eclipse.jgit.junit;version="[4.4.0,4.5.0)", org.eclipse.jgit.junit.http;version="[4.4.0,4.5.0)", 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 000000000..485cced77 --- /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. + *

+ * 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 0f3d3c6cf..073c75128 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() { - 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"); @@ -508,6 +499,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 repo = new TestRepository( + 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. 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(); @@ -677,4 +713,28 @@ public class SmartClientSmartServerTest extends HttpTestCase { cfg.setBoolean("http", null, "receivepack", true); cfg.save(); } + + private final class TestRepoResolver + implements RepositoryResolver { + + private final TestRepository repo; + + private final String repoName; + + private TestRepoResolver(TestRepository 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; + } + } } diff --git a/org.eclipse.jgit/META-INF/MANIFEST.MF b/org.eclipse.jgit/META-INF/MANIFEST.MF index 51bee1aff..a3d1a92f5 100644 --- a/org.eclipse.jgit/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit/META-INF/MANIFEST.MF @@ -60,7 +60,10 @@ Export-Package: org.eclipse.jgit.annotations;version="4.4.0", org.eclipse.jgit.ignore.internal;version="4.4.0";x-friends:="org.eclipse.jgit.test", org.eclipse.jgit.internal;version="4.4.0";x-friends:="org.eclipse.jgit.test,org.eclipse.jgit.http.test", org.eclipse.jgit.internal.ketch;version="4.4.0";x-friends:="org.eclipse.jgit.junit,org.eclipse.jgit.test,org.eclipse.jgit.pgm", - org.eclipse.jgit.internal.storage.dfs;version="4.4.0";x-friends:="org.eclipse.jgit.test,org.eclipse.jgit.http.server", + org.eclipse.jgit.internal.storage.dfs;version="4.4.0"; + x-friends:="org.eclipse.jgit.test, + org.eclipse.jgit.http.server, + org.eclipse.jgit.http.test", org.eclipse.jgit.internal.storage.file;version="4.4.0"; x-friends:="org.eclipse.jgit.test, org.eclipse.jgit.junit, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java index b312835bb..ccf1b42b3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java @@ -252,11 +252,20 @@ public class InMemoryRepository extends DfsRepository { } } - private class MemRefDatabase extends DfsRefDatabase { + /** + * A ref database storing all refs in-memory. + *

+ * This class is protected (and not private) to facilitate testing using + * subclasses of InMemoryRepository. + */ + protected class MemRefDatabase extends DfsRefDatabase { private final ConcurrentMap refs = new ConcurrentHashMap(); private final ReadWriteLock lock = new ReentrantReadWriteLock(true /* fair */); - MemRefDatabase() { + /** + * Initialize a new in-memory ref database. + */ + protected MemRefDatabase() { super(InMemoryRepository.this); } 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 8b642bb98..e49ee87b7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -82,6 +82,7 @@ import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; 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.DepthWalk; @@ -704,22 +705,22 @@ public class UploadPack { return statistics; } - private Map getAdvertisedOrDefaultRefs() { + private Map getAdvertisedOrDefaultRefs() throws IOException { if (refs == null) - setAdvertisedRefs(null); + setAdvertisedRefs(db.getRefDatabase().getRefs(RefDatabase.ALL)); return refs; } private void service() throws IOException { - if (biDirectionalPipe) - sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut)); - else if (requestValidator instanceof AnyRequestValidator) - advertised = Collections.emptySet(); - else - advertised = refIdSet(getAdvertisedOrDefaultRefs().values()); - boolean sendPack; try { + if (biDirectionalPipe) + sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut)); + else if (requestValidator instanceof AnyRequestValidator) + advertised = Collections.emptySet(); + else + advertised = refIdSet(getAdvertisedOrDefaultRefs().values()); + recvWants(); if (wantIds.isEmpty()) { preUploadHook.onBeginNegotiateRound(this, wantIds, 0);