From 9239c1038562c9c16a49165e3293f8c895ef50c2 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 3 Sep 2010 09:38:30 -0700 Subject: [PATCH] ReceivePack: Rethrow exceptions caught during indexing If we get an exception while indexing the incoming pack, its likely a stream corruption. We already report an error to the client, but we eat the stack trace, which makes debugging issues related to a bug inside of JGit nearly impossible. Rethrow it under a new type UnpackException, so embedding servers or applications can catch the error and provide it to a human who might be able to forward such traces onto a JGit developer for evaluation. Change-Id: Icad41148bbc0c76f284c7033a195a6b51911beab Signed-off-by: Shawn O. Pearce --- .../transport/ReceivePackRefFilterTest.java | 58 +++++++++++++++-- .../org/eclipse/jgit/JGitText.properties | 1 + .../src/org/eclipse/jgit/JGitText.java | 1 + .../eclipse/jgit/errors/UnpackException.java | 64 +++++++++++++++++++ .../eclipse/jgit/transport/ReceivePack.java | 4 ++ 5 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/errors/UnpackException.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java index f89959c68..edced2fbb 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java @@ -52,6 +52,8 @@ import java.util.HashMap; import java.util.Map; import java.util.zip.Deflater; +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.errors.UnpackException; import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Constants; @@ -240,7 +242,15 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { rp.setCheckReceivedObjects(true); rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); - rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); + try { + receive(rp, inBuf, outBuf); + fail("Expected UnpackException"); + } catch (UnpackException failed) { + Throwable err = failed.getCause(); + assertTrue(err instanceof MissingObjectException); + MissingObjectException moe = (MissingObjectException) err; + assertEquals(P, moe.getObjectId()); + } final PacketLineIn r = asPacketLineIn(outBuf); String master = r.readString(); @@ -254,6 +264,12 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { assertSame(PacketLineIn.END, r.readString()); } + private void receive(final ReceivePack rp, + final TemporaryBuffer.Heap inBuf, final TemporaryBuffer.Heap outBuf) + throws IOException { + rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); + } + public void testUsingHiddenDeltaBaseFails() throws Exception { final TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(1024); packHeader(pack, 1); @@ -275,7 +291,15 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { rp.setCheckReceivedObjects(true); rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); - rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); + try { + receive(rp, inBuf, outBuf); + fail("Expected UnpackException"); + } catch (UnpackException failed) { + Throwable err = failed.getCause(); + assertTrue(err instanceof MissingObjectException); + MissingObjectException moe = (MissingObjectException) err; + assertEquals(b, moe.getObjectId()); + } final PacketLineIn r = asPacketLineIn(outBuf); String master = r.readString(); @@ -316,7 +340,15 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { rp.setCheckReceivedObjects(true); rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); - rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); + try { + receive(rp, inBuf, outBuf); + fail("Expected UnpackException"); + } catch (UnpackException failed) { + Throwable err = failed.getCause(); + assertTrue(err instanceof MissingObjectException); + MissingObjectException moe = (MissingObjectException) err; + assertEquals(b, moe.getObjectId()); + } final PacketLineIn r = asPacketLineIn(outBuf); String master = r.readString(); @@ -358,7 +390,15 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { rp.setCheckReceivedObjects(true); rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); - rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); + try { + receive(rp, inBuf, outBuf); + fail("Expected UnpackException"); + } catch (UnpackException failed) { + Throwable err = failed.getCause(); + assertTrue(err instanceof MissingObjectException); + MissingObjectException moe = (MissingObjectException) err; + assertEquals(n, moe.getObjectId()); + } final PacketLineIn r = asPacketLineIn(outBuf); String master = r.readString(); @@ -397,7 +437,15 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { rp.setCheckReceivedObjects(true); rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); - rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); + try { + receive(rp, inBuf, outBuf); + fail("Expected UnpackException"); + } catch (UnpackException failed) { + Throwable err = failed.getCause(); + assertTrue(err instanceof MissingObjectException); + MissingObjectException moe = (MissingObjectException) err; + assertEquals(t, moe.getObjectId()); + } final PacketLineIn r = asPacketLineIn(outBuf); String master = r.readString(); diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties index 14c0ba0fc..017fc5da9 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties @@ -383,6 +383,7 @@ unknownObjectType=Unknown object type {0}. unknownRepositoryFormat2=Unknown repository format "{0}"; expected "0". unknownRepositoryFormat=Unknown repository format unknownZlibError=Unknown zlib error. +unpackException=Exception while parsing pack stream unmergedPath=Unmerged path: {0} unreadablePackIndex=Unreadable pack index: {0} unrecognizedRef=Unrecognized ref: {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java index ea40fe884..055864ccc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java @@ -443,6 +443,7 @@ public class JGitText extends TranslationBundle { /***/ public String unknownRepositoryFormat; /***/ public String unknownZlibError; /***/ public String unmergedPath; + /***/ public String unpackException; /***/ public String unreadablePackIndex; /***/ public String unrecognizedRef; /***/ public String unsupportedCommand0; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/errors/UnpackException.java b/org.eclipse.jgit/src/org/eclipse/jgit/errors/UnpackException.java new file mode 100644 index 000000000..cf9552148 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/errors/UnpackException.java @@ -0,0 +1,64 @@ +/* + * Copyright (C) 2010, Google Inc. + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.errors; + +import java.io.IOException; + +import org.eclipse.jgit.JGitText; + +/** Indicates a ReceivePack failure while scanning the pack stream. */ +public class UnpackException extends IOException { + private static final long serialVersionUID = 1L; + + /** + * Creates an exception with a root cause. + * + * @param why + * the root cause of the unpacking failure. + */ + public UnpackException(Throwable why) { + super(JGitText.get().unpackException); + initCause(why); + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index f77e8e6ee..53a384117 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -68,6 +68,7 @@ import java.util.Set; import org.eclipse.jgit.JGitText; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.PackProtocolException; +import org.eclipse.jgit.errors.UnpackException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.NullProgressMonitor; @@ -661,6 +662,9 @@ public class ReceivePack { } postReceive.onPostReceive(this, filterCommands(Result.OK)); + + if (unpackError != null) + throw new UnpackException(unpackError); } }