diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java index 4892dabb6..0ef29d486 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java @@ -10,7 +10,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -28,7 +27,6 @@ import java.util.Map; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.dircache.DirCacheBuilder; import org.eclipse.jgit.dircache.DirCacheEntry; -import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.errors.TransportException; import org.eclipse.jgit.internal.storage.dfs.DfsGarbageCollector; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; @@ -734,11 +732,12 @@ public class UploadPackTest { @Test public void testV2LsRefsUnrecognizedArgument() throws Exception { - PackProtocolException e = assertThrows(PackProtocolException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2("command=ls-refs\n", PacketLineIn.delimiter(), "invalid-argument\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("unexpected invalid-argument")); } @@ -791,12 +790,13 @@ public class UploadPackTest { PacketLineIn.end()); // This doesn't - TransportException e = assertThrows(TransportException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2(RequestPolicy.ADVERTISED, null, null, "command=fetch\n", PacketLineIn.delimiter(), "want " + unadvertized.name() + "\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("want " + unadvertized.name() + " not valid")); } @@ -814,12 +814,13 @@ public class UploadPackTest { "want " + reachable.name() + "\n", PacketLineIn.end()); // This doesn't - TransportException e = assertThrows(TransportException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2(RequestPolicy.REACHABLE_COMMIT, null, null, "command=fetch\n", PacketLineIn.delimiter(), "want " + unreachable.name() + "\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("want " + unreachable.name() + " not valid")); } @@ -836,12 +837,13 @@ public class UploadPackTest { "want " + tip.name() + "\n", PacketLineIn.end()); // This doesn't - TransportException e = assertThrows(TransportException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2(RequestPolicy.TIP, new RejectAllRefFilter(), null, "command=fetch\n", PacketLineIn.delimiter(), "want " + parentOfTip.name() + "\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("want " + parentOfTip.name() + " not valid")); } @@ -860,13 +862,14 @@ public class UploadPackTest { PacketLineIn.end()); // This doesn't - TransportException e = assertThrows(TransportException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2(RequestPolicy.REACHABLE_COMMIT_TIP, new RejectAllRefFilter(), null, "command=fetch\n", PacketLineIn.delimiter(), "want " + unreachable.name() + "\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("want " + unreachable.name() + " not valid")); } @@ -1302,12 +1305,13 @@ public class UploadPackTest { remote.update("branch1", tooOld); - PackProtocolException e = assertThrows(PackProtocolException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(), "deepen-since 1510000\n", "want " + tooOld.toObjectId().getName() + "\n", "done\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("No commits selected for shallow request")); } @@ -1377,12 +1381,13 @@ public class UploadPackTest { remote.update("two", two); remote.update("four", four); - PackProtocolException e = assertThrows(PackProtocolException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(), "deepen-not four\n", "want " + two.toObjectId().getName() + "\n", "done\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("No commits selected for shallow request")); } @@ -1461,10 +1466,11 @@ public class UploadPackTest { @Test public void testV2FetchUnrecognizedArgument() throws Exception { - PackProtocolException e = assertThrows(PackProtocolException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(), "invalid-argument\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("unexpected invalid-argument")); } @@ -1834,11 +1840,12 @@ public class UploadPackTest { server.getConfig().setBoolean("uploadpack", null, "allowfilter", false); - PackProtocolException e = assertThrows(PackProtocolException.class, + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, () -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(), "want " + commit.toObjectId().getName() + "\n", "filter blob:limit=5\n", "done\n", PacketLineIn.end())); - assertThat(e.getMessage(), + assertThat(e.getCause().getMessage(), containsString("unexpected filter blob:limit=5")); } @@ -1847,20 +1854,13 @@ public class UploadPackTest { RevCommit one = remote.commit().message("1").create(); remote.update("one", one); - try { - uploadPackV2( - "command=fetch\n", - PacketLineIn.delimiter(), - "want-ref refs/heads/one\n", - "done\n", - PacketLineIn.end()); - } catch (PackProtocolException e) { - assertThat( - e.getMessage(), + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, + () -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(), + "want-ref refs/heads/one\n", "done\n", + PacketLineIn.end())); + assertThat(e.getCause().getMessage(), containsString("unexpected want-ref refs/heads/one")); - return; - } - fail("expected PackProtocolException"); } @Test @@ -1902,23 +1902,17 @@ public class UploadPackTest { RevCommit one = remote.commit().message("1").create(); remote.update("one", one); - server.getConfig().setBoolean("uploadpack", null, "allowrefinwant", true); + server.getConfig().setBoolean("uploadpack", null, "allowrefinwant", + true); - try { - uploadPackV2( - "command=fetch\n", - PacketLineIn.delimiter(), - "want-ref refs/heads/one\n", - "want-ref refs/heads/nonExistentRef\n", - "done\n", - PacketLineIn.end()); - } catch (PackProtocolException e) { - assertThat( - e.getMessage(), + UploadPackInternalServerErrorException e = assertThrows( + UploadPackInternalServerErrorException.class, + () -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(), + "want-ref refs/heads/one\n", + "want-ref refs/heads/nonExistentRef\n", "done\n", + PacketLineIn.end())); + assertThat(e.getCause().getMessage(), containsString("Invalid ref name: refs/heads/nonExistentRef")); - return; - } - fail("expected PackProtocolException"); } @Test 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 926c35a69..52d2ed105 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -751,8 +751,8 @@ public class UploadPack { * other network connections this should be null. * @throws java.io.IOException */ - public void upload(final InputStream input, OutputStream output, - final OutputStream messages) throws IOException { + public void upload(InputStream input, OutputStream output, + @Nullable OutputStream messages) throws IOException { try { rawIn = input; if (messages != null) @@ -782,6 +782,36 @@ public class UploadPack { } else { service(); } + } catch (UploadPackInternalServerErrorException err) { + // UploadPackInternalServerErrorException is a special exception + // that indicates an error is already written to the client. Do + // nothing. + throw err; + } catch (ServiceMayNotContinueException err) { + if (!err.isOutput() && err.getMessage() != null && pckOut != null) { + try { + pckOut.writeString("ERR " + err.getMessage() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ + } catch (IOException e) { + err.addSuppressed(e); + throw err; + } + err.setOutput(); + } + throw err; + } catch (IOException | RuntimeException | Error err) { + if (pckOut != null) { + String msg = err instanceof PackProtocolException + ? err.getMessage() + : JGitText.get().internalServerError; + try { + pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ + } catch (IOException e) { + err.addSuppressed(e); + throw err; + } + throw new UploadPackInternalServerErrorException(err); + } + throw err; } finally { msgOut = NullOutputStream.INSTANCE; walk.close(); @@ -1013,27 +1043,6 @@ public class UploadPack { "\\x" + Integer.toHexString(eof))); //$NON-NLS-1$ } } - } catch (ServiceMayNotContinueException err) { - if (!err.isOutput() && err.getMessage() != null) { - try { - pckOut.writeString("ERR " + err.getMessage() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ - } catch (IOException e) { - err.addSuppressed(e); - throw err; - } - err.setOutput(); - } - throw err; - } catch (IOException | RuntimeException | Error err) { - String msg = err instanceof PackProtocolException ? err.getMessage() - : JGitText.get().internalServerError; - try { - pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ - } catch (IOException e) { - err.addSuppressed(e); - throw err; - } - throw new UploadPackInternalServerErrorException(err); } finally { if (!sendPack && !biDirectionalPipe) { while (0 < rawIn.skip(2048) || 0 <= rawIn.read()) {