Don't handle internal git errors as an HTTP error

The fix that fixed the propagation of error-codes:
  8984e1f66 HTTP Smart: set correct HTTP status on error [1]
made some faulty assumptions.

"Wants not valid", can be an intermittent git error and the HTTP
response should be 200 and not 400 since the request isn't necessary
faulty.

[1] https://git.eclipse.org/r/c/jgit/jgit/+/192677

Bug: 579676
Change-Id: I461bc78ff6e450636811ece50d21c57a2a7f2ae3

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 23a398f9dbbdfda82675011e7eeac7878c63eeb5..509ea4cf55dbf223c554647438779a3f53af3078 100644 (file)
--- 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,9 +10,9 @@
 
 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_OK;
 import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
 import static javax.servlet.http.HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE;
 import static org.eclipse.jgit.http.server.GitSmartHttpTools.UPLOAD_PACK;
@@ -49,7 +49,6 @@ 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;
@@ -157,8 +156,9 @@ class UploadPackServlet extends HttpServlet {
                if (error instanceof ServiceNotEnabledException) {
                        return SC_FORBIDDEN;
                }
-               if (error instanceof WantNotValidException) {
-                       return SC_BAD_REQUEST;
+               if (error instanceof PackProtocolException) {
+                       // Internal git errors is not an error from an HTTP standpoint.
+                       return SC_OK;
                }
                return SC_INTERNAL_SERVER_ERROR;
        }

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 8f3888e4d5544679a0b856559c93196d01bf9fb1..b9b10b45d04f08ac60b6e5d1682ebb655f05c81a 100644 (file)
--- 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
@@ -537,9 +537,9 @@ public class SmartClientSmartServerTest extends AllProtocolsHttpTestCase {
                                                        Collections.singletonList(
                                                                        new RefSpec(unreachableCommit.name()))));
                        assertTrue(e.getMessage().contains(
-                                       "Bad Request"));
+                                       "want " + unreachableCommit.name() + " not valid"));
                }
-               assertLastRequestStatusCode(400);
+               assertLastRequestStatusCode(200);
        }
 
        @Test
@@ -560,9 +560,9 @@ public class SmartClientSmartServerTest extends AllProtocolsHttpTestCase {
                                        () -> t.fetch(NullProgressMonitor.INSTANCE,
                                                        Collections.singletonList(new RefSpec(A.name()))));
                        assertTrue(
-                                       e.getMessage().contains("Bad Request"));
+                                       e.getMessage().contains("want " + A.name() + " not valid"));
                }
-               assertLastRequestStatusCode(400);
+               assertLastRequestStatusCode(200);
        }
 
        @Test
@@ -1610,9 +1610,9 @@ public class SmartClientSmartServerTest extends AllProtocolsHttpTestCase {
                        fail("Server accepted want " + id.name());
                } catch (TransportException err) {
                        assertTrue(err.getMessage()
-                                       .contains("Bad Request"));
+                                       .contains("want " + id.name() + " not valid"));
                }
-               assertLastRequestStatusCode(400);
+               assertLastRequestStatusCode(200);
        }
 
        @Test