Stop initCause throwing in readAdvertisedRefs

BasePackConnection::readAdvertisedRefsImpl was creating an exception by
calling `noRepository`, and then blindly calling `initCause` on it. As
`noRepository` can be overridden, it's not guaranteed to be missing a
cause.

BasePackPushConnection overrides `noRepository` and initiates a fetch,
which may throw a `NoRemoteRepositoryException` with a cause.

In this case calling `initCause` threw an `IllegalStateException`.

In order to throw the correct exception, we now return the
BasePackPushConnection exception and suppress the one thrown by
BasePackConnection

Bug: 578511
Change-Id: Ic1018b214be1e83d895979ee6c7cbce3f6765f6f

6 files changed, 156 insertions, 14 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BasePackConnectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BasePackConnectionTest.java
index 7d438c1dd8..505f334905 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BasePackConnectionTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BasePackConnectionTest.java
@@ -15,11 +15,15 @@ import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThrows;
 
+import java.io.ByteArrayInputStream;
+import java.io.EOFException;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
 
+import org.eclipse.jgit.errors.NoRemoteRepositoryException;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdRef;
@@ -30,6 +34,16 @@ import org.junit.Test;
 public class BasePackConnectionTest {
 
 	@Test
+	public void testReadAdvertisedRefsShouldThrowExceptionWithOriginalCause() {
+		try (FailingBasePackConnection basePackConnection =
+				new FailingBasePackConnection()) {
+			Exception result = assertThrows(NoRemoteRepositoryException.class,
+					basePackConnection::readAdvertisedRefs);
+			assertEquals(EOFException.class, result.getCause().getClass());
+		}
+	}
+
+	@Test
 	public void testUpdateWithSymRefsAdds() {
 		final Ref mainRef = new ObjectIdRef.Unpeeled(Ref.Storage.LOOSE,
 				"refs/heads/main", ObjectId.fromString(
@@ -244,4 +258,12 @@ public class BasePackConnectionTest {
 		assertEquals(oidName, headRef.getObjectId().name());
 		assertEquals(oidName, mainRef.getObjectId().name());
 	}
-}
\ No newline at end of file
+
+	private static class FailingBasePackConnection extends BasePackConnection {
+		FailingBasePackConnection() {
+			super(new TransportLocal(new URIish(),
+					new java.io.File("")));
+			pckIn = new PacketLineIn(new ByteArrayInputStream(new byte[0]));
+		}
+	}
+}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BasePackPushConnectionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BasePackPushConnectionTest.java
new file mode 100644
index 0000000000..cf8c5ffe79
--- /dev/null
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BasePackPushConnectionTest.java
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2022 Darius Jokilehto <darius.jokilehto+os@gmail.com>
+ * Copyright (c) 2022 Antonio Barone <syntonyze@gmail.com>
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+package org.eclipse.jgit.transport;
+
+import org.eclipse.jgit.errors.NoRemoteRepositoryException;
+import org.eclipse.jgit.errors.TransportException;
+import org.eclipse.jgit.internal.JGitText;
+import org.junit.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.EOFException;
+import java.io.IOException;
+import java.util.Arrays;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.endsWith;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+public class BasePackPushConnectionTest {
+	@Test
+	public void testNoRemoteRepository() {
+		NoRemoteRepositoryException openFetchException =
+                new NoRemoteRepositoryException( new URIish(), "not found");
+		IOException ioException = new IOException("not read");
+
+		try (FailingBasePackPushConnection fbppc =
+				new FailingBasePackPushConnection(openFetchException)) {
+			TransportException result = fbppc.noRepository(ioException);
+
+			assertEquals(openFetchException, result);
+			assertThat(Arrays.asList(result.getSuppressed()),
+					hasItem(ioException));
+		}
+	}
+
+	@Test
+	public void testPushNotPermitted() {
+		URIish uri = new URIish();
+		TransportException openFetchException = new TransportException(uri,
+				"a transport exception");
+		IOException ioException = new IOException("not read");
+
+		try (FailingBasePackPushConnection fbppc =
+				new FailingBasePackPushConnection(openFetchException)) {
+			TransportException result = fbppc.noRepository(ioException);
+
+			assertEquals(TransportException.class, result.getClass());
+			assertThat(result.getMessage(),
+					endsWith(JGitText.get().pushNotPermitted));
+			assertEquals(openFetchException, result.getCause());
+			assertThat(Arrays.asList(result.getSuppressed()),
+					hasItem(ioException));
+		}
+	}
+
+	@Test
+	public void testReadAdvertisedRefPropagatesCauseAndSuppressedExceptions() {
+		IOException ioException = new IOException("not read");
+		try (FailingBasePackPushConnection basePackConnection =
+				new FailingBasePackPushConnection(
+						new NoRemoteRepositoryException(
+								new URIish(), "not found", ioException))) {
+			Exception result = assertThrows(NoRemoteRepositoryException.class,
+					basePackConnection::readAdvertisedRefs);
+			assertEquals(ioException, result.getCause());
+			assertThat(Arrays.asList(result.getSuppressed()),
+					hasItem(instanceOf(EOFException.class)));
+		}
+	}
+
+	private static class FailingBasePackPushConnection
+			extends BasePackPushConnection {
+		FailingBasePackPushConnection(TransportException openFetchException) {
+			super(new TransportLocal(new URIish(),
+					new java.io.File("")) {
+				@Override public FetchConnection openFetch()
+						throws TransportException {
+					throw openFetchException;
+				}
+			});
+			pckIn = new PacketLineIn(new ByteArrayInputStream(new byte[0]));
+		}
+	}
+}
diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index 83a83817f4..acceac05df 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -8,6 +8,14 @@
             </message_arguments>
         </filter>
     </resource>
+    <resource path="src/org/eclipse/jgit/transport/BasePackPushConnection.java" type="org.eclipse.jgit.transport.BasePackPushConnection">
+        <filter id="338792546">
+            <message_arguments>
+                <message_argument value="org.eclipse.jgit.transport.BasePackPushConnection"/>
+                <message_argument value="noRepository()"/>
+            </message_arguments>
+        </filter>
+    </resource>
     <resource path="src/org/eclipse/jgit/transport/ProtocolV2Hook.java" type="org.eclipse.jgit.transport.ProtocolV2Hook">
         <filter id="404000815">
             <message_arguments>
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/errors/NoRemoteRepositoryException.java b/org.eclipse.jgit/src/org/eclipse/jgit/errors/NoRemoteRepositoryException.java
index 58f70f5d4b..1dd976cec9 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/errors/NoRemoteRepositoryException.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/errors/NoRemoteRepositoryException.java
@@ -29,4 +29,19 @@ public class NoRemoteRepositoryException extends TransportException {
 	public NoRemoteRepositoryException(URIish uri, String s) {
 		super(uri, s);
 	}
+
+	/**
+	 * Constructs an exception indicating a repository does not exist.
+	 *
+	 * @param uri
+	 *            URI used for transport
+	 * @param s
+	 *            message
+	 * @param cause
+	 *            root cause exception
+	 * @since 5.13
+	 */
+	public NoRemoteRepositoryException(URIish uri, String s, Throwable cause) {
+		super(uri, s, cause);
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java
index 3826bf7401..09c559d7b5 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java
@@ -210,9 +210,7 @@ abstract class BasePackConnection extends BaseConnection {
 				try {
 					line = readLine();
 				} catch (EOFException e) {
-					TransportException noRepo = noRepository();
-					noRepo.initCause(e);
-					throw noRepo;
+					throw noRepository(e);
 				}
 				if (line != null && VERSION_1.equals(line)) {
 					// Same as V0, except for this extra line. We shouldn't get
@@ -567,11 +565,14 @@ abstract class BasePackConnection extends BaseConnection {
 	 *
 	 * Subclasses may override this method to provide better diagnostics.
 	 *
+	 * @param cause
+	 *            root cause exception
 	 * @return a TransportException saying a repository cannot be found and
 	 *         possibly why.
 	 */
-	protected TransportException noRepository() {
-		return new NoRemoteRepositoryException(uri, JGitText.get().notFound);
+	protected TransportException noRepository(Throwable cause) {
+		return new NoRemoteRepositoryException(uri, JGitText.get().notFound,
+				cause);
 	}
 
 	/**
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
index eb1d2ac0a9..b87a85d934 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
@@ -139,7 +139,7 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen
 
 	/** {@inheritDoc} */
 	@Override
-	protected TransportException noRepository() {
+	protected TransportException noRepository(Throwable cause) {
 		// Sadly we cannot tell the "invalid URI" case from "push not allowed".
 		// Opening a fetch connection can help us tell the difference, as any
 		// useful repository is going to support fetch if it also would allow
@@ -147,18 +147,18 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen
 		// URI is wrong. Otherwise we can correctly state push isn't allowed
 		// as the fetch connection opened successfully.
 		//
+		TransportException te;
 		try {
 			transport.openFetch().close();
-		} catch (NotSupportedException e) {
-			// Fall through.
+			te = new TransportException(uri, JGitText.get().pushNotPermitted);
 		} catch (NoRemoteRepositoryException e) {
 			// Fetch concluded the repository doesn't exist.
-			//
-			return e;
-		} catch (TransportException e) {
-			// Fall through.
+			te = e;
+		} catch (NotSupportedException | TransportException e) {
+			te = new TransportException(uri, JGitText.get().pushNotPermitted, e);
 		}
-		return new TransportException(uri, JGitText.get().pushNotPermitted);
+		te.addSuppressed(cause);
+		return te;
 	}
 
 	/**