diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java index 192050a17..0c856d4aa 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java @@ -64,6 +64,7 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.errors.UnpackException; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser; @@ -170,9 +171,18 @@ class ReceivePackServlet extends HttpServlet { }; rp.receive(getInputStream(req), out, null); out.close(); + } catch (UnpackException e) { + // This should be already reported to the client. + getServletContext().log( + HttpServerText.get().internalErrorDuringReceivePack, + e.getCause()); + } catch (IOException e) { getServletContext().log(HttpServerText.get().internalErrorDuringReceivePack, e); - rsp.sendError(SC_INTERNAL_SERVER_ERROR); + if (!rsp.isCommitted()) { + rsp.reset(); + rsp.sendError(SC_INTERNAL_SERVER_ERROR); + } return; } } diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java index 2b9e81f1d..178473c40 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java @@ -68,6 +68,7 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser; import org.eclipse.jgit.transport.UploadPack; +import org.eclipse.jgit.transport.UploadPackInternalServerErrorException; import org.eclipse.jgit.transport.UploadPackMayNotContinueException; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; @@ -175,14 +176,23 @@ class UploadPackServlet extends HttpServlet { out.close(); } catch (UploadPackMayNotContinueException e) { - if (!e.isOutput()) + if (!e.isOutput() && !rsp.isCommitted()) { + rsp.reset(); rsp.sendError(SC_SERVICE_UNAVAILABLE); + } return; + } catch (UploadPackInternalServerErrorException e) { + getServletContext().log( + HttpServerText.get().internalErrorDuringUploadPack, + e.getCause()); + } catch (IOException e) { getServletContext().log(HttpServerText.get().internalErrorDuringUploadPack, e); - rsp.reset(); - rsp.sendError(SC_INTERNAL_SERVER_ERROR); + if (!rsp.isCommitted()) { + rsp.reset(); + rsp.sendError(SC_INTERNAL_SERVER_ERROR); + } return; } } diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties index 165ad9d40..570895e81 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties @@ -224,6 +224,7 @@ indexSignatureIsInvalid=Index signature is invalid: {0} indexWriteException=Modified index could not be written integerValueOutOfRange=Integer value {0}.{1} out of range internalRevisionError=internal revision error +internalServerError=internal server error interruptedWriting=Interrupted writing {0} inTheFuture=in the future invalidAdvertisementOf=invalid advertisement of {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java index c0f7a1479..7054cf800 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java @@ -284,6 +284,7 @@ public class JGitText extends TranslationBundle { /***/ public String indexWriteException; /***/ public String integerValueOutOfRange; /***/ public String internalRevisionError; + /***/ public String internalServerError; /***/ public String interruptedWriting; /***/ public String inTheFuture; /***/ public String invalidAdvertisementOf; 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 0e50b937b..41f64a163 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -401,25 +401,62 @@ public class UploadPack { } } - recvWants(); - if (wantIds.isEmpty()) { - preUploadHook.onBeginNegotiateRound(this, wantIds, 0); - preUploadHook.onEndNegotiateRound(this, wantIds, 0, 0, false); - return; - } + boolean sendPack; + try { + recvWants(); + if (wantIds.isEmpty()) { + preUploadHook.onBeginNegotiateRound(this, wantIds, 0); + preUploadHook.onEndNegotiateRound(this, wantIds, 0, 0, false); + return; + } - if (options.contains(OPTION_MULTI_ACK_DETAILED)) { - multiAck = MultiAck.DETAILED; - noDone = options.contains(OPTION_NO_DONE); - } else if (options.contains(OPTION_MULTI_ACK)) - multiAck = MultiAck.CONTINUE; - else - multiAck = MultiAck.OFF; + if (options.contains(OPTION_MULTI_ACK_DETAILED)) { + multiAck = MultiAck.DETAILED; + noDone = options.contains(OPTION_NO_DONE); + } else if (options.contains(OPTION_MULTI_ACK)) + multiAck = MultiAck.CONTINUE; + else + multiAck = MultiAck.OFF; + + sendPack = negotiate(); + } catch (PackProtocolException err) { + reportErrorDuringNegotiate(err.getMessage()); + throw err; + + } catch (UploadPackMayNotContinueException err) { + if (!err.isOutput() && err.getMessage() != null) { + try { + pckOut.writeString("ERR " + err.getMessage() + "\n"); + err.setOutput(); + } catch (Throwable err2) { + // Ignore this secondary failure (and not mark output). + } + } + throw err; + + } catch (IOException err) { + reportErrorDuringNegotiate(JGitText.get().internalServerError); + throw err; + } catch (RuntimeException err) { + reportErrorDuringNegotiate(JGitText.get().internalServerError); + throw err; + } catch (Error err) { + reportErrorDuringNegotiate(JGitText.get().internalServerError); + throw err; + } - if (negotiate()) + if (sendPack) sendPack(); } + private void reportErrorDuringNegotiate(String msg) { + try { + pckOut.writeString("ERR " + msg + "\n"); + } catch (Throwable err) { + // Ignore this secondary failure. + } + } + /** * Generate an advertisement of available refs and capabilities. * @@ -536,16 +573,7 @@ public class UploadPack { private ObjectId processHaveLines(List peerHas, ObjectId last) throws IOException { - try { - preUploadHook.onBeginNegotiateRound(this, wantIds, peerHas.size()); - } catch (UploadPackMayNotContinueException fail) { - if (fail.getMessage() != null) { - pckOut.writeString("ERR " + fail.getMessage() + "\n"); - fail.setOutput(); - } - throw fail; - } - + preUploadHook.onBeginNegotiateRound(this, wantIds, peerHas.size()); if (peerHas.isEmpty()) return last; @@ -576,7 +604,6 @@ public class UploadPack { if (wantIds.contains(id)) { String msg = MessageFormat.format( JGitText.get().wantNotValid, id.name()); - pckOut.writeString("ERR " + msg); throw new PackProtocolException(msg, notFound); } continue; @@ -591,7 +618,6 @@ public class UploadPack { if (!advertised.contains(obj)) { String msg = MessageFormat.format( JGitText.get().wantNotValid, obj.name()); - pckOut.writeString("ERR " + msg); throw new PackProtocolException(msg); } @@ -689,17 +715,7 @@ public class UploadPack { sentReady = true; } - try { - preUploadHook.onEndNegotiateRound(this, wantAll, // - haveCnt, missCnt, sentReady); - } catch (UploadPackMayNotContinueException fail) { - if (fail.getMessage() != null) { - pckOut.writeString("ERR " + fail.getMessage() + "\n"); - fail.setOutput(); - } - throw fail; - } - + preUploadHook.onEndNegotiateRound(this, wantAll, haveCnt, missCnt, sentReady); peerHas.clear(); return last; } @@ -768,6 +784,49 @@ public class UploadPack { "\\x" + Integer.toHexString(eof))); } + if (sideband) { + try { + sendPack(true); + } catch (UploadPackMayNotContinueException noPack) { + // This was already reported on (below). + throw noPack; + } catch (IOException err) { + if (reportInternalServerErrorOverSideband()) + throw new UploadPackInternalServerErrorException(err); + else + throw err; + } catch (RuntimeException err) { + if (reportInternalServerErrorOverSideband()) + throw new UploadPackInternalServerErrorException(err); + else + throw err; + } catch (Error err) { + if (reportInternalServerErrorOverSideband()) + throw new UploadPackInternalServerErrorException(err); + else + throw err; + } + } else { + sendPack(false); + } + } + + private boolean reportInternalServerErrorOverSideband() { + try { + SideBandOutputStream err = new SideBandOutputStream( + SideBandOutputStream.CH_ERROR, + SideBandOutputStream.SMALL_BUF, + rawOut); + err.write(Constants.encode(JGitText.get().internalServerError)); + err.flush(); + return true; + } catch (Throwable cannotReport) { + // Ignore the reason. This is a secondary failure. + return false; + } + } + + private void sendPack(final boolean sideband) throws IOException { ProgressMonitor pm = NullProgressMonitor.INSTANCE; OutputStream packOut = rawOut; SideBandOutputStream msgOut = null; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPackInternalServerErrorException.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPackInternalServerErrorException.java new file mode 100644 index 000000000..0636984dc --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPackInternalServerErrorException.java @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2011, 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.transport; + +import java.io.IOException; + +/** UploadPack has already reported an error to the client. */ +public class UploadPackInternalServerErrorException extends IOException { + private static final long serialVersionUID = 1L; + + /** + * Initialize a new exception. + * + * @param why + * root cause. + */ + public UploadPackInternalServerErrorException(Throwable why) { + super(why); + } +}