From 58ed0cb840b29139483ce3b499354a6e5669498b Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Wed, 16 Mar 2011 19:19:15 -0700 Subject: [PATCH] smart HTTP: Return errors inside payload When the client is clearly making a smart HTTP request to our smart HTTP server, return any errors like RepositoryNotFoundException or ServiceNotEnabledException inside of the payload as a Git level ERR message, rather than an HTTP error code. This prevents the C Git command line client from retrying a failed "$URL/info/refs?service=git-upload-pack" request without the smart service URL, only to fail again with "403 Forbidden" when the dumb as-is service has been disabled by the server configuration, or is unavailable because the repository is not on the local filesystem. Change-Id: I57e8756d5026e885e0ca615979bfcd729703be6c Signed-off-by: Shawn O. Pearce --- .../http/server/HttpServerText.properties | 2 + .../jgit/http/server/HttpServerText.java | 2 + .../jgit/http/server/ReceivePackServlet.java | 2 +- .../jgit/http/server/RepositoryFilter.java | 60 +++++++++++++++++-- .../jgit/http/server/UploadPackServlet.java | 4 +- .../http/test/SmartClientSmartServerTest.java | 32 ++++++++++ 6 files changed, 95 insertions(+), 7 deletions(-) diff --git a/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties b/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties index 6232f47f0..a7643c5f9 100644 --- a/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties +++ b/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties @@ -13,6 +13,8 @@ noResolverAvailable=No resolver available parameterNotSet=Parameter {0} not set pathForParamNotFound={0} (for {1}) not found pathNotSupported={0} not supported +repositoryAccessForbidden=Git access forbidden +repositoryNotFound=Git repository not found servletAlreadyInitialized=Servlet already initialized servletMustNotBeNull=servlet must not be null servletWasAlreadyBound=servlet was already bound diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/HttpServerText.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/HttpServerText.java index fc1001442..18743989b 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/HttpServerText.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/HttpServerText.java @@ -73,6 +73,8 @@ public class HttpServerText extends TranslationBundle { /***/ public String parameterNotSet; /***/ public String pathForParamNotFound; /***/ public String pathNotSupported; + /***/ public String repositoryAccessForbidden; + /***/ public String repositoryNotFound; /***/ public String servletAlreadyInitialized; /***/ public String servletMustNotBeNull; /***/ public String servletWasAlreadyBound; 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 69b5aec25..192050a17 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 @@ -128,7 +128,7 @@ class ReceivePackServlet extends HttpServlet { return; } catch (ServiceNotEnabledException e) { - rsp.sendError(SC_FORBIDDEN); + RepositoryFilter.sendError(SC_FORBIDDEN, req, rsp); return; } diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/RepositoryFilter.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/RepositoryFilter.java index 1097932ec..3e0a57256 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/RepositoryFilter.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/RepositoryFilter.java @@ -48,6 +48,7 @@ import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY; +import static org.eclipse.jgit.util.HttpSupport.HDR_ACCEPT; import java.io.IOException; import java.text.MessageFormat; @@ -64,6 +65,7 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.PacketLineOut; import org.eclipse.jgit.transport.resolver.RepositoryResolver; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; @@ -131,14 +133,14 @@ public class RepositoryFilter implements Filter { try { db = resolver.open(req, name); } catch (RepositoryNotFoundException e) { - ((HttpServletResponse) rsp).sendError(SC_NOT_FOUND); + sendError(SC_NOT_FOUND, req, (HttpServletResponse) rsp); + return; + } catch (ServiceNotEnabledException e) { + sendError(SC_FORBIDDEN, req, (HttpServletResponse) rsp); return; } catch (ServiceNotAuthorizedException e) { ((HttpServletResponse) rsp).sendError(SC_UNAUTHORIZED); return; - } catch (ServiceNotEnabledException e) { - ((HttpServletResponse) rsp).sendError(SC_FORBIDDEN); - return; } try { request.setAttribute(ATTRIBUTE_REPOSITORY, db); @@ -148,4 +150,54 @@ public class RepositoryFilter implements Filter { db.close(); } } + + static void sendError(int statusCode, HttpServletRequest req, + HttpServletResponse rsp) throws IOException { + String svc = req.getParameter("service"); + String accept = req.getHeader(HDR_ACCEPT); + + if (svc != null && svc.startsWith("git-") && accept != null + && accept.contains("application/x-" + svc + "-advertisement")) { + // Smart HTTP service request, use an ERR response. + rsp.setContentType("application/x-" + svc + "-advertisement"); + + SmartOutputStream buf = new SmartOutputStream(req, rsp); + PacketLineOut out = new PacketLineOut(buf); + out.writeString("# service=" + svc + "\n"); + out.end(); + out.writeString("ERR " + translate(statusCode)); + buf.close(); + return; + } + + if (accept != null && accept.contains(UploadPackServlet.RSP_TYPE)) { + // An upload-pack wants ACK or NAK, return ERR + // and the client will print this instead. + rsp.setContentType(UploadPackServlet.RSP_TYPE); + SmartOutputStream buf = new SmartOutputStream(req, rsp); + PacketLineOut out = new PacketLineOut(buf); + out.writeString("ERR " + translate(statusCode)); + buf.close(); + return; + } + + // Otherwise fail with an HTTP error code instead of an + // application level message. This may not be as pretty + // of a result for the user, but its better than nothing. + // + rsp.sendError(statusCode); + } + + private static String translate(int statusCode) { + switch (statusCode) { + case SC_NOT_FOUND: + return HttpServerText.get().repositoryNotFound; + + case SC_FORBIDDEN: + return HttpServerText.get().repositoryAccessForbidden; + + default: + return String.valueOf(statusCode); + } + } } 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 e60c5068c..1ceb0965a 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 @@ -77,7 +77,7 @@ import org.eclipse.jgit.transport.resolver.UploadPackFactory; class UploadPackServlet extends HttpServlet { private static final String REQ_TYPE = "application/x-git-upload-pack-request"; - private static final String RSP_TYPE = "application/x-git-upload-pack-result"; + static final String RSP_TYPE = "application/x-git-upload-pack-result"; private static final long serialVersionUID = 1L; @@ -130,7 +130,7 @@ class UploadPackServlet extends HttpServlet { return; } catch (ServiceNotEnabledException e) { - rsp.sendError(SC_FORBIDDEN); + RepositoryFilter.sendError(SC_FORBIDDEN, req, rsp); return; } diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java index 209f161c9..c3590a44f 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java @@ -55,6 +55,7 @@ import static org.junit.Assert.fail; import java.io.IOException; import java.io.PrintWriter; +import java.net.URISyntaxException; import java.util.Collections; import java.util.List; import java.util.Map; @@ -73,6 +74,7 @@ import org.eclipse.jetty.servlet.FilterMapping; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jgit.JGitText; +import org.eclipse.jgit.errors.RemoteRepositoryException; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.errors.TransportException; import org.eclipse.jgit.http.server.GitServlet; @@ -223,6 +225,36 @@ public class SmartClientSmartServerTest extends HttpTestCase { assertEquals("gzip", info.getResponseHeader(HDR_CONTENT_ENCODING)); } + @Test + public void testListRemote_BadName() throws IOException, URISyntaxException { + Repository dst = createBareRepository(); + URIish uri = new URIish(this.remoteURI.toString() + ".invalid"); + Transport t = Transport.open(dst, uri); + try { + try { + t.openFetch(); + fail("fetch connection opened"); + } catch (RemoteRepositoryException notFound) { + assertEquals(uri + ": Git repository not found", + notFound.getMessage()); + } + } finally { + t.close(); + } + + List requests = getRequests(); + assertEquals(1, requests.size()); + + AccessEvent info = requests.get(0); + assertEquals("GET", info.getMethod()); + assertEquals(join(uri, "info/refs"), info.getPath()); + assertEquals(1, info.getParameters().size()); + assertEquals("git-upload-pack", info.getParameter("service")); + assertEquals(200, info.getStatus()); + assertEquals("application/x-git-upload-pack-advertisement", + info.getResponseHeader(HDR_CONTENT_TYPE)); + } + @Test public void testInitialClone_Small() throws Exception { Repository dst = createBareRepository();