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
This commit is contained in:
Sven Selberg 2022-11-09 16:58:17 +01:00
parent cb90ed0852
commit ac127a7932
2 changed files with 10 additions and 10 deletions

View File

@ -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;
}

View File

@ -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<String, Ref> 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