Merge branch 'stable-6.1' into stable-6.2

* stable-6.1:
  Extract Exception -> HTTP status code mapping for reuse
  Don't handle internal git errors as an HTTP error
  Allow to perform PackedBatchRefUpdate without locking loose refs

Change-Id: Icb321779184d20f3871e236fda1a3acba605a6da
This commit is contained in:
Matthias Sohn 2022-11-16 10:13:20 +01:00
commit a24b22632f
6 changed files with 66 additions and 22 deletions

View File

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

View File

@ -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 @@
* @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

View File

@ -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.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.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 @@ public void destroy() {
}
}
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) {

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

View File

@ -86,10 +86,16 @@
*/
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 @@ public void execute(RevWalk walk, ProgressMonitor monitor,
refdb.inProcessPackedRefsLock.lock();
try {
PackedRefList oldPackedList;
if (!refdb.isInClone()) {
if (!refdb.isInClone() && shouldLockLooseRefs) {
locks = lockLooseRefs(pending);
if (locks == null) {
return;

View File

@ -586,6 +586,21 @@ public PackedBatchRefUpdate newBatchUpdate() {
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() {