From d8c3e98d73a4fb7409580a7d9f3395ac506f7e5d Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 11 Feb 2010 10:58:22 -0800 Subject: [PATCH] Use "ERR message" for early ReceivePack problems If the application wants to, it can use sendError(String) to send one or more error messages to clients before the advertisements are sent. These will cause a C Git client to break out of the advertisement parsing loop, display "remote error: message\n", and terminate. Servers can optionally use this to send a detailed error to a client explaining why it cannot use the ReceivePack service on a repository. Over smart HTTP these errors are sent in a 200 OK response, and are in the payload, allowing the Git client to give the end-user the custom message rather than the generic error "403 Forbidden". Change-Id: I03f4345183765d21002118617174c77f71427b5a Signed-off-by: Shawn O. Pearce --- .../jgit/http/test/AdvertiseErrorTest.java | 151 ++++++++++++++++++ .../eclipse/jgit/transport/ReceivePack.java | 39 ++++- 2 files changed, 182 insertions(+), 8 deletions(-) create mode 100644 org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/AdvertiseErrorTest.java 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);