Merge branch 'stable-6.2'
* stable-6.2: HTTP Smart: set correct HTTP status on error Change-Id: I7bf99b0c720f9dabb65da5cc777281a1d227f5a8
This commit is contained in:
commit
114325560a
|
@ -158,11 +158,11 @@ public static void sendError(HttpServletRequest req,
|
||||||
}
|
}
|
||||||
|
|
||||||
if (isInfoRefs(req)) {
|
if (isInfoRefs(req)) {
|
||||||
sendInfoRefsError(req, res, textForGit);
|
sendInfoRefsError(req, res, textForGit, httpStatus);
|
||||||
} else if (isUploadPack(req)) {
|
} else if (isUploadPack(req)) {
|
||||||
sendUploadPackError(req, res, textForGit);
|
sendUploadPackError(req, res, textForGit, httpStatus);
|
||||||
} else if (isReceivePack(req)) {
|
} else if (isReceivePack(req)) {
|
||||||
sendReceivePackError(req, res, textForGit);
|
sendReceivePackError(req, res, textForGit, httpStatus);
|
||||||
} else {
|
} else {
|
||||||
if (httpStatus < 400)
|
if (httpStatus < 400)
|
||||||
ServletUtils.consumeRequestBody(req);
|
ServletUtils.consumeRequestBody(req);
|
||||||
|
@ -171,29 +171,32 @@ public static void sendError(HttpServletRequest req,
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void sendInfoRefsError(HttpServletRequest req,
|
private static void sendInfoRefsError(HttpServletRequest req,
|
||||||
HttpServletResponse res, String textForGit) throws IOException {
|
HttpServletResponse res, String textForGit, int httpStatus)
|
||||||
|
throws IOException {
|
||||||
ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
|
ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
|
||||||
PacketLineOut pck = new PacketLineOut(buf);
|
PacketLineOut pck = new PacketLineOut(buf);
|
||||||
String svc = req.getParameter("service");
|
String svc = req.getParameter("service");
|
||||||
pck.writeString("# service=" + svc + "\n");
|
pck.writeString("# service=" + svc + "\n");
|
||||||
pck.end();
|
pck.end();
|
||||||
pck.writeString("ERR " + textForGit);
|
pck.writeString("ERR " + textForGit);
|
||||||
send(req, res, infoRefsResultType(svc), buf.toByteArray());
|
send(req, res, infoRefsResultType(svc), buf.toByteArray(), httpStatus);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void sendUploadPackError(HttpServletRequest req,
|
private static void sendUploadPackError(HttpServletRequest req,
|
||||||
HttpServletResponse res, String textForGit) throws IOException {
|
HttpServletResponse res, String textForGit, int httpStatus)
|
||||||
|
throws IOException {
|
||||||
// Do not use sideband. Sideband is acceptable only while packfile is
|
// Do not use sideband. Sideband is acceptable only while packfile is
|
||||||
// being sent. Other places, like acknowledgement section, do not
|
// being sent. Other places, like acknowledgement section, do not
|
||||||
// support sideband. Use an error packet.
|
// support sideband. Use an error packet.
|
||||||
ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
|
ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
|
||||||
PacketLineOut pckOut = new PacketLineOut(buf);
|
PacketLineOut pckOut = new PacketLineOut(buf);
|
||||||
writePacket(pckOut, textForGit);
|
writePacket(pckOut, textForGit);
|
||||||
send(req, res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray());
|
send(req, res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray(), httpStatus);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void sendReceivePackError(HttpServletRequest req,
|
private static void sendReceivePackError(HttpServletRequest req,
|
||||||
HttpServletResponse res, String textForGit) throws IOException {
|
HttpServletResponse res, String textForGit, int httpStatus)
|
||||||
|
throws IOException {
|
||||||
ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
|
ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
|
||||||
PacketLineOut pckOut = new PacketLineOut(buf);
|
PacketLineOut pckOut = new PacketLineOut(buf);
|
||||||
|
|
||||||
|
@ -212,7 +215,7 @@ private static void sendReceivePackError(HttpServletRequest req,
|
||||||
writeSideBand(buf, textForGit);
|
writeSideBand(buf, textForGit);
|
||||||
else
|
else
|
||||||
writePacket(pckOut, textForGit);
|
writePacket(pckOut, textForGit);
|
||||||
send(req, res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray());
|
send(req, res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray(), httpStatus);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static boolean isReceivePackSideBand(HttpServletRequest req) {
|
private static boolean isReceivePackSideBand(HttpServletRequest req) {
|
||||||
|
@ -246,9 +249,9 @@ private static void writePacket(PacketLineOut pckOut, String textForGit)
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void send(HttpServletRequest req, HttpServletResponse res,
|
private static void send(HttpServletRequest req, HttpServletResponse res,
|
||||||
String type, byte[] buf) throws IOException {
|
String type, byte[] buf, int httpStatus) throws IOException {
|
||||||
ServletUtils.consumeRequestBody(req);
|
ServletUtils.consumeRequestBody(req);
|
||||||
res.setStatus(HttpServletResponse.SC_OK);
|
res.setStatus(httpStatus);
|
||||||
res.setContentType(type);
|
res.setContentType(type);
|
||||||
res.setContentLength(buf.length);
|
res.setContentLength(buf.length);
|
||||||
try (OutputStream os = res.getOutputStream()) {
|
try (OutputStream os = res.getOutputStream()) {
|
||||||
|
|
|
@ -10,6 +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_INTERNAL_SERVER_ERROR;
|
||||||
import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
|
import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
|
||||||
|
@ -48,6 +49,7 @@
|
||||||
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;
|
||||||
|
@ -151,6 +153,16 @@ 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) {
|
||||||
|
@ -216,9 +228,12 @@ private void defaultUploadPackHandler(HttpServletRequest req,
|
||||||
log(up.getRepository(), e);
|
log(up.getRepository(), e);
|
||||||
if (!rsp.isCommitted()) {
|
if (!rsp.isCommitted()) {
|
||||||
rsp.reset();
|
rsp.reset();
|
||||||
String msg = e instanceof PackProtocolException ? e.getMessage()
|
String msg = null;
|
||||||
: null;
|
if (e instanceof PackProtocolException
|
||||||
sendError(req, rsp, SC_INTERNAL_SERVER_ERROR, msg);
|
|| e instanceof ServiceNotEnabledException) {
|
||||||
|
msg = e.getMessage();
|
||||||
|
}
|
||||||
|
sendError(req, rsp, statusCodeForThrowable(e), msg);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -23,6 +23,7 @@
|
||||||
import java.io.OutputStream;
|
import java.io.OutputStream;
|
||||||
import java.net.URI;
|
import java.net.URI;
|
||||||
import java.net.URL;
|
import java.net.URL;
|
||||||
|
import java.text.MessageFormat;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
import javax.servlet.http.HttpServletRequest;
|
import javax.servlet.http.HttpServletRequest;
|
||||||
|
@ -308,7 +309,10 @@ public void testListRemote_Smart_UploadPackDisabled() throws Exception {
|
||||||
fail("connection opened even though service disabled");
|
fail("connection opened even though service disabled");
|
||||||
} catch (TransportException err) {
|
} catch (TransportException err) {
|
||||||
String exp = smartAuthNoneURI + ": "
|
String exp = smartAuthNoneURI + ": "
|
||||||
+ JGitText.get().serviceNotEnabledNoName;
|
+ MessageFormat.format(
|
||||||
|
JGitText.get().serviceNotPermitted,
|
||||||
|
smartAuthNoneURI.toString() + "/",
|
||||||
|
"git-upload-pack");
|
||||||
assertEquals(exp, err.getMessage());
|
assertEquals(exp, err.getMessage());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -57,7 +57,7 @@
|
||||||
import org.eclipse.jetty.servlet.ServletHolder;
|
import org.eclipse.jetty.servlet.ServletHolder;
|
||||||
import org.eclipse.jgit.api.Git;
|
import org.eclipse.jgit.api.Git;
|
||||||
import org.eclipse.jgit.api.TransportConfigCallback;
|
import org.eclipse.jgit.api.TransportConfigCallback;
|
||||||
import org.eclipse.jgit.errors.RemoteRepositoryException;
|
import org.eclipse.jgit.errors.NoRemoteRepositoryException;
|
||||||
import org.eclipse.jgit.errors.TransportException;
|
import org.eclipse.jgit.errors.TransportException;
|
||||||
import org.eclipse.jgit.errors.UnsupportedCredentialItem;
|
import org.eclipse.jgit.errors.UnsupportedCredentialItem;
|
||||||
import org.eclipse.jgit.http.server.GitServlet;
|
import org.eclipse.jgit.http.server.GitServlet;
|
||||||
|
@ -496,8 +496,9 @@ public void testListRemote_BadName() throws IOException, URISyntaxException {
|
||||||
try {
|
try {
|
||||||
t.openFetch();
|
t.openFetch();
|
||||||
fail("fetch connection opened");
|
fail("fetch connection opened");
|
||||||
} catch (RemoteRepositoryException notFound) {
|
} catch (NoRemoteRepositoryException notFound) {
|
||||||
assertEquals(uri + ": Git repository not found",
|
assertEquals(uri + ": " + uri
|
||||||
|
+ "/info/refs?service=git-upload-pack not found: Not Found",
|
||||||
notFound.getMessage());
|
notFound.getMessage());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -510,7 +511,7 @@ public void testListRemote_BadName() throws IOException, URISyntaxException {
|
||||||
assertEquals(join(uri, "info/refs"), info.getPath());
|
assertEquals(join(uri, "info/refs"), info.getPath());
|
||||||
assertEquals(1, info.getParameters().size());
|
assertEquals(1, info.getParameters().size());
|
||||||
assertEquals("git-upload-pack", info.getParameter("service"));
|
assertEquals("git-upload-pack", info.getParameter("service"));
|
||||||
assertEquals(200, info.getStatus());
|
assertEquals(404, info.getStatus());
|
||||||
assertEquals("application/x-git-upload-pack-advertisement",
|
assertEquals("application/x-git-upload-pack-advertisement",
|
||||||
info.getResponseHeader(HDR_CONTENT_TYPE));
|
info.getResponseHeader(HDR_CONTENT_TYPE));
|
||||||
}
|
}
|
||||||
|
@ -536,8 +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(
|
||||||
"want " + unreachableCommit.name() + " not valid"));
|
"Bad Request"));
|
||||||
}
|
}
|
||||||
|
assertLastRequestStatusCode(400);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -558,8 +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("want " + A.name() + " not valid"));
|
e.getMessage().contains("Bad Request"));
|
||||||
}
|
}
|
||||||
|
assertLastRequestStatusCode(400);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -916,6 +919,7 @@ public void testInitialClone_RedirectOnPostForbidden() throws Exception {
|
||||||
} catch (TransportException e) {
|
} catch (TransportException e) {
|
||||||
assertTrue(e.getMessage().contains("301"));
|
assertTrue(e.getMessage().contains("301"));
|
||||||
}
|
}
|
||||||
|
assertLastRequestStatusCode(301);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -934,6 +938,7 @@ public void testInitialClone_RedirectForbidden() throws Exception {
|
||||||
assertTrue(
|
assertTrue(
|
||||||
e.getMessage().contains("http.followRedirects is false"));
|
e.getMessage().contains("http.followRedirects is false"));
|
||||||
}
|
}
|
||||||
|
assertLastRequestStatusCode(301);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void assertFetchRequests(List<AccessEvent> requests, int index) {
|
private void assertFetchRequests(List<AccessEvent> requests, int index) {
|
||||||
|
@ -1605,8 +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("want " + id.name() + " not valid"));
|
.contains("Bad Request"));
|
||||||
}
|
}
|
||||||
|
assertLastRequestStatusCode(400);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -1650,7 +1656,7 @@ public void testFetch_RefsUnreadableOnUpload() throws Exception {
|
||||||
fail("Successfully served ref with value " + c.getRef(master));
|
fail("Successfully served ref with value " + c.getRef(master));
|
||||||
} catch (TransportException err) {
|
} catch (TransportException err) {
|
||||||
assertTrue("Unexpected exception message " + err.getMessage(),
|
assertTrue("Unexpected exception message " + err.getMessage(),
|
||||||
err.getMessage().contains("Internal server error"));
|
err.getMessage().contains("Server Error"));
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
noRefServer.tearDown();
|
noRefServer.tearDown();
|
||||||
|
@ -1821,6 +1827,11 @@ public void testPush_ChunkedEncoding() throws Exception {
|
||||||
.getResponseHeader(HDR_CONTENT_TYPE));
|
.getResponseHeader(HDR_CONTENT_TYPE));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void assertLastRequestStatusCode(int statusCode) {
|
||||||
|
List<AccessEvent> requests = getRequests();
|
||||||
|
assertEquals(statusCode, requests.get(requests.size() - 1).getStatus());
|
||||||
|
}
|
||||||
|
|
||||||
private void enableReceivePack() throws IOException {
|
private void enableReceivePack() throws IOException {
|
||||||
final StoredConfig cfg = remoteRepository.getConfig();
|
final StoredConfig cfg = remoteRepository.getConfig();
|
||||||
cfg.setBoolean("http", null, "receivepack", true);
|
cfg.setBoolean("http", null, "receivepack", true);
|
||||||
|
|
Loading…
Reference in New Issue