diff options
6 files changed, 66 insertions, 22 deletions
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 @@ +<?xml version="1.0" encoding="UTF-8" standalone="no"?> +<component id="org.eclipse.jgit.http.server" version="2"> + <resource path="src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java" type="org.eclipse.jgit.http.server.UploadPackErrorHandler"> + <filter id="1210056707"> + <message_arguments> + <message_argument value="6.1.1"/> + <message_argument value="statusCodeForThrowable(Throwable)"/> + </message_arguments> + </filter> + </resource> +</component> 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. @@ -35,6 +41,24 @@ import org.eclipse.jgit.transport.UploadPack; */ 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 * @param rsp 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..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 @@ -10,9 +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; 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; @@ -49,7 +48,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; @@ -153,16 +151,6 @@ 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) { 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 8f3888e4d5..b9b10b45d0 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 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 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java index 9c1d33dc3c..8b0ea4fcd6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java @@ -86,10 +86,16 @@ import org.eclipse.jgit.util.RefList; */ class PackedBatchRefUpdate extends BatchRefUpdate { private RefDirectory refdb; + private boolean shouldLockLooseRefs; PackedBatchRefUpdate(RefDirectory refdb) { - super(refdb); - this.refdb = refdb; + this(refdb, true); + } + + PackedBatchRefUpdate(RefDirectory refdb, boolean shouldLockLooseRefs) { + super(refdb); + this.refdb = refdb; + this.shouldLockLooseRefs = shouldLockLooseRefs; } /** {@inheritDoc} */ @@ -155,7 +161,7 @@ class PackedBatchRefUpdate extends BatchRefUpdate { refdb.inProcessPackedRefsLock.lock(); try { PackedRefList oldPackedList; - if (!refdb.isInClone()) { + if (!refdb.isInClone() && shouldLockLooseRefs) { locks = lockLooseRefs(pending); if (locks == null) { return; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java index 4aa2edff38..7d3792ef46 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java @@ -586,6 +586,21 @@ public class RefDirectory extends RefDatabase { return new PackedBatchRefUpdate(this); } + /** + * Create a new batch update to attempt on this database. + * + * @param shouldLockLooseRefs + * whether loose refs should be locked during the batch ref + * update. Note that this should only be set to {@code false} if + * the application using this ensures that no other ref updates + * run concurrently to avoid lost updates caused by a race. In + * such cases it can improve performance. + * @return a new batch update object + */ + public PackedBatchRefUpdate newBatchUpdate(boolean shouldLockLooseRefs) { + return new PackedBatchRefUpdate(this, shouldLockLooseRefs); + } + /** {@inheritDoc} */ @Override public boolean performsAtomicTransactions() { |