CandyShop / gerrit

Automatically exported from code.google.com/p/gerrit
Apache License 2.0
1 stars 0 forks source link

Plugins that instantiate web servlet cause "404 not found" error when running Gerrit in tomcat. #1966

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
************************************************************
***** NOTE: THIS BUG TRACKER IS FOR GERRIT CODE REVIEW *****
***** DO NOT SUBMIT BUGS FOR CHROME, ANDROID, INTERNAL *****
***** ISSUES WITH YOUR COMPANY'S GERRIT SETUP, ETC.    *****
***** THOSE ISSUE BELONG IN DIFFERENT ISSUE TRACKERS!  *****
************************************************************

Affected Version: 2.5.x, 2.6-rcx

What steps will reproduce the problem?
1. Install and run gerrit in tomcat
2. Add a plugin that instantiates a web servelet to handle web access in some 
form
3. Try to access the main webpage.

What is the expected output? What do you see instead?

Expected: Normal gerrit and ability to access the pages provided by gerrit

Found: The servelet overrides gerrit's "servelet" and thus breaks both.

Please provide any additional information below.

This has been tested with both in house plugins and Lucas branch-network.

Everything works fine in Jetty.

Original issue reported on code.google.com by Ian.Kuml...@gmail.com on 18 Jun 2013 at 12:10

GoogleCodeExporter commented 9 years ago
We have the same issue with Gitblit plugin

Original comment by bassem.rabil on 10 Jul 2013 at 8:08

GoogleCodeExporter commented 9 years ago
Issue 2004 has been merged into this issue.

Original comment by david.pu...@sonymobile.com on 12 Jul 2013 at 5:17

GoogleCodeExporter commented 9 years ago

Original comment by david.pu...@sonymobile.com on 12 Jul 2013 at 5:19

GoogleCodeExporter commented 9 years ago
Has someone tried to verify that this issue still exists on current master 
(2.8)?

Original comment by David.Os...@gmail.com on 12 Jul 2013 at 6:16

GoogleCodeExporter commented 9 years ago
i can reproduce the problem on stable-2.6.

The culprit seems to be that the ManagedServletPipeline-Singleton get 
overridden,
when Plugin with HTP Module is bound.

