From ac127a7932e3dfa5ac09ccc527123fc0223be5ba Mon Sep 17 00:00:00 2001 From: Sven Selberg Date: Wed, 9 Nov 2022 16:58:17 +0100 Subject: 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 --- .../src/org/eclipse/jgit/http/server/UploadPackServlet.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'org.eclipse.jgit.http.server/src/org') 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 23a398f9db..509ea4cf55 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.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; } -- cgit v1.2.3 From 5bf7472e0ca967893f9fdb653c3f1b22fb6d56e5 Mon Sep 17 00:00:00 2001 From: Sven Selberg Date: Wed, 9 Nov 2022 18:28:45 +0100 Subject: Extract Exception -> HTTP status code mapping for reuse Extract private static method UploadPackServlet#statusCodeForThrowable to a public static method in the UploadPackErrorHandler interface so that implementers of this interface can reuse the default mapping. Change-Id: Ie4a0a006b0148d5b828d610c55d19ce407aab055 --- .../.settings/.api_filters | 11 ++++++++++ .../jgit/http/server/UploadPackErrorHandler.java | 24 ++++++++++++++++++++++ .../jgit/http/server/UploadPackServlet.java | 14 +------------ 3 files changed, 36 insertions(+), 13 deletions(-) create mode 100644 org.eclipse.jgit.http.server/.settings/.api_filters (limited to 'org.eclipse.jgit.http.server/src/org') diff --git a/org.eclipse.jgit.http.server/.settings/.api_filters b/org.eclipse.jgit.http.server/.settings/.api_filters new file mode 100644 index 0000000000..2c32c9864b --- /dev/null +++ b/org.eclipse.jgit.http.server/.settings/.api_filters @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java index 03be0873b0..2aadbbc984 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java @@ -9,13 +9,19 @@ */ package org.eclipse.jgit.http.server; +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 java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.transport.ServiceMayNotContinueException; import org.eclipse.jgit.transport.UploadPack; +import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; /** * Handle git-upload-pack errors. @@ -34,6 +40,24 @@ import org.eclipse.jgit.transport.UploadPack; * @since 5.6 */ public interface UploadPackErrorHandler { + /** + * Maps a thrown git related Exception to an appropriate HTTP status code. + * + * @param error + * The thrown Exception. + * @return the HTTP status code as an int + * @since 6.1.1 + */ + public static int statusCodeForThrowable(Throwable error) { + if (error instanceof ServiceNotEnabledException) { + return SC_FORBIDDEN; + } + if (error instanceof PackProtocolException) { + // Internal git errors are not errors from an HTTP standpoint. + return SC_OK; + } + return SC_INTERNAL_SERVER_ERROR; + } /** * @param req * The HTTP request 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 509ea4cf55..b0a07f1d5d 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 @@ -11,8 +11,6 @@ package org.eclipse.jgit.http.server; 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; @@ -23,6 +21,7 @@ import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER; import static org.eclipse.jgit.http.server.ServletUtils.consumeRequestBody; import static org.eclipse.jgit.http.server.ServletUtils.getInputStream; import static org.eclipse.jgit.http.server.ServletUtils.getRepository; +import static org.eclipse.jgit.http.server.UploadPackErrorHandler.statusCodeForThrowable; import static org.eclipse.jgit.util.HttpSupport.HDR_USER_AGENT; import java.io.IOException; @@ -152,17 +151,6 @@ class UploadPackServlet extends HttpServlet { } } - private static int statusCodeForThrowable(Throwable error) { - if (error instanceof ServiceNotEnabledException) { - return SC_FORBIDDEN; - } - if (error instanceof PackProtocolException) { - // Internal git errors is not an error from an HTTP standpoint. - return SC_OK; - } - return SC_INTERNAL_SERVER_ERROR; - } - private final UploadPackErrorHandler handler; UploadPackServlet(@Nullable UploadPackErrorHandler handler) { -- cgit v1.2.3