Merge branch 'stable-6.2' into stable-6.3
* stable-6.2: 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: I562be0802efa231023c5f10e6461339b2d7fbacf
This commit is contained in:
commit
1cd9a1f804
|
@ -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>
|
|
@ -9,13 +9,19 @@
|
||||||
*/
|
*/
|
||||||
package org.eclipse.jgit.http.server;
|
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 java.io.IOException;
|
||||||
|
|
||||||
import javax.servlet.http.HttpServletRequest;
|
import javax.servlet.http.HttpServletRequest;
|
||||||
import javax.servlet.http.HttpServletResponse;
|
import javax.servlet.http.HttpServletResponse;
|
||||||
|
|
||||||
|
import org.eclipse.jgit.errors.PackProtocolException;
|
||||||
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
|
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
|
||||||
import org.eclipse.jgit.transport.UploadPack;
|
import org.eclipse.jgit.transport.UploadPack;
|
||||||
|
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Handle git-upload-pack errors.
|
* Handle git-upload-pack errors.
|
||||||
|
@ -34,6 +40,24 @@
|
||||||
* @since 5.6
|
* @since 5.6
|
||||||
*/
|
*/
|
||||||
public interface UploadPackErrorHandler {
|
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
|
* @param req
|
||||||
* The HTTP request
|
* The HTTP request
|
||||||
|
|
|
@ -10,9 +10,7 @@
|
||||||
|
|
||||||
package org.eclipse.jgit.http.server;
|
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_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_UNAUTHORIZED;
|
||||||
import static javax.servlet.http.HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE;
|
import static javax.servlet.http.HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE;
|
||||||
import static org.eclipse.jgit.http.server.GitSmartHttpTools.UPLOAD_PACK;
|
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.consumeRequestBody;
|
||||||
import static org.eclipse.jgit.http.server.ServletUtils.getInputStream;
|
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.ServletUtils.getRepository;
|
||||||
|
import static org.eclipse.jgit.http.server.UploadPackErrorHandler.statusCodeForThrowable;
|
||||||
import static org.eclipse.jgit.util.HttpSupport.HDR_USER_AGENT;
|
import static org.eclipse.jgit.util.HttpSupport.HDR_USER_AGENT;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
@ -49,7 +48,6 @@
|
||||||
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
|
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
|
||||||
import org.eclipse.jgit.transport.UploadPack;
|
import org.eclipse.jgit.transport.UploadPack;
|
||||||
import org.eclipse.jgit.transport.UploadPackInternalServerErrorException;
|
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.ServiceNotAuthorizedException;
|
||||||
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
|
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
|
||||||
import org.eclipse.jgit.transport.resolver.UploadPackFactory;
|
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;
|
private final UploadPackErrorHandler handler;
|
||||||
|
|
||||||
UploadPackServlet(@Nullable UploadPackErrorHandler handler) {
|
UploadPackServlet(@Nullable UploadPackErrorHandler handler) {
|
||||||
|
|
|
@ -537,9 +537,9 @@ public void testFetchBySHA1Unreachable() throws Exception {
|
||||||
Collections.singletonList(
|
Collections.singletonList(
|
||||||
new RefSpec(unreachableCommit.name()))));
|
new RefSpec(unreachableCommit.name()))));
|
||||||
assertTrue(e.getMessage().contains(
|
assertTrue(e.getMessage().contains(
|
||||||
"Bad Request"));
|
"want " + unreachableCommit.name() + " not valid"));
|
||||||
}
|
}
|
||||||
assertLastRequestStatusCode(400);
|
assertLastRequestStatusCode(200);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -560,9 +560,9 @@ protected Map<String, Ref> getAdvertisedRefs(Repository repository,
|
||||||
() -> t.fetch(NullProgressMonitor.INSTANCE,
|
() -> t.fetch(NullProgressMonitor.INSTANCE,
|
||||||
Collections.singletonList(new RefSpec(A.name()))));
|
Collections.singletonList(new RefSpec(A.name()))));
|
||||||
assertTrue(
|
assertTrue(
|
||||||
e.getMessage().contains("Bad Request"));
|
e.getMessage().contains("want " + A.name() + " not valid"));
|
||||||
}
|
}
|
||||||
assertLastRequestStatusCode(400);
|
assertLastRequestStatusCode(200);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -1610,9 +1610,9 @@ public void testInvalidWant() throws Exception {
|
||||||
fail("Server accepted want " + id.name());
|
fail("Server accepted want " + id.name());
|
||||||
} catch (TransportException err) {
|
} catch (TransportException err) {
|
||||||
assertTrue(err.getMessage()
|
assertTrue(err.getMessage()
|
||||||
.contains("Bad Request"));
|
.contains("want " + id.name() + " not valid"));
|
||||||
}
|
}
|
||||||
assertLastRequestStatusCode(400);
|
assertLastRequestStatusCode(200);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|
|
@ -86,10 +86,16 @@
|
||||||
*/
|
*/
|
||||||
class PackedBatchRefUpdate extends BatchRefUpdate {
|
class PackedBatchRefUpdate extends BatchRefUpdate {
|
||||||
private RefDirectory refdb;
|
private RefDirectory refdb;
|
||||||
|
private boolean shouldLockLooseRefs;
|
||||||
|
|
||||||
PackedBatchRefUpdate(RefDirectory refdb) {
|
PackedBatchRefUpdate(RefDirectory refdb) {
|
||||||
|
this(refdb, true);
|
||||||
|
}
|
||||||
|
|
||||||
|
PackedBatchRefUpdate(RefDirectory refdb, boolean shouldLockLooseRefs) {
|
||||||
super(refdb);
|
super(refdb);
|
||||||
this.refdb = refdb;
|
this.refdb = refdb;
|
||||||
|
this.shouldLockLooseRefs = shouldLockLooseRefs;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** {@inheritDoc} */
|
/** {@inheritDoc} */
|
||||||
|
@ -155,7 +161,7 @@ public void execute(RevWalk walk, ProgressMonitor monitor,
|
||||||
refdb.inProcessPackedRefsLock.lock();
|
refdb.inProcessPackedRefsLock.lock();
|
||||||
try {
|
try {
|
||||||
PackedRefList oldPackedList;
|
PackedRefList oldPackedList;
|
||||||
if (!refdb.isInClone()) {
|
if (!refdb.isInClone() && shouldLockLooseRefs) {
|
||||||
locks = lockLooseRefs(pending);
|
locks = lockLooseRefs(pending);
|
||||||
if (locks == null) {
|
if (locks == null) {
|
||||||
return;
|
return;
|
||||||
|
|
|
@ -586,6 +586,21 @@ public PackedBatchRefUpdate newBatchUpdate() {
|
||||||
return new PackedBatchRefUpdate(this);
|
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} */
|
/** {@inheritDoc} */
|
||||||
@Override
|
@Override
|
||||||
public boolean performsAtomicTransactions() {
|
public boolean performsAtomicTransactions() {
|
||||||
|
|
Loading…
Reference in New Issue