HTTP Smart: set correct HTTP status on error

Previous behavior was that status code was automatically set to 200
regardless of reported status and according to HTTP Smart protocol[1]:

  If there is no repository at $GIT_URL, or the resource pointed to by
  a location matching $GIT_URL does not exist, the server MUST NOT
  respond with 200 OK response. A server SHOULD respond with
  404 Not Found, 410 Gone, or any other suitable HTTP status code which
  does not imply the resource exists as requested.

Since the jgit HTTP client isn't able to handle reading content from a
response reporting an error (calling HttpURLConnection#getInputStream
on a "failed" connection throws an exception and the internal interface
HttpConnection does not expose HttpURLConnection#getErrorStream) the
SmartClientSmartServerTest needed to be rewritten to expect the generic
response messages.

[1] https://git-scm.com/docs/http-protocol#_general_request_processing

Bug: 579676
Change-Id: Ibb942d02124a0bc279df09600b091354019ce064

2 files changed, 32 insertions, 14 deletions

diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java
index 012f9a3dc0..f1155dcf57 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java
@@ -158,11 +158,11 @@ public class GitSmartHttpTools {
 		}
 
 		if (isInfoRefs(req)) {
-			sendInfoRefsError(req, res, textForGit);
+			sendInfoRefsError(req, res, textForGit, httpStatus);
 		} else if (isUploadPack(req)) {
-			sendUploadPackError(req, res, textForGit);
+			sendUploadPackError(req, res, textForGit, httpStatus);
 		} else if (isReceivePack(req)) {
-			sendReceivePackError(req, res, textForGit);
+			sendReceivePackError(req, res, textForGit, httpStatus);
 		} else {
 			if (httpStatus < 400)
 				ServletUtils.consumeRequestBody(req);
@@ -171,29 +171,32 @@ public class GitSmartHttpTools {
 	}
 
 	private static void sendInfoRefsError(HttpServletRequest req,
-			HttpServletResponse res, String textForGit) throws IOException {
+			HttpServletResponse res, String textForGit, int httpStatus)
+			throws IOException {
 		ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
 		PacketLineOut pck = new PacketLineOut(buf);
 		String svc = req.getParameter("service");
 		pck.writeString("# service=" + svc + "\n");
 		pck.end();
 		pck.writeString("ERR " + textForGit);
-		send(req, res, infoRefsResultType(svc), buf.toByteArray());
+		send(req, res, infoRefsResultType(svc), buf.toByteArray(), httpStatus);
 	}
 
 	private static void sendUploadPackError(HttpServletRequest req,
-			HttpServletResponse res, String textForGit) throws IOException {
+			HttpServletResponse res, String textForGit, int httpStatus)
+			throws IOException {
 		// Do not use sideband. Sideband is acceptable only while packfile is
 		// being sent. Other places, like acknowledgement section, do not
 		// support sideband. Use an error packet.
 		ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
 		PacketLineOut pckOut = new PacketLineOut(buf);
 		writePacket(pckOut, textForGit);
-		send(req, res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray());
+		send(req, res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray(), httpStatus);
 	}
 
 	private static void sendReceivePackError(HttpServletRequest req,
-			HttpServletResponse res, String textForGit) throws IOException {
+			HttpServletResponse res, String textForGit, int httpStatus)
+			throws IOException {
 		ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
 		PacketLineOut pckOut = new PacketLineOut(buf);
 
@@ -212,7 +215,7 @@ public class GitSmartHttpTools {
 			writeSideBand(buf, textForGit);
 		else
 			writePacket(pckOut, textForGit);
-		send(req, res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray());
+		send(req, res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray(), httpStatus);
 	}
 
 	private static boolean isReceivePackSideBand(HttpServletRequest req) {
@@ -246,9 +249,9 @@ public class GitSmartHttpTools {
 	}
 
 	private static void send(HttpServletRequest req, HttpServletResponse res,
-			String type, byte[] buf) throws IOException {
+			String type, byte[] buf, int httpStatus) throws IOException {
 		ServletUtils.consumeRequestBody(req);
-		res.setStatus(HttpServletResponse.SC_OK);
+		res.setStatus(httpStatus);
 		res.setContentType(type);
 		res.setContentLength(buf.length);
 		try (OutputStream os = res.getOutputStream()) {
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 77ed09cdfb..d3437b7eb9 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
@@ -10,6 +10,7 @@
 
 package org.eclipse.jgit.http.server;
 
+import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
 import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
 import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
 import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
@@ -48,6 +49,7 @@ import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser;
 import org.eclipse.jgit.transport.ServiceMayNotContinueException;
 import org.eclipse.jgit.transport.UploadPack;
 import org.eclipse.jgit.transport.UploadPackInternalServerErrorException;
+import org.eclipse.jgit.transport.WantNotValidException;
 import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
 import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
 import org.eclipse.jgit.transport.resolver.UploadPackFactory;
@@ -151,6 +153,16 @@ class UploadPackServlet extends HttpServlet {
 		}
 	}
 
+	private static int statusCodeForThrowable(Throwable error) {
+		if (error instanceof ServiceNotEnabledException) {
+			return SC_FORBIDDEN;
+		}
+		if (error instanceof WantNotValidException) {
+			return SC_BAD_REQUEST;
+		}
+		return SC_INTERNAL_SERVER_ERROR;
+	}
+
 	private final UploadPackErrorHandler handler;
 
 	UploadPackServlet(@Nullable UploadPackErrorHandler handler) {
@@ -216,9 +228,12 @@ class UploadPackServlet extends HttpServlet {
 			log(up.getRepository(), e);
 			if (!rsp.isCommitted()) {
 				rsp.reset();
-				String msg = e instanceof PackProtocolException ? e.getMessage()
-						: null;
-				sendError(req, rsp, SC_INTERNAL_SERVER_ERROR, msg);
+				String msg = null;
+				if (e instanceof PackProtocolException
+						|| e instanceof ServiceNotEnabledException) {
+					msg = e.getMessage();
+				}
+				sendError(req, rsp, statusCodeForThrowable(e), msg);
 			}
 		}
 	}