From c828cf2db88956094a31a79741145688876879df Mon Sep 17 00:00:00 2001 From: James Moger Date: Fri, 23 May 2014 08:17:11 -0400 Subject: Use Guice-Servlet rather than custom code and expose the Injector This is a fairly functional variation of Gitblit with one notable exception: The security filters are not working properly. This is a design flaw in Guice that I have reported upstream [1]. The general idea is that Guice-Servlet filters are not properly wrapping the ServletRequest. This has historically been a problem for Guice-Servlet servlets but Google has fixed most of those issues. Unfortunately, all the same flaws reported against the servlet delegation also exist in Guice-Servlet filter delegation. :( [1]: https://code.google.com/p/google-guice/issues/detail?id=807 --- .../com/gitblit/servlet/AuthenticationFilter.java | 8 +- .../java/com/gitblit/servlet/GitblitContext.java | 81 ++++--- .../gitblit/servlet/InjectionContextListener.java | 241 --------------------- 3 files changed, 43 insertions(+), 287 deletions(-) delete mode 100644 src/main/java/com/gitblit/servlet/InjectionContextListener.java (limited to 'src/main/java/com/gitblit/servlet') diff --git a/src/main/java/com/gitblit/servlet/AuthenticationFilter.java b/src/main/java/com/gitblit/servlet/AuthenticationFilter.java index 6f13252a..35a62365 100644 --- a/src/main/java/com/gitblit/servlet/AuthenticationFilter.java +++ b/src/main/java/com/gitblit/servlet/AuthenticationFilter.java @@ -99,8 +99,12 @@ public abstract class AuthenticationFilter implements Filter { * @return url */ protected String getFullUrl(HttpServletRequest httpRequest) { - String servletUrl = httpRequest.getContextPath() + httpRequest.getServletPath(); - String url = httpRequest.getRequestURI().substring(servletUrl.length()); + String contextPath = httpRequest.getContextPath(); + String servletPath = httpRequest.getServletPath(); + String pathInfo = httpRequest.getPathInfo(); + String servletUrl = contextPath + servletPath; + String requestURI = httpRequest.getRequestURI(); + String url = requestURI.substring(servletUrl.length()); String params = httpRequest.getQueryString(); if (url.length() > 0 && url.charAt(0) == '/') { url = url.substring(1); diff --git a/src/main/java/com/gitblit/servlet/GitblitContext.java b/src/main/java/com/gitblit/servlet/GitblitContext.java index fb16d32d..86b2fb3a 100644 --- a/src/main/java/com/gitblit/servlet/GitblitContext.java +++ b/src/main/java/com/gitblit/servlet/GitblitContext.java @@ -23,24 +23,26 @@ import java.io.InputStream; import java.io.OutputStream; import java.text.MessageFormat; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import javax.naming.Context; import javax.naming.InitialContext; import javax.naming.NamingException; import javax.servlet.ServletContext; +import javax.servlet.ServletContextEvent; import javax.servlet.annotation.WebListener; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.gitblit.Constants; import com.gitblit.FileSettings; import com.gitblit.IStoredSettings; import com.gitblit.Keys; import com.gitblit.WebXmlSettings; import com.gitblit.extensions.LifeCycleListener; -import com.gitblit.guice.GuiceContext; -import com.gitblit.guice.GuiceModule; +import com.gitblit.guice.CoreModule; +import com.gitblit.guice.WebModule; import com.gitblit.manager.IAuthenticationManager; import com.gitblit.manager.IFederationManager; import com.gitblit.manager.IGitblit; @@ -54,9 +56,10 @@ import com.gitblit.manager.IUserManager; import com.gitblit.transport.ssh.IPublicKeyManager; import com.gitblit.utils.ContainerUtils; import com.gitblit.utils.StringUtils; -import com.gitblit.wicket.GitblitWicketFilter; import com.google.inject.AbstractModule; +import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.servlet.GuiceServletContextListener; /** * This class is the main entry point for the entire webapp. It is a singleton @@ -70,10 +73,12 @@ import com.google.inject.Injector; * */ @WebListener -public class GitblitContext extends GuiceContext { +public class GitblitContext extends GuiceServletContextListener { private static GitblitContext gitblit; + protected final Logger logger = LoggerFactory.getLogger(getClass()); + private final List managers = new ArrayList(); private final IStoredSettings goSettings; @@ -115,20 +120,37 @@ public class GitblitContext extends GuiceContext { return null; } + @Override + protected Injector getInjector() { + return Guice.createInjector(getModules()); + } + /** * Returns Gitblit's Guice injection modules. */ - @Override protected AbstractModule [] getModules() { - return new AbstractModule [] { new GuiceModule() }; + return new AbstractModule [] { new CoreModule(), new WebModule() }; } /** - * Prepare runtime settings and start all manager instances. + * Configure Gitblit from the web.xml, if no configuration has already been + * specified. + * + * @see ServletContextListener.contextInitialize(ServletContextEvent) */ @Override - protected void beforeServletInjection(ServletContext context) { - Injector injector = getInjector(context); + public final void contextInitialized(ServletContextEvent contextEvent) { + super.contextInitialized(contextEvent); + + ServletContext context = contextEvent.getServletContext(); + startCore(context); + } + + /** + * Prepare runtime settings and start all manager instances. + */ + protected void startCore(ServletContext context) { + Injector injector = (Injector) context.getAttribute(Injector.class.getName()); // create the runtime settings object IStoredSettings runtimeSettings = injector.getInstance(IStoredSettings.class); @@ -229,46 +251,17 @@ public class GitblitContext extends GuiceContext { logger.info("----[{}]----", clazz.getName()); } - /** - * Instantiate and inject all filters and servlets into the container using - * the servlet 3 specification. - */ @Override - protected void injectServlets(ServletContext context) { - // access restricted servlets - serve(context, Constants.R_PATH, GitServlet.class, GitFilter.class); - serve(context, Constants.GIT_PATH, GitServlet.class, GitFilter.class); - serve(context, Constants.RAW_PATH, RawServlet.class, RawFilter.class); - serve(context, Constants.PAGES, PagesServlet.class, PagesFilter.class); - serve(context, Constants.RPC_PATH, RpcServlet.class, RpcFilter.class); - serve(context, Constants.ZIP_PATH, DownloadZipServlet.class, DownloadZipFilter.class); - serve(context, Constants.SYNDICATION_PATH, SyndicationServlet.class, SyndicationFilter.class); - - // servlets - serve(context, Constants.FEDERATION_PATH, FederationServlet.class); - serve(context, Constants.SPARKLESHARE_INVITE_PATH, SparkleShareInviteServlet.class); - serve(context, Constants.BRANCH_GRAPH_PATH, BranchGraphServlet.class); - serve(context, Constants.PT_PATH, PtServlet.class); - file(context, "/robots.txt", RobotsTxtServlet.class); - file(context, "/logo.png", LogoServlet.class); - - // global filters - filter(context, "/*", ProxyFilter.class, null); - filter(context, "/*", EnforceAuthenticationFilter.class, null); - - // Wicket - String toIgnore = StringUtils.flattenStrings(getRegisteredPaths(), ","); - Map params = new HashMap(); - params.put(GitblitWicketFilter.FILTER_MAPPING_PARAM, "/*"); - params.put(GitblitWicketFilter.IGNORE_PATHS_PARAM, toIgnore); - filter(context, "/*", GitblitWicketFilter.class, params); + public final void contextDestroyed(ServletContextEvent contextEvent) { + super.contextDestroyed(contextEvent); + ServletContext context = contextEvent.getServletContext(); + destroyContext(context); } /** * Gitblit is being shutdown either because the servlet container is * shutting down or because the servlet container is re-deploying Gitblit. */ - @Override protected void destroyContext(ServletContext context) { logger.info("Gitblit context destroyed by servlet container."); diff --git a/src/main/java/com/gitblit/servlet/InjectionContextListener.java b/src/main/java/com/gitblit/servlet/InjectionContextListener.java deleted file mode 100644 index 17de6dad..00000000 --- a/src/main/java/com/gitblit/servlet/InjectionContextListener.java +++ /dev/null @@ -1,241 +0,0 @@ -/* - * Copyright 2014 gitblit.com. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.gitblit.servlet; - -import java.util.ArrayList; -import java.util.EnumSet; -import java.util.List; -import java.util.Map; - -import javax.servlet.DispatcherType; -import javax.servlet.Filter; -import javax.servlet.FilterRegistration; -import javax.servlet.Servlet; -import javax.servlet.ServletContext; -import javax.servlet.ServletContextEvent; -import javax.servlet.ServletContextListener; -import javax.servlet.ServletRegistration; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Injection context listener instantiates and injects servlets, filters, and - * anything else you might want into a servlet context. This class provides - * convenience methods for servlet & filter registration and also tracks - * registered paths. - * - * @author James Moger - * - */ -public abstract class InjectionContextListener implements ServletContextListener { - - protected final Logger logger = LoggerFactory.getLogger(getClass()); - - private final List registeredPaths = new ArrayList(); - - protected final List getRegisteredPaths() { - return registeredPaths; - } - - /** - * Hook for subclasses to manipulate context initialization before - * standard initialization procedure. - * - * @param context - */ - protected void beforeServletInjection(ServletContext context) { - // NOOP - } - - /** - * Hook for subclasses to instantiate and inject servlets and filters - * into the servlet context. - * - * @param context - */ - protected abstract void injectServlets(ServletContext context); - - /** - * Hook for subclasses to manipulate context initialization after - * servlet registration. - * - * @param context - */ - protected void afterServletInjection(ServletContext context) { - // NOOP - } - - /** - * Configure Gitblit from the web.xml, if no configuration has already been - * specified. - * - * @see ServletContextListener.contextInitialize(ServletContextEvent) - */ - @Override - public final void contextInitialized(ServletContextEvent contextEvent) { - ServletContext context = contextEvent.getServletContext(); - beforeServletInjection(context); - injectServlets(context); - afterServletInjection(context); - } - - - /** - * Registers a file path. - * - * @param context - * @param file - * @param servletClass - */ - protected void file(ServletContext context, String file, Class servletClass) { - file(context, file, servletClass, null); - } - - /** - * Registers a file path with init parameters. - * - * @param context - * @param file - * @param servletClass - * @param initParams - */ - protected void file(ServletContext context, String file, Class servletClass, Map initParams) { - Servlet servlet = instantiate(context, servletClass); - ServletRegistration.Dynamic d = context.addServlet(sanitize(servletClass.getSimpleName() + file), servlet); - d.addMapping(file); - if (initParams != null) { - d.setInitParameters(initParams); - } - registeredPaths.add(file); - } - - /** - * Serves a path (trailing wildcard will be appended). - * - * @param context - * @param route - * @param servletClass - */ - protected void serve(ServletContext context, String route, Class servletClass) { - serve(context, route, servletClass, (Class) null); - } - - /** - * Serves a path (trailing wildcard will be appended) with init parameters. - * - * @param context - * @param route - * @param servletClass - * @param initParams - */ - protected void serve(ServletContext context, String route, Class servletClass, Map initParams) { - Servlet servlet = instantiate(context, servletClass); - ServletRegistration.Dynamic d = context.addServlet(sanitize(servletClass.getSimpleName() + route), servlet); - d.addMapping(route + "*"); - if (initParams != null) { - d.setInitParameters(initParams); - } - registeredPaths.add(route); - } - - /** - * Serves a path (trailing wildcard will be appended) and also maps a filter - * to that path. - * - * @param context - * @param route - * @param servletClass - * @param filterClass - */ - protected void serve(ServletContext context, String route, Class servletClass, Class filterClass) { - Servlet servlet = instantiate(context, servletClass); - ServletRegistration.Dynamic d = context.addServlet(sanitize(servletClass.getSimpleName() + route), servlet); - d.addMapping(route + "*"); - if (filterClass != null) { - filter(context, route + "*", filterClass); - } - registeredPaths.add(route); - } - - /** - * Registers a path filter. - * - * @param context - * @param route - * @param filterClass - */ - protected void filter(ServletContext context, String route, Class filterClass) { - filter(context, route, filterClass, null); - } - - /** - * Registers a path filter with init parameters. - * - * @param context - * @param route - * @param filterClass - * @param initParams - */ - protected void filter(ServletContext context, String route, Class filterClass, Map initParams) { - Filter filter = instantiate(context, filterClass); - FilterRegistration.Dynamic d = context.addFilter(sanitize(filterClass.getSimpleName() + route), filter); - d.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), true, route); - if (initParams != null) { - d.setInitParameters(initParams); - } - } - - /** - * Limit the generated servlet/filter names to alpha-numeric values with a - * handful of acceptable other characters. - * - * @param name - * @return a sanitized name - */ - protected String sanitize(String name) { - StringBuilder sb = new StringBuilder(); - for (char c : name.toCharArray()) { - if (Character.isLetterOrDigit(c)) { - sb.append(c); - } else if ('-' == c) { - sb.append(c); - } else if ('*' == c) { - sb.append("all"); - } else if ('.' == c) { - sb.append('.'); - } else { - sb.append('_'); - } - } - return sb.toString(); - } - - /** - * Instantiates an object. - * - * @param clazz - * @return the object - */ - protected X instantiate(ServletContext context, Class clazz) { - try { - return clazz.newInstance(); - } catch (Throwable t) { - logger.error(null, t); - } - return null; - } -} -- cgit v1.2.3