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 <spearce@spearce.org>

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 6232f47f05..a7643c5f9e 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 fc10014427..18743989ba 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 69b5aec25e..192050a17c 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 1097932ece..3e0a572564 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 e60c5068cf..1ceb0965a3 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 209f161c9b..c3590a44fb 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;
@@ -224,6 +226,36 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 	}
 
 	@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<AccessEvent> 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();
 		assertFalse(dst.hasObject(A_txt));