opendev/gerrit
Code Issues Proposed changes

Cleanup the binding of Git into the URL space

Browse Source 
When I removed the /p/ directory from the Git URL access I forgot
Guice can actually setup the request's path info for JGit if we
use serveRegex() with an HttpServlet. This vastly cleans up the
code and makes it a little easier to understand the URL mapping.

Switch back to extending GitServlet, and configure Guice to only
trigger on the 3 major Git suffix patterns. If /p is present at
the start of the URL, Guice will omit it from the path info before
passing on the request to JGit.

Because of the legacy redirection support in UrlModule (to point
web browsers from /p/foo to /q/status:open+project:foo,n,z) we must
bind GitOverHttpModule ahead of WebModule (which binds UrlModule).

Change-Id: I238ef64d9b2255f839b3c9c2f1038c5dbe2ff978
This commit is contained in:
Shawn O. Pearce
2012-01-20 08:02:48 -08:00
parent 31d7c88643
commit ac59f1eeda
7 changed files with 20 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-httpd/src/main/java/com/google/gerrit/httpd/ContainerAuthFilter.java
View File

@@ -49,7 +49,7 @@ import javax.servlet.http.HttpServletResponseWrapper;
* <p>
* This filter should only be configured to run, when authentication is
* configured to trust container authentication. This filter is intended only to
* protect the {@link GitOverHttpFilter} and its handled URLs, which provide remote
* protect the {@link GitOverHttpServlet} and its handled URLs, which provide remote
* repository access over HTTP.
*/
@Singleton

4
gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpModule.java
View File

@@ -38,8 +38,8 @@ public class GitOverHttpModule extends ServletModule {
authFilter = ProjectDigestFilter.class;
}
String git = GitOverHttpFilter.URL_REGEX;
String git = GitOverHttpServlet.URL_REGEX;
filterRegex(git).through(authFilter);
filterRegex(git).through(GitOverHttpFilter.class);
serveRegex(git).with(GitOverHttpServlet.class);
}
}

54
gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpFilter.java → gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java
View File

@@ -37,7 +37,7 @@ import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.http.server.GitFilter;
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;
@@ -65,20 +65,27 @@ import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
/** Serves Git repositories over HTTP. */
@Singleton
public class GitOverHttpFilter extends GitFilter {
public class GitOverHttpServlet extends GitServlet {
private static final long serialVersionUID = 1L;
private static final String ATT_CONTROL = ProjectControl.class.getName();
private static final String ATT_RC = ReceiveCommits.class.getName();
private static final String ID_CACHE = "adv_bases";
public static final String URL_REGEX =
"^/.*/(?:info/refs|git-upload-pack|git-receive-pack)$";
public static final String URL_REGEX;
static {
StringBuilder url = new StringBuilder();
url.append("^(?:/p/|/)(.*/(?:info/refs");
for (String name : GitSmartHttpTools.VALID_SERVICES) {
url.append('|').append(name);
}
url.append("))$");
URL_REGEX = url.toString();
}
static class Module extends AbstractModule {
@Override
@@ -102,7 +109,7 @@ public class GitOverHttpFilter extends GitFilter {
}
@Inject
GitOverHttpFilter(Resolver resolver,
GitOverHttpServlet(Resolver resolver,
UploadFactory upload, UploadFilter uploadFilter,
ReceiveFactory receive, ReceiveFilter receiveFilter) {
setRepositoryResolver(resolver);
@@ -115,29 +122,6 @@ public class GitOverHttpFilter extends GitFilter {
addReceivePackFilter(receiveFilter);
}
@Override
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException {
// JGit doesn't handle parsing the request as-is. It assumes
// the relevant data is available in getPathInfo(), but this
// isn't true in GuiceFilter. Massage the request for JGit.
final HttpServletRequest req = (HttpServletRequest) request;
HttpServletRequestWrapper wrapped = new HttpServletRequestWrapper(req) {
@Override
public String getPathInfo() {
return req.getRequestURI().substring(1);
}
@Override
public String getServletPath() {
return "/";
}
};
super.doFilter(wrapped, response, chain);
}
static class Resolver implements RepositoryResolver<HttpServletRequest> {
private final GitRepositoryManager manager;
private final ProjectControl.Factory projectControlFactory;
@@ -153,9 +137,6 @@ public class GitOverHttpFilter extends GitFilter {
public Repository open(HttpServletRequest req, String projectName)
throws RepositoryNotFoundException, ServiceNotAuthorizedException,
ServiceNotEnabledException {
if (projectName.startsWith("p/")) {
projectName = projectName.substring(2);
}
while (projectName.endsWith("/")) {
projectName = projectName.substring(0, projectName.length() - 1);
}
@@ -170,15 +151,6 @@ public class GitOverHttpFilter extends GitFilter {
}
}
while (projectName.startsWith("/")) {
// Be nice and drop the leading "/" if supplied by an absolute path.
// We don't have a file system hierarchy, just a flat namespace in
// the database's Project entities. We never encode these with a
// leading '/' but users might accidentally include them in Git URLs.
//
projectName = projectName.substring(1);
}
final ProjectControl pc;
try {
pc = projectControlFactory.controlFor(new Project.NameKey(projectName));

2
gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectDigestFilter.java
View File

@@ -59,7 +59,7 @@ import javax.servlet.http.HttpServletResponseWrapper;
* <p>
* The current HTTP request is authenticated by looking up the username from the
* Authorization header and checking the digest response against the stored
* password. This filter is intended only to protect the {@link GitOverHttpFilter}
* password. This filter is intended only to protect the {@link GitOverHttpServlet}
* and its handled URLs, which provide remote repository access over HTTP.
*
* @see <a href="http://www.ietf.org/rfc/rfc2617.txt">RFC 2617</a>

2
gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java
View File

@@ -119,7 +119,7 @@ public class WebModule extends FactoryModule {
install(new UrlModule());
install(new UiRpcModule());
install(new GerritRequestModule());
install(new GitOverHttpFilter.Module());
install(new GitOverHttpServlet.Module());
bind(GitWebConfig.class).toInstance(gitWebConfig);
if (gitWebConfig.getGitwebCGI() != null) {

2
gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
View File

@@ -252,8 +252,8 @@ public class Daemon extends SiteProgram {
final List<Module> modules = new ArrayList<Module>();
modules.add(CacheBasedWebSession.module());
modules.add(HttpContactStoreConnection.module());
modules.add(sysInjector.getInstance(WebModule.class));
modules.add(sysInjector.getInstance(GitOverHttpModule.class));
modules.add(sysInjector.getInstance(WebModule.class));
if (sshd) {
modules.add(sshInjector.getInstance(WebSshGlueModule.class));
modules.add(new ProjectQoSFilter.Module());

2
gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
View File

@@ -205,8 +205,8 @@ public class WebAppInitializer extends GuiceServletContextListener {
private Injector createWebInjector() {
final List<Module> modules = new ArrayList<Module>();
modules.add(sshInjector.getInstance(WebModule.class));
modules.add(sysInjector.getInstance(GitOverHttpModule.class));
modules.add(sshInjector.getInstance(WebModule.class));
modules.add(sshInjector.getInstance(WebSshGlueModule.class));
modules.add(CacheBasedWebSession.module());
modules.add(HttpContactStoreConnection.module());