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>

5 files changed, 63 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;
 			}