smart HTTP: Return errors inside payload
When the client is clearly making a smart HTTP request to our smart HTTP server, return any errors like RepositoryNotFoundException or ServiceNotEnabledException inside of the payload as a Git level ERR message, rather than an HTTP error code. This prevents the C Git command line client from retrying a failed "$URL/info/refs?service=git-upload-pack" request without the smart service URL, only to fail again with "403 Forbidden" when the dumb as-is service has been disabled by the server configuration, or is unavailable because the repository is not on the local filesystem. Change-Id: I57e8756d5026e885e0ca615979bfcd729703be6c Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
This commit is contained in:
parent
64b524e3ca
commit
58ed0cb840
|
@ -13,6 +13,8 @@ noResolverAvailable=No resolver available
|
|||
parameterNotSet=Parameter {0} not set
|
||||
pathForParamNotFound={0} (for {1}) not found
|
||||
pathNotSupported={0} not supported
|
||||
repositoryAccessForbidden=Git access forbidden
|
||||
repositoryNotFound=Git repository not found
|
||||
servletAlreadyInitialized=Servlet already initialized
|
||||
servletMustNotBeNull=servlet must not be null
|
||||
servletWasAlreadyBound=servlet was already bound
|
||||
|
|
|
@ -73,6 +73,8 @@ public static HttpServerText get() {
|
|||
/***/ public String parameterNotSet;
|
||||
/***/ public String pathForParamNotFound;
|
||||
/***/ public String pathNotSupported;
|
||||
/***/ public String repositoryAccessForbidden;
|
||||
/***/ public String repositoryNotFound;
|
||||
/***/ public String servletAlreadyInitialized;
|
||||
/***/ public String servletMustNotBeNull;
|
||||
/***/ public String servletWasAlreadyBound;
|
||||
|
|
|
@ -128,7 +128,7 @@ public void doFilter(ServletRequest request, ServletResponse response,
|
|||
return;
|
||||
|
||||
} catch (ServiceNotEnabledException e) {
|
||||
rsp.sendError(SC_FORBIDDEN);
|
||||
RepositoryFilter.sendError(SC_FORBIDDEN, req, rsp);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
@ -48,6 +48,7 @@
|
|||
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
|
||||
import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
|
||||
import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY;
|
||||
import static org.eclipse.jgit.util.HttpSupport.HDR_ACCEPT;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.text.MessageFormat;
|
||||
|
@ -64,6 +65,7 @@
|
|||
|
||||
import org.eclipse.jgit.errors.RepositoryNotFoundException;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
import org.eclipse.jgit.transport.PacketLineOut;
|
||||
import org.eclipse.jgit.transport.resolver.RepositoryResolver;
|
||||
import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
|
||||
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
|
||||
|
@ -131,14 +133,14 @@ public void doFilter(final ServletRequest request,
|
|||
try {
|
||||
db = resolver.open(req, name);
|
||||
} catch (RepositoryNotFoundException e) {
|
||||
((HttpServletResponse) rsp).sendError(SC_NOT_FOUND);
|
||||
sendError(SC_NOT_FOUND, req, (HttpServletResponse) rsp);
|
||||
return;
|
||||
} catch (ServiceNotEnabledException e) {
|
||||
sendError(SC_FORBIDDEN, req, (HttpServletResponse) rsp);
|
||||
return;
|
||||
} catch (ServiceNotAuthorizedException e) {
|
||||
((HttpServletResponse) rsp).sendError(SC_UNAUTHORIZED);
|
||||
return;
|
||||
} catch (ServiceNotEnabledException e) {
|
||||
((HttpServletResponse) rsp).sendError(SC_FORBIDDEN);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
request.setAttribute(ATTRIBUTE_REPOSITORY, db);
|
||||
|
@ -148,4 +150,54 @@ public void doFilter(final ServletRequest request,
|
|||
db.close();
|
||||
}
|
||||
}
|
||||
|
||||
static void sendError(int statusCode, HttpServletRequest req,
|
||||
HttpServletResponse rsp) throws IOException {
|
||||
String svc = req.getParameter("service");
|
||||
String accept = req.getHeader(HDR_ACCEPT);
|
||||
|
||||
if (svc != null && svc.startsWith("git-") && accept != null
|
||||
&& accept.contains("application/x-" + svc + "-advertisement")) {
|
||||
// Smart HTTP service request, use an ERR response.
|
||||
rsp.setContentType("application/x-" + svc + "-advertisement");
|
||||
|
||||
SmartOutputStream buf = new SmartOutputStream(req, rsp);
|
||||
PacketLineOut out = new PacketLineOut(buf);
|
||||
out.writeString("# service=" + svc + "\n");
|
||||
out.end();
|
||||
out.writeString("ERR " + translate(statusCode));
|
||||
buf.close();
|
||||
return;
|
||||
}
|
||||
|
||||
if (accept != null && accept.contains(UploadPackServlet.RSP_TYPE)) {
|
||||
// An upload-pack wants ACK or NAK, return ERR
|
||||
// and the client will print this instead.
|
||||
rsp.setContentType(UploadPackServlet.RSP_TYPE);
|
||||
SmartOutputStream buf = new SmartOutputStream(req, rsp);
|
||||
PacketLineOut out = new PacketLineOut(buf);
|
||||
out.writeString("ERR " + translate(statusCode));
|
||||
buf.close();
|
||||
return;
|
||||
}
|
||||
|
||||
// Otherwise fail with an HTTP error code instead of an
|
||||
// application level message. This may not be as pretty
|
||||
// of a result for the user, but its better than nothing.
|
||||
//
|
||||
rsp.sendError(statusCode);
|
||||
}
|
||||
|
||||
private static String translate(int statusCode) {
|
||||
switch (statusCode) {
|
||||
case SC_NOT_FOUND:
|
||||
return HttpServerText.get().repositoryNotFound;
|
||||
|
||||
case SC_FORBIDDEN:
|
||||
return HttpServerText.get().repositoryAccessForbidden;
|
||||
|
||||
default:
|
||||
return String.valueOf(statusCode);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -77,7 +77,7 @@
|
|||
class UploadPackServlet extends HttpServlet {
|
||||
private static final String REQ_TYPE = "application/x-git-upload-pack-request";
|
||||
|
||||
private static final String RSP_TYPE = "application/x-git-upload-pack-result";
|
||||
static final String RSP_TYPE = "application/x-git-upload-pack-result";
|
||||
|
||||
private static final long serialVersionUID = 1L;
|
||||
|
||||
|
@ -130,7 +130,7 @@ public void doFilter(ServletRequest request, ServletResponse response,
|
|||
return;
|
||||
|
||||
} catch (ServiceNotEnabledException e) {
|
||||
rsp.sendError(SC_FORBIDDEN);
|
||||
RepositoryFilter.sendError(SC_FORBIDDEN, req, rsp);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
@ -55,6 +55,7 @@
|
|||
|
||||
import java.io.IOException;
|
||||
import java.io.PrintWriter;
|
||||
import java.net.URISyntaxException;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
@ -73,6 +74,7 @@
|
|||
import org.eclipse.jetty.servlet.ServletContextHandler;
|
||||
import org.eclipse.jetty.servlet.ServletHolder;
|
||||
import org.eclipse.jgit.JGitText;
|
||||
import org.eclipse.jgit.errors.RemoteRepositoryException;
|
||||
import org.eclipse.jgit.errors.RepositoryNotFoundException;
|
||||
import org.eclipse.jgit.errors.TransportException;
|
||||
import org.eclipse.jgit.http.server.GitServlet;
|
||||
|
@ -223,6 +225,36 @@ public void testListRemote() throws IOException {
|
|||
assertEquals("gzip", info.getResponseHeader(HDR_CONTENT_ENCODING));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testListRemote_BadName() throws IOException, URISyntaxException {
|
||||
Repository dst = createBareRepository();
|
||||
URIish uri = new URIish(this.remoteURI.toString() + ".invalid");
|
||||
Transport t = Transport.open(dst, uri);
|
||||
try {
|
||||
try {
|
||||
t.openFetch();
|
||||
fail("fetch connection opened");
|
||||
} catch (RemoteRepositoryException notFound) {
|
||||
assertEquals(uri + ": Git repository not found",
|
||||
notFound.getMessage());
|
||||
}
|
||||
} finally {
|
||||
t.close();
|
||||
}
|
||||
|
||||
List<AccessEvent> requests = getRequests();
|
||||
assertEquals(1, requests.size());
|
||||
|
||||
AccessEvent info = requests.get(0);
|
||||
assertEquals("GET", info.getMethod());
|
||||
assertEquals(join(uri, "info/refs"), info.getPath());
|
||||
assertEquals(1, info.getParameters().size());
|
||||
assertEquals("git-upload-pack", info.getParameter("service"));
|
||||
assertEquals(200, info.getStatus());
|
||||
assertEquals("application/x-git-upload-pack-advertisement",
|
||||
info.getResponseHeader(HDR_CONTENT_TYPE));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_Small() throws Exception {
|
||||
Repository dst = createBareRepository();
|
||||
|
|
Loading…
Reference in New Issue