motiejus
/
jgit
1
Fork 0
Code Releases Activity

smart HTTP server: Pass along "want X not valid" to client 

If the client sends a SHA-1 that the server does not recognize echo
this back to the client with an explicit error message instead of
the generic "internal server error".

This was always the intent of the implementation but it was being
dropped on smart HTTP due to the UploadPackServlet catching the
PackProtocolException, discarding the buffered message UploadPack
meant to send, and sending along a generic message instead.

Change-Id: I8d96b064ec655aef64ac2ef3e01853625af32cd1
This commit is contained in:
Shawn Pearce 2016-02-16 15:09:05 -08:00
parent ff5c756c79
commit e7a6e85b95
2 changed files with 36 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
View File

@ -56,6 +56,7 @@
import java.io.IOException;
import java.io.PrintWriter;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@ -89,6 +90,8 @@
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectIdRef;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.ReflogEntry;
import org.eclipse.jgit.lib.ReflogReader;
@ -486,6 +489,25 @@ public void testInitialClone_BrokenServer() throws Exception {
service.getResponseHeader(HDR_CONTENT_TYPE));
}
@Test
public void testInvalidWant() throws Exception {
@SuppressWarnings("resource")
ObjectId id = new ObjectInserter.Formatter().idFor(Constants.OBJ_BLOB,
"testInvalidWant".getBytes(StandardCharsets.UTF_8));
Repository dst = createBareRepository();
try (Transport t = Transport.open(dst, remoteURI);
FetchConnection c = t.openFetch()) {
Ref want = new ObjectIdRef.Unpeeled(Ref.Storage.NETWORK, id.name(),
id);
c.fetch(NullProgressMonitor.INSTANCE, Collections.singleton(want),
Collections.<ObjectId> emptySet());
fail("Server accepted want " + id.name());
} catch (TransportException err) {
assertEquals("want " + id.name() + " not valid", err.getMessage());
}
}
@Test
public void testPush_NotAuthorized() throws Exception {
final TestRepository src = createTestRepository();

35
org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
View File

@ -742,10 +742,6 @@ else if (requestValidator instanceof AnyRequestValidator)
if (!clientShallowCommits.isEmpty())
walk.assumeShallow(clientShallowCommits);
sendPack = negotiate();
} catch (PackProtocolException err) {
reportErrorDuringNegotiate(err.getMessage());
throw err;
} catch (ServiceMayNotContinueException err) {
if (!err.isOutput() && err.getMessage() != null) {
try {
@ -756,15 +752,20 @@ else if (requestValidator instanceof AnyRequestValidator)
}
}
throw err;
} catch (IOException err) {
reportErrorDuringNegotiate(JGitText.get().internalServerError);
throw err;
} catch (RuntimeException err) {
reportErrorDuringNegotiate(JGitText.get().internalServerError);
throw err;
} catch (Error err) {
reportErrorDuringNegotiate(JGitText.get().internalServerError);
} catch (IOException | RuntimeException | Error err) {
boolean output = false;
try {
String msg = err instanceof PackProtocolException
? err.getMessage()
: JGitText.get().internalServerError;
pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
output = true;
} catch (Throwable err2) {
// Ignore this secondary failure, leave output false.
}
if (output) {
throw new UploadPackInternalServerErrorException(err);
}
throw err;
}
@ -781,14 +782,6 @@ private static Set<ObjectId> refIdSet(Collection<Ref> refs) {
return ids;
}
private void reportErrorDuringNegotiate(String msg) {
try {
pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
} catch (Throwable err) {
// Ignore this secondary failure.
}
}
private void processShallow() throws IOException {
try (DepthWalk.RevWalk depthWalk = new DepthWalk.RevWalk(
walk.getObjectReader(), depth)) {