@Singleton
class ManagedServletPipeline
[...]

  public boolean service(ServletRequest request, ServletResponse response)
      throws IOException, ServletException {

89    //stop at the first matching servlet and service
90    for (ServletDefinition servletDefinition : servletDefinitions) {
91      if (servletDefinition.service(request, response)) {
92        return true;
93      }

Without plugins the pipeline contains 40 Gerrit core servlets in line 90.
And no one plugin owned, (only .*plugins.*).

With plugin loaded (with HTTP-Module), the servletDefinitions collected twice,
and the Gerrit core servlets get overridden by plugin owned. To request time,
there is only plugin owned requests, with pattern like "/log" (from plugin),
and clearly there is no match for pattern "/".

Interestingly and may be related to original problem: our own buildbot-plugin
doesn't even start on tomcat. While it is all fine
on jetty, the same plugin in tomcat fails to start, with 

class BuildbotModule extends AbstractModule {

    @Override
    protected void configure() {
        bind(BuildbotConfig.class).toProvider(BuildbotConfigProvider.class).in(
                SINGLETON);
36        bind(BuildbotLogicControl.class).toProvider(
37                BuildbotLogicControlProvider.class).in(SINGLETON);
38        bind(AllProjectsName.class).toProvider(AllProjectsNameProvider.class);

1) A binding to com.google.gerrit.server.config.AllProjectsName was already 
configured at 
com.google.gerrit.server.plugins.PluginGuiceEnvironment$2.configure(PluginGuiceE
nvironment.java:495).
  at org.libreoffice.ci.gerrit.buildbot.BuildbotModule.configure(BuildbotModule.java:38)

1 error
at 
com.google.inject.internal.Errors.throwCreationExceptionIfErrorsExist(Errors.jav
a:435)
at 
com.google.inject.internal.InternalInjectorCreator.initializeStatically(Internal
InjectorCreator.java:154)

So the line PluginGuiceEnvironment.java:495 seems to behave differently on 
tomcat and jetty:

    return new AbstractModule() {
      @SuppressWarnings("unchecked")
      @Override
      protected void configure() {
        for (Map.Entry<Key<?>, Binding<?>> e : bindings.entrySet()) {
          Key<Object> k = (Key<Object>) e.getKey();
          Binding<Object> b = (Binding<Object>) e.getValue();
495          bind(k).toProvider(b.getProvider());
        }
      }
    };

On Jetty, when i uncomment the line 

38        bind(AllProjectsName.class).toProvider(AllProjectsNameProvider.class);

in Buildbot-plugin above, the plugin doesn't start with the error:

1) Could not find a suitable constructor in 
com.google.gerrit.server.config.AllProjectsName. 
Classes must have either one (and only one) constructor annotated with 
@Inject or a zero-argument constructor that is not private.
  at com.google.gerrit.server.config.AllProjectsName.class(AllProjectsName.java:23)
  while locating com.google.gerrit.server.config.AllProjectsName
    for field at org.libreoffice.ci.gerrit.buildbot.commands.VerifyCommand.allProjects(VerifyCommand.java:46)
  at com.google.gerrit.sshd.CommandModule.command(CommandModule.java:77)

1 error
    at com.google.inject.internal.Errors.throwCreationExceptionIfErrorsExist(Errors.java:435)
    at com.google.inject.internal.InternalInjectorCreator.initializeStatically(InternalInjectorCreator.java:154)

Original comment by David.Os...@gmail.com on 13 Jul 2013 at 9:33

GoogleCodeExporter commented 9 years ago
OK i see what happens:

the problem seems to be related to guice-servlet bug or missing feature in 
handling of multiple filter pipelines.

When this method from startPlugin(PluginGuiceEnvironment env) called:
        httpInjector = sysInjector.createChildInjector(modules);

then we are getting this warning.

  //VisibleForTesting
  @Inject
  static void setPipeline(FilterPipeline pipeline) {

    // This can happen if you create many injectors and they all have their own
    // servlet module. This is legal, caveat a small warning.
    if (GuiceFilter.pipeline instanceof ManagedFilterPipeline) {
=>        
Logger.getLogger(GuiceFilter.class.getName()).warning(MULTIPLE_INJECTORS_WARNING
);
    }

    // We overwrite the default pipeline
    GuiceFilter.pipeline = pipeline; 
  }

the pipeline from Gerrit core is clobbered and is never accessible again.
All Gerrit core servlet definitions are lost.

This is a known bug. I wonder why it doesn't happen in Jetty?

http://code.google.com/p/google-guice/issues/detail?id=635
https://groups.google.com/forum/#!msg/google-guice/wJBwzE5E7Y0/1LQJhCrmDnwJ

Original comment by David.Os...@gmail.com on 14 Jul 2013 at 7:00

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Fixed: https://gerrit-review.googlesource.com/47922/

Original comment by David.Os...@gmail.com on 16 Jul 2013 at 9:52

GoogleCodeExporter commented 9 years ago

Original comment by sop@google.com on 12 Aug 2013 at 6:59

GoogleCodeExporter commented 9 years ago

Original comment by david.pu...@sonymobile.com on 13 Aug 2013 at 2:51

GoogleCodeExporter commented 9 years ago

Original comment by david.pu...@sonymobile.com on 10 Dec 2013 at 4:33

GoogleCodeExporter commented 9 years ago

Original comment by david.pu...@sonymobile.com on 10 Dec 2013 at 4:35

GoogleCodeExporter commented 9 years ago

Original comment by david.pu...@sonymobile.com on 10 Dec 2013 at 4:40

GoogleCodeExporter commented 9 years ago
On Gerrit v2.10-rc0 in Tomcat:
WARN  com.google.gerrit.server.plugins.PluginLoader : Cannot load plugin gitiles
com.google.inject.CreationException: Guice creation errors:

1) A binding to com.google.gerrit.server.config.AllProjectsName was already 
configured at 
com.google.gerrit.server.plugins.PluginGuiceEnvironment$2.configure(PluginGuiceE
nvironment.java:532).
  at com.googlesource.gerrit.plugins.gitiles.Module.configure(Module.java:50)

1 error
    at com.google.inject.internal.Errors.throwCreationExceptionIfErrorsExist(Errors.java:448
...

Original comment by sven.sel...@sonymobile.com on 20 Nov 2014 at 3:49

GoogleCodeExporter commented 9 years ago
"This is a known bug. I wonder why it doesn't happen in Jetty?"
It does now!

Gerrit - v2.11-rc0
Gitiles - master (a4dd362 Update soy and gitiles-servlet to recent versions)
Under Jetty:

------------ "error_log" -----------

2015-02-27 10:58:14,476] WARN  com.google.gerrit.server.plugins.PluginLoader : 
Cannot load plugin gitiles
com.google.inject.CreationException: Unable to create injector, see the 
following errors:

1) A binding to com.google.gerrit.server.config.AllProjectsName was already 
configured at 
com.google.gerrit.server.plugins.PluginGuiceEnvironment$2.configure(PluginGuiceE
nvironment.java:534) (via modules: 
com.google.gerrit.server.plugins.PluginGuiceEnvironment$1 -> 
com.google.gerrit.server.plugins.PluginGuiceEnvironment$2).
  at com.googlesource.gerrit.plugins.gitiles.Module.configure(Module.java:50)

1 error
        at com.google.inject.internal.Errors.throwCreationExceptionIfErrorsExist(Errors.java:448)
        at com.google.inject.internal.InternalInjectorCreator.initializeStatically(InternalInjectorCreator.java:155)
        at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:107)

Original comment by sven.sel...@sonymobile.com on 27 Feb 2015 at 11:42

GoogleCodeExporter commented 9 years ago
Nevermind

https://gerrit-review.googlesource.com/#/c/65500/

Original comment by sven.sel...@sonymobile.com on 27 Feb 2015 at 11:48