From ac127a7932e3dfa5ac09ccc527123fc0223be5ba Mon Sep 17 00:00:00 2001 From: Sven Selberg Date: Wed, 9 Nov 2022 16:58:17 +0100 Subject: [PATCH] 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 --- .../eclipse/jgit/http/server/UploadPackServlet.java | 8 ++++---- .../jgit/http/test/SmartClientSmartServerTest.java | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) 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 23a398f9d..509ea4cf5 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,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.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 @@ private static int statusCodeForThrowable(Throwable error) { 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 8f3888e4d..b9b10b45d 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 @@ -537,9 +537,9 @@ public void testFetchBySHA1Unreachable() throws Exception { 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 @@ protected Map getAdvertisedRefs(Repository repository, () -> 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 void testInvalidWant() throws Exception { 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