opendev/gerrit
Code Issues Proposed changes

Use custom error messages for Git-over-HTTP

Browse Source 
Ensure clients see messages related to contributor agreement not
being activated even if they push over HTTP. These used to be put
into the server error log, but the new GitSmartHttpTools class in
JGit allows formatting it directly to the client.

Refactor other things like the server-level receive and upload
controls to also report a custom error message that matches with
the SSH version of the same code.

Change-Id: Idd35853198fcb3103ebb099bab185c0620573e0f
This commit is contained in:
Shawn O. Pearce
2011-12-05 14:58:31 -08:00
parent 9fb183161d
commit 298afc1766
1 changed files with 87 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

170
gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectServlet.java
View File

@@ -40,6 +40,8 @@ import com.google.inject.name.Named;
import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.http.server.GitServlet; import org.eclipse.jgit.http.server.GitServlet;
import org.eclipse.jgit.http.server.GitSmartHttpTools;
import org.eclipse.jgit.http.server.ServletUtils;
import org.eclipse.jgit.http.server.resolver.AsIsFileService; import org.eclipse.jgit.http.server.resolver.AsIsFileService;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
@@ -51,8 +53,6 @@ import org.eclipse.jgit.transport.resolver.RepositoryResolver;
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;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException; import java.io.IOException;
import java.util.Collections; import java.util.Collections;
@@ -76,8 +76,6 @@ import javax.servlet.http.HttpServletResponse;
@Singleton @Singleton
public class ProjectServlet extends GitServlet { public class ProjectServlet extends GitServlet {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
private static final Logger log =
LoggerFactory.getLogger(ProjectServlet.class);
private static final String ATT_CONTROL = ProjectControl.class.getName(); private static final String ATT_CONTROL = ProjectControl.class.getName();
private static final String ATT_RC = ReceiveCommits.class.getName(); private static final String ATT_RC = ReceiveCommits.class.getName();
@@ -87,8 +85,9 @@ public class ProjectServlet extends GitServlet {
@Override @Override
protected void configure() { protected void configure() {
bind(Resolver.class); bind(Resolver.class);
bind(Upload.class); bind(UploadFactory.class);
bind(Receive.class); bind(UploadFilter.class);
bind(ReceiveFactory.class);
bind(ReceiveFilter.class); bind(ReceiveFilter.class);
install(new CacheModule() { install(new CacheModule() {
@Override @Override
@@ -103,28 +102,21 @@ public class ProjectServlet extends GitServlet {
} }
} }
static ProjectControl getProjectControl(ServletRequest req)
throws ServiceNotEnabledException {
ProjectControl pc = (ProjectControl) req.getAttribute(ATT_CONTROL);
if (pc == null) {
log.error("No " + ATT_CONTROL + " in request", new Exception("here"));
throw new ServiceNotEnabledException();
}
return pc;
}
private final Provider<String> urlProvider; private final Provider<String> urlProvider;
@Inject @Inject
ProjectServlet(final Resolver resolver, final Upload upload, ProjectServlet(final Resolver resolver,
final Receive receive, final UploadFactory upload, final UploadFilter uploadFilter,
final ReceiveFilter receiveFilter, final ReceiveFactory receive, final ReceiveFilter receiveFilter,
@CanonicalWebUrl @Nullable Provider<String> urlProvider) { @CanonicalWebUrl @Nullable Provider<String> urlProvider) {
this.urlProvider = urlProvider; this.urlProvider = urlProvider;
setRepositoryResolver(resolver); setRepositoryResolver(resolver);
setAsIsFileService(AsIsFileService.DISABLED); setAsIsFileService(AsIsFileService.DISABLED);
setUploadPackFactory(upload); setUploadPackFactory(upload);
addUploadPackFilter(uploadFilter);
setReceivePackFactory(receive); setReceivePackFactory(receive);
addReceivePackFilter(receiveFilter); addReceivePackFilter(receiveFilter);
} }
@@ -133,20 +125,13 @@ public class ProjectServlet extends GitServlet {
public void init(ServletConfig config) throws ServletException { public void init(ServletConfig config) throws ServletException {
super.init(config); super.init(config);
serveRegex("^/(.*?)/?$").with(new HttpServlet() { serve("*").with(new HttpServlet() {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
@Override @Override
protected void doGet(HttpServletRequest req, HttpServletResponse rsp) protected void doGet(HttpServletRequest req, HttpServletResponse rsp)
throws IOException { throws IOException {
ProjectControl pc; ProjectControl pc = (ProjectControl) req.getAttribute(ATT_CONTROL);
try {
pc = getProjectControl(req);
} catch (ServiceNotEnabledException e) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
}
Project.NameKey dst = pc.getProject().getNameKey(); Project.NameKey dst = pc.getProject().getNameKey();
StringBuilder r = new StringBuilder(); StringBuilder r = new StringBuilder();
r.append(urlProvider.get()); r.append(urlProvider.get());
@@ -198,8 +183,7 @@ public class ProjectServlet extends GitServlet {
final ProjectControl pc; final ProjectControl pc;
try { try {
final Project.NameKey nameKey = new Project.NameKey(projectName); pc = projectControlFactory.controlFor(new Project.NameKey(projectName));
pc = projectControlFactory.controlFor(nameKey);
} catch (NoSuchProjectException err) { } catch (NoSuchProjectException err) {
throw new RepositoryNotFoundException(projectName); throw new RepositoryNotFoundException(projectName);
} }
@@ -216,75 +200,86 @@ public class ProjectServlet extends GitServlet {
} }
} }
static class Upload implements UploadPackFactory<HttpServletRequest> { static class UploadFactory implements UploadPackFactory<HttpServletRequest> {
private final Provider<ReviewDb> db;
private final PackConfig packConfig; private final PackConfig packConfig;
private final TagCache tagCache;
@Inject @Inject
Upload(final Provider<ReviewDb> db, final TransferConfig tc, UploadFactory(final TransferConfig tc) {
final TagCache tagCache) {
this.db = db;
this.packConfig = tc.getPackConfig(); this.packConfig = tc.getPackConfig();
this.tagCache = tagCache;
} }
@Override @Override
public UploadPack create(HttpServletRequest req, Repository repo) public UploadPack create(HttpServletRequest req, Repository repo) {
throws ServiceNotEnabledException, ServiceNotAuthorizedException {
ProjectControl pc = getProjectControl(req);
if (!pc.canRunUploadPack()) {
throw new ServiceNotAuthorizedException();
}
// The Resolver above already checked READ access for us.
//
UploadPack up = new UploadPack(repo); UploadPack up = new UploadPack(repo);
up.setPackConfig(packConfig); up.setPackConfig(packConfig);
if (!pc.allRefsAreVisible()) {
up.setRefFilter(new VisibleRefFilter(tagCache, repo, pc, db.get(), true));
}
return up; return up;
} }
} }
static class Receive implements ReceivePackFactory<HttpServletRequest> { static class UploadFilter implements Filter {
private final Provider<ReviewDb> db;
private final TagCache tagCache;
@Inject
UploadFilter(Provider<ReviewDb> db, TagCache tagCache) {
this.db = db;
this.tagCache = tagCache;
}
@Override
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain next) throws IOException, ServletException {
// The Resolver above already checked READ access for us.
Repository repo = ServletUtils.getRepository(request);
ProjectControl pc = (ProjectControl) request.getAttribute(ATT_CONTROL);
UploadPack up = (UploadPack) request.getAttribute(ServletUtils.ATTRIBUTE_HANDLER);
if (!pc.canRunUploadPack()) {
GitSmartHttpTools.sendError((HttpServletRequest) request,
(HttpServletResponse) response, HttpServletResponse.SC_FORBIDDEN,
"upload-pack not permitted on this server");
return;
}
if (!pc.allRefsAreVisible()) {
up.setRefFilter(new VisibleRefFilter(tagCache, repo, pc, db.get(), true));
}
next.doFilter(request, response);
}
@Override
public void init(FilterConfig config) {
}
@Override
public void destroy() {
}
}
static class ReceiveFactory implements ReceivePackFactory<HttpServletRequest> {
private final ReceiveCommits.Factory factory; private final ReceiveCommits.Factory factory;
@Inject @Inject
Receive(final ReceiveCommits.Factory factory) { ReceiveFactory(final ReceiveCommits.Factory factory) {
this.factory = factory; this.factory = factory;
} }
@Override @Override
public ReceivePack create(HttpServletRequest req, Repository db) public ReceivePack create(HttpServletRequest req, Repository db)
throws ServiceNotEnabledException, ServiceNotAuthorizedException { throws ServiceNotAuthorizedException {
final ProjectControl pc = getProjectControl(req); final ProjectControl pc = (ProjectControl) req.getAttribute(ATT_CONTROL);
if (!pc.canRunReceivePack()) {
if (!(pc.getCurrentUser() instanceof IdentifiedUser)) {
// Anonymous users are not permitted to push.
throw new ServiceNotAuthorizedException(); throw new ServiceNotAuthorizedException();
} }
if (pc.getCurrentUser() instanceof IdentifiedUser) { final IdentifiedUser user = (IdentifiedUser) pc.getCurrentUser();
final IdentifiedUser user = (IdentifiedUser) pc.getCurrentUser(); final ReceiveCommits rc = factory.create(pc, db);
final ReceiveCommits rc = factory.create(pc, db); rc.getReceivePack().setRefLogIdent(user.newRefLogIdent());
final Capable s = rc.canUpload(); req.setAttribute(ATT_RC, rc);
if (s != Capable.OK) { return rc.getReceivePack();
// TODO We should alert the user to this message on the HTTP
// response channel, assuming Git will even report it to them.
//
final String who = user.getUserName();
final String why = s.getMessage();
log.warn("Rejected push from " + who + ": " + why);
throw new ServiceNotEnabledException();
}
rc.getReceivePack().setRefLogIdent(user.newRefLogIdent());
req.setAttribute(ATT_RC, rc);
return rc.getReceivePack();
} else {
throw new ServiceNotAuthorizedException();
}
} }
} }
@@ -305,15 +300,24 @@ public class ProjectServlet extends GitServlet {
ReceiveCommits rc = (ReceiveCommits) request.getAttribute(ATT_RC); ReceiveCommits rc = (ReceiveCommits) request.getAttribute(ATT_RC);
ReceivePack rp = rc.getReceivePack(); ReceivePack rp = rc.getReceivePack();
ProjectControl pc; ProjectControl pc = (ProjectControl) request.getAttribute(ATT_CONTROL);
try { IdentifiedUser user = (IdentifiedUser) pc.getCurrentUser();
pc = getProjectControl(request); Project.NameKey projectName = pc.getProject().getNameKey();
} catch (ServiceNotEnabledException e) {
// This shouldn't occur, the parent should have stopped processing. if (!pc.canRunReceivePack()) {
throw new ServletException(e); GitSmartHttpTools.sendError((HttpServletRequest) request,
(HttpServletResponse) response, HttpServletResponse.SC_FORBIDDEN,
"receive-pack not permitted on this server");
return;
} }
Project.NameKey projectName = pc.getProject().getNameKey(); final Capable s = rc.canUpload();
if (s != Capable.OK) {
GitSmartHttpTools.sendError((HttpServletRequest) request,
(HttpServletResponse) response, HttpServletResponse.SC_FORBIDDEN,
s.getMessage());
return;
}
if (!rp.isCheckReferencedObjectsAreReachable()) { if (!rp.isCheckReferencedObjectsAreReachable()) {
if (isGet) { if (isGet) {
@@ -352,7 +356,7 @@ public class ProjectServlet extends GitServlet {
} }
@Override @Override
public void init(FilterConfig arg0) throws ServletException { public void init(FilterConfig arg0) {
} }
@Override @Override