diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/AdvertiseErrorTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/AdvertiseErrorTest.java new file mode 100644 index 000000000..47d7806a1 --- /dev/null +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/AdvertiseErrorTest.java @@ -0,0 +1,151 @@ +/* + * 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.http.test; + +import java.util.Collections; + +import javax.servlet.http.HttpServletRequest; + +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jgit.errors.RemoteRepositoryException; +import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.http.server.GitServlet; +import org.eclipse.jgit.http.server.resolver.DefaultReceivePackFactory; +import org.eclipse.jgit.http.server.resolver.RepositoryResolver; +import org.eclipse.jgit.http.server.resolver.ServiceNotAuthorizedException; +import org.eclipse.jgit.http.server.resolver.ServiceNotEnabledException; +import org.eclipse.jgit.http.test.util.HttpTestCase; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.RepositoryConfig; +import org.eclipse.jgit.revwalk.RevBlob; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.transport.ReceivePack; +import org.eclipse.jgit.transport.RemoteRefUpdate; +import org.eclipse.jgit.transport.Transport; +import org.eclipse.jgit.transport.URIish; + +public class AdvertiseErrorTest extends HttpTestCase { + private Repository remoteRepository; + + private URIish remoteURI; + + protected void setUp() throws Exception { + super.setUp(); + + final TestRepository src = createTestRepository(); + final String srcName = src.getRepository().getDirectory().getName(); + + ServletContextHandler app = server.addContext("/git"); + GitServlet gs = new GitServlet(); + gs.setRepositoryResolver(new RepositoryResolver() { + public Repository open(HttpServletRequest req, String name) + throws RepositoryNotFoundException, + ServiceNotEnabledException { + if (!name.equals(srcName)) + throw new RepositoryNotFoundException(name); + + final Repository db = src.getRepository(); + db.incrementOpen(); + return db; + } + }); + gs.setReceivePackFactory(new DefaultReceivePackFactory() { + public ReceivePack create(HttpServletRequest req, Repository db) + throws ServiceNotEnabledException, + ServiceNotAuthorizedException { + ReceivePack rp = super.create(req, db); + rp.sendError("message line 1"); + rp.sendError("no soup for you!"); + rp.sendError("come back next year!"); + return rp; + } + + }); + app.addServlet(new ServletHolder(gs), "/*"); + + server.setUp(); + + remoteRepository = src.getRepository(); + remoteURI = toURIish(app, srcName); + + RepositoryConfig cfg = remoteRepository.getConfig(); + cfg.setBoolean("http", null, "receivepack", true); + cfg.save(); + } + + public void testPush_CreateBranch() throws Exception { + final TestRepository src = createTestRepository(); + final RevBlob Q_txt = src.blob("new text"); + final RevCommit Q = src.commit().add("Q", Q_txt).create(); + final Repository db = src.getRepository(); + final String dstName = Constants.R_HEADS + "new.branch"; + final Transport t = Transport.open(db, remoteURI); + try { + final String srcExpr = Q.name(); + final boolean forceUpdate = false; + final String localName = null; + final ObjectId oldId = null; + + RemoteRefUpdate update = new RemoteRefUpdate(src.getRepository(), + srcExpr, dstName, forceUpdate, localName, oldId); + try { + t.push(NullProgressMonitor.INSTANCE, Collections + .singleton(update)); + fail("push completed without throwing exception"); + } catch (RemoteRepositoryException error) { + assertEquals(remoteURI + ": message line 1\n" // + + "no soup for you!\n" // + + "come back next year!", // + error.getMessage()); + } + } finally { + t.close(); + } + } +} 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 c5bbdac5a..d79965870 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -163,6 +163,9 @@ public class ReceivePack { /** Commands to execute, as received by the client. */ private List commands; + /** Error to display instead of advertising the references. */ + private StringBuilder advertiseError; + /** An exception caught while unpacking and fsck'ing the objects. */ private Throwable unpackError; @@ -428,10 +431,17 @@ public List getAllCommands() { } /** - * Send an error message to the client, if it supports receiving them. + * Send an error message to the client. *

- * If the client doesn't support receiving messages, the message will be - * discarded, with no other indication to the caller or to the client. + * If any error messages are sent before the references are advertised to + * the client, the errors will be sent instead of the advertisement and the + * receive operation will be aborted. All clients should receive and display + * such early stage errors. + *

+ * If the reference advertisements have already been sent, messages are sent + * in a side channel. If the client doesn't support receiving messages, the + * message will be discarded, with no other indication to the caller or to + * the client. *

* {@link PreReceiveHook}s should always try to use * {@link ReceiveCommand#setResult(Result, String)} with a result status of @@ -444,11 +454,17 @@ public List getAllCommands() { * string must not end with an LF, and must not contain an LF. */ public void sendError(final String what) { - try { - if (msgs != null) - msgs.write("error: " + what + "\n"); - } catch (IOException e) { - // Ignore write failures. + if (refs == null) { + if (advertiseError == null) + advertiseError = new StringBuilder(); + advertiseError.append(what).append('\n'); + } else { + try { + if (msgs != null) + msgs.write("error: " + what + "\n"); + } catch (IOException e) { + // Ignore write failures. + } } } @@ -558,6 +574,8 @@ private void service() throws IOException { sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut)); else refs = refFilter.filter(db.getAllRefs()); + if (advertiseError != null) + return; recvCommands(); if (!commands.isEmpty()) { enableCapabilities(); @@ -618,6 +636,11 @@ private void unlockPack() { * the formatter failed to write an advertisement. */ public void sendAdvertisedRefs(final RefAdvertiser adv) throws IOException { + if (advertiseError != null) { + adv.writeOne("ERR " + advertiseError); + return; + } + final RevFlag advertised = walk.newFlag("ADVERTISED"); adv.init(walk, advertised); adv.advertiseCapability(CAPABILITY_SIDE_BAND_64K);