RefDirectory.getRef: Treat fake missing symrefs like real ones

getRef() loops over its search path to find a ref:

	Ref ref = null;
	for (String prefix : SEARCH_PATH) {
		ref = readRef(prefix + needle, packed);
		if (ref != null) {
			ref = resolve(ref, 0, null, null, packed);
			break;
		}
	}
	fireRefsChanged();
	return ref;

If readRef returns null (indicating that the ref does not exist), the
loop continues so we can find the ref later in the search path.  And
resolve should never return null, so if we return null it should mean
we exhausted the entire search path and didn't find the ref.

... except that resolve can return null: it does so when it has
followed too many symrefs and concluded that there is a symref loop:

	if (MAX_SYMBOLIC_REF_DEPTH <= depth)
		return null; // claim it doesn't exist

Continue the loop instead of returning null immediately.  This makes
the behavior more consistent.

Arguably getRef should throw an exception when a symref loop is
detected.  That would be a more invasive change, so if it's a good
idea it will have to wait for another patch.

Change-Id: Icb1c7fafd4f1e34c9b43538e27ab5bbc17ad9eef
Signed-off-by: Jonathan Nieder <jrn@google.com>

1 files changed, 30 insertions, 0 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java
index d66753da08..52e181bc80 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java
@@ -858,6 +858,36 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 	}
 
 	@Test
+	public void testGetRef_CycleInSymbolicRef() throws IOException {
+		Ref r;
+
+		writeLooseRef("refs/1", "ref: refs/2\n");
+		writeLooseRef("refs/2", "ref: refs/3\n");
+		writeLooseRef("refs/3", "ref: refs/4\n");
+		writeLooseRef("refs/4", "ref: refs/5\n");
+		writeLooseRef("refs/5", "ref: refs/end\n");
+		writeLooseRef("refs/end", A);
+
+		r = refdir.getRef("1");
+		assertEquals("refs/1", r.getName());
+		assertEquals(A, r.getObjectId());
+		assertTrue(r.isSymbolic());
+
+		writeLooseRef("refs/5", "ref: refs/6\n");
+		writeLooseRef("refs/6", "ref: refs/end\n");
+
+		r = refdir.getRef("1");
+		assertNull("missing 1 due to cycle", r);
+
+		writeLooseRef("refs/heads/1", B);
+
+		r = refdir.getRef("1");
+		assertEquals("refs/heads/1", r.getName());
+		assertEquals(B, r.getObjectId());
+		assertFalse(r.isSymbolic());
+	}
+
+	@Test
 	public void testGetRefs_PackedNotPeeled_Sorted() throws IOException {
 		Map<String, Ref> all;